From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) by sourceware.org (Postfix) with ESMTPS id 64D513858D3C for ; Mon, 16 May 2022 16:40:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 64D513858D3C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=maskray.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pf1-f170.google.com with SMTP id 204so14541802pfx.3 for ; Mon, 16 May 2022 09:40:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=7lfRfWhJIwrF3hSCPSyfiG0+SsCw2Ula5rK/rK3J+zg=; b=tYbReJblcaIHZG4ifm1J6QAnRV2Tu4UqEqEyun+kATEmM1PVGEOEc1GviLSMXO5i/O J7pwcFz8dkeeC75WqJlkv1PBmLSYlnW0J+/4GF1FGAzGbAN4i+/a3rkKihH6Nd077BCV SJj0i28I6d7UWNEcC/myOsvIJjVlQGiHi/VjdEaJQXyjpChyz9MfMS4NWQaEi4SiX82V 3zDP/r3UB4hmVIKsb9J6RohaMrHL1E8fpb2Bq6lIv46vN2rz98FClgScRJBN5Hwvawhr aDsAkDBDtLPmIYQwhXbaUPJ1VBkhkzXTVxbbtFeWDzZB8NgkDAlDeEs3RgBchGZiJNUg KBqg== X-Gm-Message-State: AOAM532cSlIQvGQmXcjX/LMl/Iic/lHOsmgD8Xo0u6qZotJAEmTHMpyZ MTNIPbdj2INRYnroO+PDvGr6JwJrAYk= X-Google-Smtp-Source: ABdhPJwTJs9rluSZL3o5qeCQppzLxLubUBG9WKnNX3EeS2BVMwlZcL5juf3izGKCu0jWHoHgDjCJqw== X-Received: by 2002:a63:534b:0:b0:3db:aa8f:ff1e with SMTP id t11-20020a63534b000000b003dbaa8fff1emr12867613pgl.570.1652719204973; Mon, 16 May 2022 09:40:04 -0700 (PDT) Received: from localhost ([2601:647:6300:b760:7aa:8fbf:a798:13ee]) by smtp.gmail.com with ESMTPSA id jc13-20020a17090325cd00b001618b4d86b3sm1742743plb.180.2022.05.16.09.40.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 May 2022 09:40:04 -0700 (PDT) Date: Mon, 16 May 2022 09:40:03 -0700 From: Fangrui Song To: luca.boccassi@gmail.com Cc: binutils@sourceware.org Subject: Re: [PATCH] ld: add --package-metadata Message-ID: <20220516164003.iivw6lthknhvce42@gmail.com> References: <20220515191846.114257-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20220515191846.114257-1-luca.boccassi@gmail.com> X-Spam-Status: No, score=-8.9 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_INFOUSMEBIZ, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2022 16:40:10 -0000 On 2022-05-15, luca.boccassi--- via Binutils wrote: >From: Luca Boccassi > >Generate a .note.package FDO package metadata ELF note, following >the spec: https://systemd.io/COREDUMP_PACKAGE_METADATA/ > >If the jansson library is available at build time (and not explicitly >disabled), link ld to it, and use it to validate that the input is >correct JSON, to avoid writing garbage to the file. >If no configure option is passed and jansson (or pkg-config) is not >found, then it is just skipped (with a non-fatal warning). The >configure option --enable-jansson can be used to explicitly enable it >(error out when not found) or to explicitly disable it (don't even >look for it). This allows bootstrappers (or others who are not >interested) to seamlessly skip it without issues." >--- >Now that this spec has shipped for all packages in Fedora 36, it seems >like a good time to add a specific option for it to the default linker. >Follows the same pattern of --build-id. Both the "FDO" and the json dependency make me concerned - if a linker script approach works quite well, why bother with a new linker option with a very specific application? --build-id is a bit different in that it adds a section whose content is computed by the linker. I am not sure having --build-id implies the necessity of --package-metadata. >I've added an optional usage of libjansson to provide additional validation, >so that it doesn't have to be reimplemented everywhere a thousand times over. >For bootstrapping purposes the dep is optional and can be skipped, for example >in Debian/Ubuntu it would be annotated with so that everything >works out of the box. > > bfd/elf-bfd.h | 8 ++ > bfd/elf.c | 6 +- > ld/Makefile.am | 6 +- > ld/configure.ac | 35 ++++++++ > ld/emultempl/elf.em | 9 ++ > ld/ld.texi | 7 ++ > ld/ldelf.c | 121 ++++++++++++++++++++++++++- > ld/ldelf.h | 2 + > ld/lexsup.c | 2 + > ld/testsuite/ld-elf/package-note.exp | 49 +++++++++++ > ld/testsuite/ld-elf/package-note.rd | 6 ++ > 11 files changed, 244 insertions(+), 7 deletions(-) > create mode 100644 ld/testsuite/ld-elf/package-note.exp > create mode 100644 ld/testsuite/ld-elf/package-note.rd > >diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h >index c7c0a793b15..65bd1128263 100644 >--- a/bfd/elf-bfd.h >+++ b/bfd/elf-bfd.h >@@ -1918,6 +1918,14 @@ struct output_elf_obj_tdata > asection *sec; > } build_id; > >+ /* FDO_PACKAGING_METADATA note type info. */ >+ struct >+ { >+ bool (*after_write_object_contents) (bfd *); >+ const char *json; >+ asection *sec; >+ } package_metadata; >+ > /* Records the result of `get_program_header_size'. */ > bfd_size_type program_header_size; > >diff --git a/bfd/elf.c b/bfd/elf.c >index c493aa5b172..558c630fd7b 100644 >--- a/bfd/elf.c >+++ b/bfd/elf.c >@@ -6779,8 +6779,10 @@ _bfd_elf_write_object_contents (bfd *abfd) > return false; > > /* This is last since write_shdrs_and_ehdr can touch i_shdrp[0]. */ >- if (t->o->build_id.after_write_object_contents != NULL) >- return (*t->o->build_id.after_write_object_contents) (abfd); >+ if (t->o->build_id.after_write_object_contents != NULL && !(*t->o->build_id.after_write_object_contents) (abfd)) >+ return false; >+ if (t->o->package_metadata.after_write_object_contents != NULL) >+ return (*t->o->package_metadata.after_write_object_contents) (abfd); > > return true; > } >diff --git a/ld/Makefile.am b/ld/Makefile.am >index e53bef13bb8..a3cd7891840 100644 >--- a/ld/Makefile.am >+++ b/ld/Makefile.am >@@ -45,7 +45,7 @@ ELF_CLFAGS=-DELF_LIST_OPTIONS=@elf_list_options@ \ > -DELF_PLT_UNWIND_LIST_OPTIONS=@elf_plt_unwind_list_options@ > WARN_CFLAGS = @WARN_CFLAGS@ > NO_WERROR = @NO_WERROR@ >-AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS) >+AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS) $(JANSSON_CFLAGS) > > # We put the scripts in the directory $(scriptdir)/ldscripts. > # We can't put the scripts in $(datadir) because the SEARCH_DIR >@@ -964,8 +964,8 @@ ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmai > ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \ > ldbuildid.c > ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \ >- $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP) >-ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB) >+ $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP) $(JANSSON_LIBS) >+ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB) $(JANSSON_LIBS) > > # Dependency tracking for the generated emulation files. > EXTRA_ld_new_SOURCES += $(ALL_EMULATION_SOURCES) $(ALL_64_EMULATION_SOURCES) >diff --git a/ld/configure.ac b/ld/configure.ac >index 0b29e810dde..fdfb4d46344 100644 >--- a/ld/configure.ac >+++ b/ld/configure.ac >@@ -287,6 +287,41 @@ fi > AM_CONDITIONAL(ENABLE_LIBCTF, test "${enable_libctf}" = yes) > AC_SUBST(enable_libctf) > >+# If pkg-config and libjansson are available, use them, skip if not. >+# Allow explicitly enabling (error if not found)/disabling (skip even if found). >+AC_ARG_ENABLE([jansson], >+ [AS_HELP_STRING([--enable-jansson], >+ [enable jansson [default=auto]])], >+ [enable_jansson=$enableval], >+ [enable_jansson="auto"]) >+ >+if test "x$enable_jansson" != "xno"; then >+ PKG_PROG_PKG_CONFIG >+ AS_IF([test -n "$PKG_CONFIG"], >+ [ >+ PKG_CHECK_MODULES(JANSSON, [jansson], >+ [ >+ AC_DEFINE(HAVE_JANSSON, 1, [The jansson library is to be used]) >+ AC_SUBST([JANSSON_CFLAGS]) >+ AC_SUBST([JANSSON_LIBS]) >+ ], >+ [ >+ if test "x$enable_jansson" = "xyes"; then >+ AC_MSG_ERROR([Cannot find jansson library]) >+ else >+ AC_MSG_WARN([Cannot find jansson library]) >+ fi >+ ]) >+ ], >+ [ >+ if test "x$enable_jansson" = "xyes"; then >+ AC_MSG_ERROR([Cannot find pkg-config]) >+ else >+ AC_MSG_WARN([Cannot find pkg-config]) >+ fi >+ ]) >+fi >+ > AM_BINUTILS_WARNINGS > > AM_LC_MESSAGES >diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em >index c027559908c..c52484f35d3 100644 >--- a/ld/emultempl/elf.em >+++ b/ld/emultempl/elf.em >@@ -572,6 +572,7 @@ enum elf_options > OPTION_EXCLUDE_LIBS, > OPTION_HASH_STYLE, > OPTION_BUILD_ID, >+ OPTION_PACKAGE_METADATA, > OPTION_AUDIT, > OPTION_COMPRESS_DEBUG > }; >@@ -602,6 +603,7 @@ EOF > fi > fragment < {"build-id", optional_argument, NULL, OPTION_BUILD_ID}, >+ {"package-metadata", optional_argument, NULL, OPTION_PACKAGE_METADATA}, > {"compress-debug-sections", required_argument, NULL, OPTION_COMPRESS_DEBUG}, > EOF > if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then >@@ -650,6 +652,13 @@ gld${EMULATION_NAME}_handle_option (int optc) > ldelf_emit_note_gnu_build_id = xstrdup (optarg); > break; > >+ case OPTION_PACKAGE_METADATA: >+ free ((char *) ldelf_emit_note_fdo_package_metadata); >+ ldelf_emit_note_fdo_package_metadata = NULL; >+ if (optarg != NULL && strlen(optarg) > 0) >+ ldelf_emit_note_fdo_package_metadata = xstrdup (optarg); >+ break; >+ > case OPTION_COMPRESS_DEBUG: > if (strcasecmp (optarg, "none") == 0) > link_info.compress_debug = COMPRESS_DEBUG_NONE; >diff --git a/ld/ld.texi b/ld/ld.texi >index 8cad8478140..8406560397c 100644 >--- a/ld/ld.texi >+++ b/ld/ld.texi >@@ -2935,6 +2935,13 @@ string identifying the original linked file does not change. > > Passing @code{none} for @var{style} disables the setting from any > @code{--build-id} options earlier on the command line. >+ >+@kindex --package-metadata=@var{JSON} >+@item --package-metadata=@var{JSON} >+Request the creation of a @code{.note.package} ELF note section. The >+contents of the note are in JSON format, as per the package metadata >+specification. For more information see: >+https://systemd.io/COREDUMP_PACKAGE_METADATA/ > @end table > > @c man end >diff --git a/ld/ldelf.c b/ld/ldelf.c >index 4094640b3f7..161fb476121 100644 >--- a/ld/ldelf.c >+++ b/ld/ldelf.c >@@ -39,6 +39,9 @@ > #include > #endif > #include "ldelf.h" >+#ifdef HAVE_JANSSON >+#include >+#endif > > struct dt_needed > { >@@ -49,6 +52,9 @@ struct dt_needed > /* Style of .note.gnu.build-id section. */ > const char *ldelf_emit_note_gnu_build_id; > >+/* Content of .note.package section. */ >+const char *ldelf_emit_note_fdo_package_metadata; >+ > /* These variables are required to pass information back and forth > between after_open and check_needed and stat_needed and vercheck. */ > >@@ -1249,7 +1255,8 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd, > } > } > >- if (ldelf_emit_note_gnu_build_id != NULL) >+ if (ldelf_emit_note_gnu_build_id != NULL || >+ ldelf_emit_note_fdo_package_metadata != NULL) > { > /* Find an ELF input. */ > for (abfd = link_info.input_bfds; >@@ -1262,11 +1269,18 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd, > /* PR 10555: If there are no ELF input files do not try to > create a .note.gnu-build-id section. */ > if (abfd == NULL >- || !ldelf_setup_build_id (abfd)) >+ || (ldelf_emit_note_gnu_build_id != NULL && !ldelf_setup_build_id (abfd))) > { > free ((char *) ldelf_emit_note_gnu_build_id); > ldelf_emit_note_gnu_build_id = NULL; > } >+ >+ if (abfd == NULL >+ || (ldelf_emit_note_fdo_package_metadata != NULL && !ldelf_setup_package_metadata (abfd))) >+ { >+ free ((char *) ldelf_emit_note_fdo_package_metadata); >+ ldelf_emit_note_fdo_package_metadata = NULL; >+ } > } > > get_elf_backend_data (link_info.output_bfd)->setup_gnu_properties (&link_info); >@@ -1501,6 +1515,109 @@ ldelf_setup_build_id (bfd *ibfd) > return false; > } > >+static bool >+write_package_metadata (bfd *abfd) >+{ >+ struct elf_obj_tdata *t = elf_tdata (abfd); >+ const char *json; >+ asection *asec; >+ Elf_Internal_Shdr *i_shdr; >+ unsigned char *contents, *json_bits; >+ bfd_size_type size; >+ file_ptr position; >+ Elf_External_Note *e_note; >+ >+ json = t->o->package_metadata.json; >+ asec = t->o->package_metadata.sec; >+ if (bfd_is_abs_section (asec->output_section)) >+ { >+ einfo (_("%P: warning: .note.package section discarded," >+ " --package-metadata ignored\n")); >+ return true; >+ } >+ i_shdr = &elf_section_data (asec->output_section)->this_hdr; >+ >+ if (i_shdr->contents == NULL) >+ { >+ if (asec->contents == NULL) >+ asec->contents = (unsigned char *) xmalloc (asec->size); >+ contents = asec->contents; >+ } >+ else >+ contents = i_shdr->contents + asec->output_offset; >+ >+ e_note = (Elf_External_Note *) contents; >+ size = offsetof (Elf_External_Note, name[sizeof "FDO"]); >+ size = (size + 3) & -(bfd_size_type) 4; >+ json_bits = contents + size; >+ size = asec->size - size; >+ >+ /* Clear the package metadata field. */ >+ memset (json_bits, 0, size); >+ >+ bfd_h_put_32 (abfd, sizeof "FDO", &e_note->namesz); >+ bfd_h_put_32 (abfd, size, &e_note->descsz); >+ bfd_h_put_32 (abfd, FDO_PACKAGING_METADATA, &e_note->type); >+ memcpy (e_note->name, "FDO", sizeof "FDO"); >+ memcpy (json_bits, json, size); >+ >+ position = i_shdr->sh_offset + asec->output_offset; >+ size = asec->size; >+ return (bfd_seek (abfd, position, SEEK_SET) == 0 >+ && bfd_bwrite (contents, size, abfd) == size); >+} >+ >+/* Make .note.package section. >+ https://systemd.io/COREDUMP_PACKAGE_METADATA/ */ >+ >+bool >+ldelf_setup_package_metadata (bfd *ibfd) >+{ >+ asection *s; >+ bfd_size_type size; >+ flagword flags; >+ >+ if (!ldelf_emit_note_fdo_package_metadata || strlen(ldelf_emit_note_fdo_package_metadata) == 0) >+ return false; >+ >+#ifdef HAVE_JANSSON >+ json_error_t json_error; >+ json_t *json = json_loads(ldelf_emit_note_fdo_package_metadata, 0, &json_error); >+ if (!json) >+ { >+ einfo (_("%P: warning: --package-metadata=%s does not contain valid " >+ "JSON, ignoring: %s\n"), ldelf_emit_note_fdo_package_metadata, >+ json_error.text); >+ return false; >+ } >+ else >+ json_decref(json); >+#endif >+ >+ size = offsetof (Elf_External_Note, name[sizeof "FDO"]); >+ size += strlen(ldelf_emit_note_fdo_package_metadata) + 1; >+ size = (size + 3) & -(bfd_size_type) 4; >+ >+ flags = (SEC_ALLOC | SEC_LOAD | SEC_IN_MEMORY >+ | SEC_LINKER_CREATED | SEC_READONLY | SEC_DATA); >+ s = bfd_make_section_anyway_with_flags (ibfd, ".note.package", >+ flags); >+ if (s != NULL && bfd_set_section_alignment (s, 2)) >+ { >+ struct elf_obj_tdata *t = elf_tdata (link_info.output_bfd); >+ t->o->package_metadata.after_write_object_contents = &write_package_metadata; >+ t->o->package_metadata.json = ldelf_emit_note_fdo_package_metadata; >+ t->o->package_metadata.sec = s; >+ elf_section_type (s) = SHT_NOTE; >+ s->size = size; >+ return true; >+ } >+ >+ einfo (_("%P: warning: cannot create .note.package section," >+ " --package-metadata ignored\n")); >+ return false; >+} >+ > /* Look through an expression for an assignment statement. */ > > static void >diff --git a/ld/ldelf.h b/ld/ldelf.h >index efa8b45851c..a3ded3dd04c 100644 >--- a/ld/ldelf.h >+++ b/ld/ldelf.h >@@ -19,6 +19,7 @@ > MA 02110-1301, USA. */ > > extern const char *ldelf_emit_note_gnu_build_id; >+extern const char *ldelf_emit_note_fdo_package_metadata; > > extern void ldelf_after_parse (void); > extern bool ldelf_load_symbols (lang_input_statement_type *); >@@ -26,6 +27,7 @@ extern void ldelf_before_plugin_all_symbols_read (int, int, int, int, > int, const char *); > extern void ldelf_after_open (int, int, int, int, int, const char *); > extern bool ldelf_setup_build_id (bfd *); >+extern bool ldelf_setup_package_metadata (bfd *); > extern void ldelf_append_to_separated_string (char **, char *); > extern void ldelf_before_allocation (char *, char *, const char *); > extern bool ldelf_open_dynamic_archive >diff --git a/ld/lexsup.c b/ld/lexsup.c >index 78190472907..3aeba07c3a7 100644 >--- a/ld/lexsup.c >+++ b/ld/lexsup.c >@@ -2144,6 +2144,8 @@ elf_static_list_options (FILE *file) > fprintf (file, _("\ > --build-id[=STYLE] Generate build ID note\n")); > fprintf (file, _("\ >+ --package-metadata[=JSON] Generate package metadata note\n")); >+ fprintf (file, _("\ > --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi]\n\ > Compress DWARF debug sections using zlib\n")); > #ifdef DEFAULT_FLAG_COMPRESS_DEBUG >diff --git a/ld/testsuite/ld-elf/package-note.exp b/ld/testsuite/ld-elf/package-note.exp >new file mode 100644 >index 00000000000..51a790b25de >--- /dev/null >+++ b/ld/testsuite/ld-elf/package-note.exp >@@ -0,0 +1,49 @@ >+# Expect script for --package-note tests. >+# Copyright (C) 2022 Free Software Foundation, Inc. >+# >+# This file is part of the GNU Binutils. >+# >+# This program is free software; you can redistribute it and/or modify >+# it under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# This program is distributed in the hope that it will be useful, >+# but WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with this program; if not, write to the Free Software >+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, >+# MA 02110-1301, USA. >+# >+ >+# Exclude non-ELF targets. >+ >+if ![is_elf_format] { >+ return >+} >+ >+if { [istarget frv-*-*] || [istarget lm32-*-*] } { >+ return >+} >+ >+if { !([istarget *-*-linux*] >+ || [istarget arm*-*-uclinuxfdpiceabi] >+ || [istarget *-*-nacl*] >+ || [istarget *-*-gnu*]) } then { >+ return >+} >+ >+run_ld_link_tests [list \ >+ [list \ >+ "package-note.o" \ >+ "--package-metadata='{\"foo\":\"bar\"}'" \ >+ "" \ >+ "" \ >+ {start.s} \ >+ {{readelf {--notes} package-note.rd}} \ >+ "package-note.o" \ >+ ] \ >+] >diff --git a/ld/testsuite/ld-elf/package-note.rd b/ld/testsuite/ld-elf/package-note.rd >new file mode 100644 >index 00000000000..c65dd21cdac >--- /dev/null >+++ b/ld/testsuite/ld-elf/package-note.rd >@@ -0,0 +1,6 @@ >+#... >+Displaying notes found in: \.note\.package >+ Owner Data size Description >+ FDO 0x00000010 FDO_PACKAGING_METADATA >+ Packaging Metadata: {"foo":"bar"} >+#pass >-- >2.35.1 >