[PATCH] gold: add --package-metadata

[PATCH] gold: add --package-metadata

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

Following the same format as the implementation in ld:
9e2bb0cb5e74aed4158f08495534922d7108f928

Generate a .note.package FDO package metadata ELF note, following
the spec: https://systemd.io/ELF_PACKAGE_METADATA/

If the jansson library is available at build time (and it is explicitly
enabled), link ld to it, and use it to validate that the input is
correct JSON, to avoid writing garbage to the file. The
configure option --enable-jansson has to be used to explicitly enable
it (error out when not found). This allows bootstrappers (or others who
are not interested) to seamlessly skip it without issues.
---
Re-sending after rebase, no changes. The same functionality was added to ld:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=9e2bb0cb5e74aed4158f08495534922d7108f928

 elfcpp/elfcpp.h                        |  4 ++-
 gold/Makefile.am                       |  8 ++---
 gold/configure.ac                      | 26 ++++++++++++++
 gold/layout.cc                         | 48 ++++++++++++++++++++++++++
 gold/layout.h                          |  6 ++++
 gold/options.h                         |  4 +++
 gold/testsuite/Makefile.am             | 15 +++++---
 gold/testsuite/package_metadata_main.c |  5 +++
 8 files changed, 107 insertions(+), 9 deletions(-)
 create mode 100644 gold/testsuite/package_metadata_main.c

diff --git a/elfcpp/elfcpp.h b/elfcpp/elfcpp.h
index 4b0aff0da7f..3ca2d614947 100644
--- a/elfcpp/elfcpp.h
+++ b/elfcpp/elfcpp.h
@@ -999,7 +999,9 @@ enum
   // string.
   NT_GNU_GOLD_VERSION = 4,
   // Program property note, as described in "Linux Extensions to the gABI".
-  NT_GNU_PROPERTY_TYPE_0 = 5
+  NT_GNU_PROPERTY_TYPE_0 = 5,
+  // FDO .note.package notes as defined on https://systemd.io/ELF_PACKAGE_METADATA/
+  FDO_PACKAGING_METADATA = 0xcafe1a7e
 };
 
 // The OS values which may appear in word 0 of a NT_GNU_ABI_TAG note.
diff --git a/gold/Makefile.am b/gold/Makefile.am
index 2e406716f29..934e3669977 100644
--- a/gold/Makefile.am
+++ b/gold/Makefile.am
@@ -35,7 +35,7 @@ THREADFLAGS = @PTHREAD_CFLAGS@
 THREADLIBS = @PTHREAD_LIBS@
 
 AM_CFLAGS = $(WARN_CFLAGS) $(LFS_CFLAGS) $(RANDOM_SEED_CFLAGS) $(ZLIBINC) $(THREADFLAGS)
-AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) $(RANDOM_SEED_CFLAGS) $(ZLIBINC) $(THREADFLAGS)
+AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) $(RANDOM_SEED_CFLAGS) $(ZLIBINC) $(THREADFLAGS) $(JANSSON_CFLAGS)
 AM_LDFLAGS = $(THREADFLAGS)
 
 AM_CPPFLAGS = \
@@ -187,7 +187,7 @@ libgold_a_LIBADD = $(LIBOBJS)
 sources_var = main.cc
 deps_var = $(TARGETOBJS) libgold.a $(LIBIBERTY) $(LIBINTL_DEP)
 ldadd_var = $(TARGETOBJS) libgold.a $(LIBIBERTY) $(GOLD_LDADD) $(LIBINTL) \
-	 $(THREADLIBS) $(LIBDL) $(ZLIB)
+	 $(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 ldflags_var = $(GOLD_LDFLAGS)
 
 ld_new_SOURCES = $(sources_var)
@@ -201,12 +201,12 @@ incremental_dump_SOURCES = incremental-dump.cc
 incremental_dump_DEPENDENCIES = $(TARGETOBJS) libgold.a $(LIBIBERTY) \
 	$(LIBINTL_DEP)
 incremental_dump_LDADD = $(TARGETOBJS) libgold.a $(LIBIBERTY) $(LIBINTL) \
-	 $(THREADLIBS) $(LIBDL) $(ZLIB)
+	 $(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 dwp_SOURCES = dwp.cc
 dwp_DEPENDENCIES = libgold.a $(LIBIBERTY) $(LIBINTL_DEP)
 dwp_LDADD = libgold.a $(LIBIBERTY) $(GOLD_LDADD) $(LIBINTL) $(THREADLIBS) \
-	$(LIBDL) $(ZLIB)
+	$(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 dwp_LDFLAGS = $(GOLD_LDFLAGS)
 
 CONFIG_STATUS_DEPENDENCIES = $(srcdir)/../bfd/development.sh
diff --git a/gold/configure.ac b/gold/configure.ac
index 4f432809b37..25fae6b998b 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -591,6 +591,32 @@ if test "$threads" = "yes"; then
 fi
 AM_CONDITIONAL(THREADS, test "$threads" = "yes")
 
+# Used to validate --package-metadata= input. Disabled by default.
+AC_ARG_ENABLE([jansson],
+  [AS_HELP_STRING([--enable-jansson],
+    [enable jansson [default=no]])],
+  [enable_jansson=$enableval],
+  [enable_jansson="no"])
+
+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])
+	],
+	[
+	  AC_MSG_ERROR([Cannot find jansson library])
+	])
+    ],
+    [
+      AC_MSG_ERROR([Cannot find pkg-config])
+    ])
+fi
+
 dnl We have to check these in C, not C++, because autoconf generates
 dnl tests which have no type information, and current glibc provides
 dnl multiple declarations of functions like basename when compiling
diff --git a/gold/layout.cc b/gold/layout.cc
index 3efe8d98ae9..ece14a07088 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -38,6 +38,9 @@
 #include <windows.h>
 #include <rpcdce.h>
 #endif
+#ifdef HAVE_JANSSON
+#include <jansson.h>
+#endif
 
 #include "parameters.h"
 #include "options.h"
@@ -2437,6 +2440,7 @@ Layout::create_notes()
   this->create_gold_note();
   this->create_stack_segment();
   this->create_build_id();
+  this->create_package_metadata();
 }
 
 // Create the dynamic sections which are needed before we read the
@@ -3534,6 +3538,50 @@ Layout::create_build_id()
     }
 }
 
+// If --package-metadata was used, set up the package metadata note.
+// https://systemd.io/ELF_PACKAGE_METADATA/
+
+void
+Layout::create_package_metadata()
+{
+  if (!parameters->options().user_set_package_metadata())
+    return;
+
+  const char* desc = parameters->options().package_metadata();
+  if (strcmp(desc, "") == 0)
+    return;
+
+#ifdef HAVE_JANSSON
+  json_error_t json_error;
+  json_t *json = json_loads (desc, 0, &json_error);
+  if (!json)
+    gold_fatal(_("error: --package-metadata=%s does not contain valid "
+	     "JSON: %s\n"),
+	   desc, json_error.text);
+  else
+    json_decref (json);
+#endif
+
+  // Create the note.
+  size_t trailing_padding;
+  // Ensure the trailing NULL byte is always included, as per specification.
+  size_t descsz = strlen(desc) + 1;
+  Output_section* os = this->create_note("FDO", elfcpp::FDO_PACKAGING_METADATA,
+					 ".note.package", descsz, true,
+					 &trailing_padding);
+  if (os == NULL)
+    return;
+
+  Output_section_data* posd = new Output_data_const(desc, descsz, 4);
+  os->add_output_section_data(posd);
+
+  if (trailing_padding != 0)
+  {
+    posd = new Output_data_zero_fill(trailing_padding, 0);
+    os->add_output_section_data(posd);
+  }
+}
+
 // If we have both .stabXX and .stabXXstr sections, then the sh_link
 // field of the former should point to the latter.  I'm not sure who
 // started this, but the GNU linker does it, and some tools depend
diff --git a/gold/layout.h b/gold/layout.h
index 98c03b93f5f..ee4f78c3c78 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -1107,6 +1107,10 @@ class Layout
   void
   create_build_id();
 
+  // Create a package metadata note if needed.
+  void
+  create_package_metadata();
+
   // Link .stab and .stabstr sections.
   void
   link_stabs_sections();
@@ -1453,6 +1457,8 @@ class Layout
   Gdb_index* gdb_index_data_;
   // The space for the build ID checksum if there is one.
   Output_section_data* build_id_note_;
+  // The space for the package metadata JSON if there is one.
+  Output_section_data* package_metadata_note_;
   // The output section containing dwarf abbreviations
   Output_reduced_debug_abbrev_section* debug_abbrev_;
   // The output section containing the dwarf debug info tree
diff --git a/gold/options.h b/gold/options.h
index 9509a445e8e..17236eb9cb9 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1102,6 +1102,10 @@ class General_options
   DEFINE_bool(p, options::ONE_DASH, 'p', false,
 	      N_("Ignored for ARM compatibility"), NULL);
 
+  DEFINE_optional_string(package_metadata, options::TWO_DASHES, '\0', NULL,
+			 N_("Generate package metadata note"),
+			 N_("[=JSON]"));
+
   DEFINE_bool(pie, options::ONE_DASH, '\0', false,
 	      N_("Create a position independent executable"),
 	      N_("Do not create a position independent executable"));
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 38e54818f48..b15000ee7f3 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -149,25 +149,25 @@ check_PROGRAMS += object_unittest
 object_unittest_SOURCES = object_unittest.cc
 object_unittest_LDFLAGS = $(THREADFLAGS)
 object_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 check_PROGRAMS += binary_unittest
 binary_unittest_SOURCES = binary_unittest.cc
 binary_unittest_LDFLAGS = $(THREADFLAGS)
 binary_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 check_PROGRAMS += leb128_unittest
 leb128_unittest_SOURCES = leb128_unittest.cc
 leb128_unittest_LDFLAGS = $(THREADFLAGS)
 leb128_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 check_PROGRAMS += overflow_unittest
 overflow_unittest_SOURCES = overflow_unittest.cc
 overflow_unittest_LDFLAGS = $(THREADFLAGS)
 overflow_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 overflow_unittest.o: overflow_unittest.cc
 	$(CXXCOMPILE) -O3 -c -o $@ $<
 
@@ -4435,3 +4435,10 @@ retain_2.o: retain_2.s
 	$(TEST_AS) -o $@ $<
 
 endif DEFAULT_TARGET_X86_64
+
+check_PROGRAMS += package_metadata_test
+package_metadata_test.o: package_metadata_main.c
+	$(COMPILE) -c -o $@ $<
+package_metadata_test$(EXEEXT): package_metadata_test.o gcctestdir/ld
+	$(CXXLINK) package_metadata_test.o -Wl,--package-metadata='{"foo":"bar"}'
+	$(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
diff --git a/gold/testsuite/package_metadata_main.c b/gold/testsuite/package_metadata_main.c
new file mode 100644
index 00000000000..77bc677e8eb
--- /dev/null
+++ b/gold/testsuite/package_metadata_main.c
@@ -0,0 +1,5 @@
+int
+main(void)
+{
+  return 0;
+}
-- 
2.34.1

Re: [PATCH] gold: add --package-metadata

[Cary Coutant]

> Following the same format as the implementation in ld:
> 9e2bb0cb5e74aed4158f08495534922d7108f928
>
> Generate a .note.package FDO package metadata ELF note, following
> the spec: https://systemd.io/ELF_PACKAGE_METADATA/
>
> If the jansson library is available at build time (and it is explicitly
> enabled), link ld to it, and use it to validate that the input is
> correct JSON, to avoid writing garbage to the file. The
> configure option --enable-jansson has to be used to explicitly enable
> it (error out when not found). This allows bootstrappers (or others who
> are not interested) to seamlessly skip it without issues.

I reviewed the earlier discussion and I had some of the same questions
and concerns as others did there, so they've all been answered. I'd
have preferred an option syntax that would let you build up the
metadata info one key/value pair at a time. Given that it has already
been accepted for ld, however, I'll OK it for gold as well, with the
following fixes...

+  json_t *json = json_loads (desc, 0, &json_error);

C++ coding conventions here: no space before the paren in a function call.

+  if (!json)
+    gold_fatal(_("error: --package-metadata=%s does not contain valid "
+      "JSON: %s\n"),
+    desc, json_error.text);
+  else
+    json_decref (json);

Put the shorter, non-error path first (i.e., make it "if
(json)...else..."), and use { } for the longer error path (even though
it's only one statement, I think braces around a multi-line statement
help readability). Also, no space before the paren.

+  if (trailing_padding != 0)
+  {
+    posd = new Output_data_zero_fill(trailing_padding, 0);
+    os->add_output_section_data(posd);
+  }

The braces and what's inside them need an extra two spaces of indent.

-cary

[PATCH v2] gold: add --package-metadata

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

Following the same format as the implementation in ld:
9e2bb0cb5e74aed4158f08495534922d7108f928

Generate a .note.package FDO package metadata ELF note, following
the spec: https://systemd.io/ELF_PACKAGE_METADATA/

If the jansson library is available at build time (and it is explicitly
enabled), link ld to it, and use it to validate that the input is
correct JSON, to avoid writing garbage to the file. The
configure option --enable-jansson has to be used to explicitly enable
it (error out when not found). This allows bootstrappers (or others who
are not interested) to seamlessly skip it without issues.
---
v2: apply fixes from review

 elfcpp/elfcpp.h                        |  4 ++-
 gold/Makefile.am                       |  8 ++---
 gold/configure.ac                      | 26 ++++++++++++++
 gold/layout.cc                         | 50 ++++++++++++++++++++++++++
 gold/layout.h                          |  6 ++++
 gold/options.h                         |  4 +++
 gold/testsuite/Makefile.am             | 15 +++++---
 gold/testsuite/package_metadata_main.c |  5 +++
 8 files changed, 109 insertions(+), 9 deletions(-)
 create mode 100644 gold/testsuite/package_metadata_main.c

diff --git a/elfcpp/elfcpp.h b/elfcpp/elfcpp.h
index 4b0aff0da7f..3ca2d614947 100644
--- a/elfcpp/elfcpp.h
+++ b/elfcpp/elfcpp.h
@@ -999,7 +999,9 @@ enum
   // string.
   NT_GNU_GOLD_VERSION = 4,
   // Program property note, as described in "Linux Extensions to the gABI".
-  NT_GNU_PROPERTY_TYPE_0 = 5
+  NT_GNU_PROPERTY_TYPE_0 = 5,
+  // FDO .note.package notes as defined on https://systemd.io/ELF_PACKAGE_METADATA/
+  FDO_PACKAGING_METADATA = 0xcafe1a7e
 };
 
 // The OS values which may appear in word 0 of a NT_GNU_ABI_TAG note.
diff --git a/gold/Makefile.am b/gold/Makefile.am
index 2e406716f29..934e3669977 100644
--- a/gold/Makefile.am
+++ b/gold/Makefile.am
@@ -35,7 +35,7 @@ THREADFLAGS = @PTHREAD_CFLAGS@
 THREADLIBS = @PTHREAD_LIBS@
 
 AM_CFLAGS = $(WARN_CFLAGS) $(LFS_CFLAGS) $(RANDOM_SEED_CFLAGS) $(ZLIBINC) $(THREADFLAGS)
-AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) $(RANDOM_SEED_CFLAGS) $(ZLIBINC) $(THREADFLAGS)
+AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) $(RANDOM_SEED_CFLAGS) $(ZLIBINC) $(THREADFLAGS) $(JANSSON_CFLAGS)
 AM_LDFLAGS = $(THREADFLAGS)
 
 AM_CPPFLAGS = \
@@ -187,7 +187,7 @@ libgold_a_LIBADD = $(LIBOBJS)
 sources_var = main.cc
 deps_var = $(TARGETOBJS) libgold.a $(LIBIBERTY) $(LIBINTL_DEP)
 ldadd_var = $(TARGETOBJS) libgold.a $(LIBIBERTY) $(GOLD_LDADD) $(LIBINTL) \
-	 $(THREADLIBS) $(LIBDL) $(ZLIB)
+	 $(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 ldflags_var = $(GOLD_LDFLAGS)
 
 ld_new_SOURCES = $(sources_var)
@@ -201,12 +201,12 @@ incremental_dump_SOURCES = incremental-dump.cc
 incremental_dump_DEPENDENCIES = $(TARGETOBJS) libgold.a $(LIBIBERTY) \
 	$(LIBINTL_DEP)
 incremental_dump_LDADD = $(TARGETOBJS) libgold.a $(LIBIBERTY) $(LIBINTL) \
-	 $(THREADLIBS) $(LIBDL) $(ZLIB)
+	 $(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 dwp_SOURCES = dwp.cc
 dwp_DEPENDENCIES = libgold.a $(LIBIBERTY) $(LIBINTL_DEP)
 dwp_LDADD = libgold.a $(LIBIBERTY) $(GOLD_LDADD) $(LIBINTL) $(THREADLIBS) \
-	$(LIBDL) $(ZLIB)
+	$(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 dwp_LDFLAGS = $(GOLD_LDFLAGS)
 
 CONFIG_STATUS_DEPENDENCIES = $(srcdir)/../bfd/development.sh
diff --git a/gold/configure.ac b/gold/configure.ac
index 4f432809b37..25fae6b998b 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -591,6 +591,32 @@ if test "$threads" = "yes"; then
 fi
 AM_CONDITIONAL(THREADS, test "$threads" = "yes")
 
+# Used to validate --package-metadata= input. Disabled by default.
+AC_ARG_ENABLE([jansson],
+  [AS_HELP_STRING([--enable-jansson],
+    [enable jansson [default=no]])],
+  [enable_jansson=$enableval],
+  [enable_jansson="no"])
+
+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])
+	],
+	[
+	  AC_MSG_ERROR([Cannot find jansson library])
+	])
+    ],
+    [
+      AC_MSG_ERROR([Cannot find pkg-config])
+    ])
+fi
+
 dnl We have to check these in C, not C++, because autoconf generates
 dnl tests which have no type information, and current glibc provides
 dnl multiple declarations of functions like basename when compiling
diff --git a/gold/layout.cc b/gold/layout.cc
index 3efe8d98ae9..184caa2c8ad 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -38,6 +38,9 @@
 #include <windows.h>
 #include <rpcdce.h>
 #endif
+#ifdef HAVE_JANSSON
+#include <jansson.h>
+#endif
 
 #include "parameters.h"
 #include "options.h"
@@ -2437,6 +2440,7 @@ Layout::create_notes()
   this->create_gold_note();
   this->create_stack_segment();
   this->create_build_id();
+  this->create_package_metadata();
 }
 
 // Create the dynamic sections which are needed before we read the
@@ -3534,6 +3538,52 @@ Layout::create_build_id()
     }
 }
 
+// If --package-metadata was used, set up the package metadata note.
+// https://systemd.io/ELF_PACKAGE_METADATA/
+
+void
+Layout::create_package_metadata()
+{
+  if (!parameters->options().user_set_package_metadata())
+    return;
+
+  const char* desc = parameters->options().package_metadata();
+  if (strcmp(desc, "") == 0)
+    return;
+
+#ifdef HAVE_JANSSON
+  json_error_t json_error;
+  json_t *json = json_loads(desc, 0, &json_error);
+  if (json)
+    json_decref(json);
+  else
+    {
+      gold_fatal(_("error: --package-metadata=%s does not contain valid "
+		   "JSON: %s\n"),
+		 desc, json_error.text);
+    }
+#endif
+
+  // Create the note.
+  size_t trailing_padding;
+  // Ensure the trailing NULL byte is always included, as per specification.
+  size_t descsz = strlen(desc) + 1;
+  Output_section* os = this->create_note("FDO", elfcpp::FDO_PACKAGING_METADATA,
+					 ".note.package", descsz, true,
+					 &trailing_padding);
+  if (os == NULL)
+    return;
+
+  Output_section_data* posd = new Output_data_const(desc, descsz, 4);
+  os->add_output_section_data(posd);
+
+  if (trailing_padding != 0)
+    {
+      posd = new Output_data_zero_fill(trailing_padding, 0);
+      os->add_output_section_data(posd);
+    }
+}
+
 // If we have both .stabXX and .stabXXstr sections, then the sh_link
 // field of the former should point to the latter.  I'm not sure who
 // started this, but the GNU linker does it, and some tools depend
diff --git a/gold/layout.h b/gold/layout.h
index 98c03b93f5f..ee4f78c3c78 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -1107,6 +1107,10 @@ class Layout
   void
   create_build_id();
 
+  // Create a package metadata note if needed.
+  void
+  create_package_metadata();
+
   // Link .stab and .stabstr sections.
   void
   link_stabs_sections();
@@ -1453,6 +1457,8 @@ class Layout
   Gdb_index* gdb_index_data_;
   // The space for the build ID checksum if there is one.
   Output_section_data* build_id_note_;
+  // The space for the package metadata JSON if there is one.
+  Output_section_data* package_metadata_note_;
   // The output section containing dwarf abbreviations
   Output_reduced_debug_abbrev_section* debug_abbrev_;
   // The output section containing the dwarf debug info tree
diff --git a/gold/options.h b/gold/options.h
index 9509a445e8e..17236eb9cb9 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1102,6 +1102,10 @@ class General_options
   DEFINE_bool(p, options::ONE_DASH, 'p', false,
 	      N_("Ignored for ARM compatibility"), NULL);
 
+  DEFINE_optional_string(package_metadata, options::TWO_DASHES, '\0', NULL,
+			 N_("Generate package metadata note"),
+			 N_("[=JSON]"));
+
   DEFINE_bool(pie, options::ONE_DASH, '\0', false,
 	      N_("Create a position independent executable"),
 	      N_("Do not create a position independent executable"));
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 38e54818f48..b15000ee7f3 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -149,25 +149,25 @@ check_PROGRAMS += object_unittest
 object_unittest_SOURCES = object_unittest.cc
 object_unittest_LDFLAGS = $(THREADFLAGS)
 object_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 check_PROGRAMS += binary_unittest
 binary_unittest_SOURCES = binary_unittest.cc
 binary_unittest_LDFLAGS = $(THREADFLAGS)
 binary_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 check_PROGRAMS += leb128_unittest
 leb128_unittest_SOURCES = leb128_unittest.cc
 leb128_unittest_LDFLAGS = $(THREADFLAGS)
 leb128_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 
 check_PROGRAMS += overflow_unittest
 overflow_unittest_SOURCES = overflow_unittest.cc
 overflow_unittest_LDFLAGS = $(THREADFLAGS)
 overflow_unittest_LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \
-	$(THREADLIBS) $(LIBDL) $(ZLIB)
+	$(THREADLIBS) $(LIBDL) $(ZLIB) $(JANSSON_LIBS)
 overflow_unittest.o: overflow_unittest.cc
 	$(CXXCOMPILE) -O3 -c -o $@ $<
 
@@ -4435,3 +4435,10 @@ retain_2.o: retain_2.s
 	$(TEST_AS) -o $@ $<
 
 endif DEFAULT_TARGET_X86_64
+
+check_PROGRAMS += package_metadata_test
+package_metadata_test.o: package_metadata_main.c
+	$(COMPILE) -c -o $@ $<
+package_metadata_test$(EXEEXT): package_metadata_test.o gcctestdir/ld
+	$(CXXLINK) package_metadata_test.o -Wl,--package-metadata='{"foo":"bar"}'
+	$(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
diff --git a/gold/testsuite/package_metadata_main.c b/gold/testsuite/package_metadata_main.c
new file mode 100644
index 00000000000..77bc677e8eb
--- /dev/null
+++ b/gold/testsuite/package_metadata_main.c
@@ -0,0 +1,5 @@
+int
+main(void)
+{
+  return 0;
+}
-- 
2.35.1

Re: [PATCH] gold: add --package-metadata

[Luca Boccassi]

On Fri, 29 Jul 2022 at 20:41, Cary Coutant <ccoutant@gmail.com> wrote:
>
> > Following the same format as the implementation in ld:
> > 9e2bb0cb5e74aed4158f08495534922d7108f928
> >
> > Generate a .note.package FDO package metadata ELF note, following
> > the spec: https://systemd.io/ELF_PACKAGE_METADATA/
> >
> > If the jansson library is available at build time (and it is explicitly
> > enabled), link ld to it, and use it to validate that the input is
> > correct JSON, to avoid writing garbage to the file. The
> > configure option --enable-jansson has to be used to explicitly enable
> > it (error out when not found). This allows bootstrappers (or others who
> > are not interested) to seamlessly skip it without issues.
>
> I reviewed the earlier discussion and I had some of the same questions
> and concerns as others did there, so they've all been answered. I'd
> have preferred an option syntax that would let you build up the
> metadata info one key/value pair at a time. Given that it has already
> been accepted for ld, however, I'll OK it for gold as well, with the
> following fixes...
>
> +  json_t *json = json_loads (desc, 0, &json_error);
>
> C++ coding conventions here: no space before the paren in a function call.
>
> +  if (!json)
> +    gold_fatal(_("error: --package-metadata=%s does not contain valid "
> +      "JSON: %s\n"),
> +    desc, json_error.text);
> +  else
> +    json_decref (json);
>
> Put the shorter, non-error path first (i.e., make it "if
> (json)...else..."), and use { } for the longer error path (even though
> it's only one statement, I think braces around a multi-line statement
> help readability). Also, no space before the paren.
>
> +  if (trailing_padding != 0)
> +  {
> +    posd = new Output_data_zero_fill(trailing_padding, 0);
> +    os->add_output_section_data(posd);
> +  }
>
> The braces and what's inside them need an extra two spaces of indent.
>
> -cary

Thank you for the review, sent v2 with the requested changes.

Kind regards,
Luca Boccassi

Re: [PATCH v2] gold: add --package-metadata

[Luca Boccassi]

On Fri, 29 Jul 2022 at 23:29, <luca.boccassi@gmail.com> wrote:
>
> From: Luca Boccassi <bluca@debian.org>
>
> Following the same format as the implementation in ld:
> 9e2bb0cb5e74aed4158f08495534922d7108f928
>
> Generate a .note.package FDO package metadata ELF note, following
> the spec: https://systemd.io/ELF_PACKAGE_METADATA/
>
> If the jansson library is available at build time (and it is explicitly
> enabled), link ld to it, and use it to validate that the input is
> correct JSON, to avoid writing garbage to the file. The
> configure option --enable-jansson has to be used to explicitly enable
> it (error out when not found). This allows bootstrappers (or others who
> are not interested) to seamlessly skip it without issues.
> ---
> v2: apply fixes from review

Hello Cary,

Anything else you'd like me to change here? Thanks!

Kind regards,
Luca Boccassi

Re: [PATCH v2] gold: add --package-metadata

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

On Tue, 2022-08-02 at 11:50 +0100, Luca Boccassi wrote:
> On Fri, 29 Jul 2022 at 23:29, <luca.boccassi@gmail.com> wrote:
> > 
> > From: Luca Boccassi <bluca@debian.org>
> > 
> > Following the same format as the implementation in ld:
> > 9e2bb0cb5e74aed4158f08495534922d7108f928
> > 
> > Generate a .note.package FDO package metadata ELF note, following
> > the spec: https://systemd.io/ELF_PACKAGE_METADATA/
> > 
> > If the jansson library is available at build time (and it is explicitly
> > enabled), link ld to it, and use it to validate that the input is
> > correct JSON, to avoid writing garbage to the file. The
> > configure option --enable-jansson has to be used to explicitly enable
> > it (error out when not found). This allows bootstrappers (or others who
> > are not interested) to seamlessly skip it without issues.
> > ---
> > v2: apply fixes from review
> 
> Hello Cary,
> 
> Anything else you'd like me to change here? Thanks!

CC'ing Ian on suggestion from Nick. Would be great if a maintainer
could have a look and let me know if we need more changes.

Meanwhile, Fedora has backported this patch:

https://src.fedoraproject.org/rpms/binutils/c/9ee9ffc894cc632b54d862d6213e2d8fcb8db829?branch=rawhide

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] gold: add --package-metadata

[Cary Coutant]

> v2: apply fixes from review
>
>  elfcpp/elfcpp.h                        |  4 ++-
>  gold/Makefile.am                       |  8 ++---
>  gold/configure.ac                      | 26 ++++++++++++++
>  gold/layout.cc                         | 50 ++++++++++++++++++++++++++
>  gold/layout.h                          |  6 ++++
>  gold/options.h                         |  4 +++
>  gold/testsuite/Makefile.am             | 15 +++++---
>  gold/testsuite/package_metadata_main.c |  5 +++

Committed. Thanks!

elfcpp/
        * elfcpp.h: Add FDO_PACKAGING_METADATA note type.

gold/
        * Makefile.am: Add jansson flags and libraries.
        * configure.ac: Check for jansson library.
        * layout.cc (Layout::create_notes): Call create_package_metadata().
        (Layout::create_package_metadata): New function.
        * layout.h (Layout::create_package_metadata): New function.
        (Layout::package_metadata_note_): New data member.
        * options.h (class General_options): Add --package-metadata option.
        * testsuite/Makefile.am (object_unittest): Add jansson libraries.
        (binary_unittest): Likewise.
        (leb128_unittest): Likewise.
        (overflow_unittest): Likewise.
        (package_metadata_test): New test.
        * testsuite/package_metadata_main.c: New test source.

-cary

Re: [PATCH v2] gold: add --package-metadata

[Luca Boccassi]

On Fri, 5 Aug 2022 at 01:41, Cary Coutant <ccoutant@gmail.com> wrote:
>
> > v2: apply fixes from review
> >
> >  elfcpp/elfcpp.h                        |  4 ++-
> >  gold/Makefile.am                       |  8 ++---
> >  gold/configure.ac                      | 26 ++++++++++++++
> >  gold/layout.cc                         | 50 ++++++++++++++++++++++++++
> >  gold/layout.h                          |  6 ++++
> >  gold/options.h                         |  4 +++
> >  gold/testsuite/Makefile.am             | 15 +++++---
> >  gold/testsuite/package_metadata_main.c |  5 +++
>
> Committed. Thanks!
>
> elfcpp/
>         * elfcpp.h: Add FDO_PACKAGING_METADATA note type.
>
> gold/
>         * Makefile.am: Add jansson flags and libraries.
>         * configure.ac: Check for jansson library.
>         * layout.cc (Layout::create_notes): Call create_package_metadata().
>         (Layout::create_package_metadata): New function.
>         * layout.h (Layout::create_package_metadata): New function.
>         (Layout::package_metadata_note_): New data member.
>         * options.h (class General_options): Add --package-metadata option.
>         * testsuite/Makefile.am (object_unittest): Add jansson libraries.
>         (binary_unittest): Likewise.
>         (leb128_unittest): Likewise.
>         (overflow_unittest): Likewise.
>         (package_metadata_test): New test.
>         * testsuite/package_metadata_main.c: New test source.
>
> -cary

Fantastic, thank you very much!

Kind regards,
Luca Boccassi