[PATCH v3] gold: add --encoded-package-metadata

Benjamin Drung benjamin.drung@canonical.com
Thu Jul 18 15:56:24 GMT 2024


Specifying the compiler flag `-Wl,--package-metadata=<JSON>` might not
work, because the shells might eat the quotation marks and the compiler
splits the JSON at the commas.

Ubuntu tried to using a specs file to set `--package-metadata` but that
turned out to be too fragile. autopkgtests might use the compiler flags
but the needed environment variables are not set in the test
environment. Debugging a crash of an application build with the -spec
parameter lacks the environment variables. People like to iteratively
continue building the software in the build directory while hacking on
the package and then have no environment variable set.

So introduce a `--encoded-package-metadata` linker flag that takes a
percent-encoded JSON. Percent-encoding is used because it is a
standard and simple to implement.

Bug-Ubutru: https://bugs.launchpad.net/bugs/2071468
Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
---
 gold/layout.cc             | 27 ++++++++++++++++++++-----
 gold/options.cc            | 41 ++++++++++++++++++++++++++++++++++++++
 gold/options.h             | 19 ++++++++++++++++++
 gold/testsuite/Makefile.am |  6 ++++++
 gold/testsuite/Makefile.in |  6 ++++++
 5 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/gold/layout.cc b/gold/layout.cc
index b43ae841a6c..5a9cd643d4d 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -3560,14 +3560,31 @@ Layout::create_build_id()
 // If --package-metadata was used, set up the package metadata note.
 // https://systemd.io/ELF_PACKAGE_METADATA/
 
+static const char*
+get_package_metadata()
+{
+  if (parameters->options().user_set_package_metadata())
+    {
+      const char* desc = parameters->options().package_metadata();
+      if (strcmp(desc, "") != 0)
+	return desc;
+    }
+
+  if (parameters->options().user_set_encoded_package_metadata())
+    {
+      const char* desc = parameters->options().encoded_package_metadata();
+      if (strcmp(desc, "") != 0)
+	return desc;
+    }
+
+  return NULL;
+}
+
 void
 Layout::create_package_metadata()
 {
-  if (!parameters->options().user_set_package_metadata())
-    return;
-
-  const char* desc = parameters->options().package_metadata();
-  if (strcmp(desc, "") == 0)
+  const char* desc = get_package_metadata();
+  if (desc == NULL)
     return;
 
 #ifdef HAVE_JANSSON
diff --git a/gold/options.cc b/gold/options.cc
index 46f067fa72f..ae553aa23a0 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -255,6 +255,47 @@ parse_string(const char* option_name, const char* arg, const char** retval)
   *retval = arg;
 }
 
+void
+parse_optional_encoded_string(const char* option_name, const char* arg, const char** retval)
+{
+  hex_init();
+  size_t arg_len = strlen (arg);
+  char *package_metadata = (char *)malloc (arg_len + 1);
+  if (package_metadata == NULL)
+    gold_fatal(_("%s: malloc failed"), option_name);
+
+  const char *src = arg;
+  char *dst = package_metadata;
+  while (*src != '\0')
+    {
+      char c = *src++;
+      if (c != '%')
+	{
+	  *dst++ = c;
+	  continue;
+	}
+      char next1 = *src++;
+      if (next1 == '%')
+	{
+	  *dst++ = '%';
+	  continue;
+	}
+      if (next1 == '\0')
+	gold_fatal(_("%s: invalid option value "
+		     "(expected a percent-encoded string): %s\n"),
+		   option_name, arg);
+      char next2 = *src++;
+      if (!hex_p(next1) || !hex_p(next2))
+	gold_fatal(_("%s: invalid option value "
+		     "(expected a percent-encoded string): %s\n"),
+		   option_name, arg);
+      *dst++ = (char) ((hex_value(next1) << 4) + hex_value(next2));
+    }
+  *dst = '\0';
+
+  *retval = package_metadata;
+}
+
 void
 parse_optional_string(const char*, const char* arg, const char** retval)
 {
diff --git a/gold/options.h b/gold/options.h
index 446e8d42614..230eef9cb8e 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -108,6 +108,10 @@ parse_percent(const char* option_name, const char* arg, double* retval);
 extern void
 parse_string(const char* option_name, const char* arg, const char** retval);
 
+extern void
+parse_optional_encoded_string(const char* option_name, const char* arg,
+			      const char** retval);
+
 extern void
 parse_optional_string(const char* option_name, const char* arg,
 		      const char** retval);
@@ -589,6 +593,17 @@ struct Struct_special : public Struct_var
   };                                                                    \
   Struct_##varname__ varname__##_initializer_
 
+// An option that takes an optional string argument.  If the option is
+// used with no argument, the value will be the default, and
+// user_set_via_option will be true.
+#define DEFINE_optional_encoded_string(varname__, dashes__,		\
+				       shortname__, default_value__,	\
+				       helpstring__, helparg__)		\
+  DEFINE_var(varname__, dashes__, shortname__, default_value__,		\
+	     default_value__, helpstring__, helparg__, true,		\
+	     const char*, const char*,					\
+	     options::parse_optional_encoded_string, false)
+
 // An option that takes an optional string argument.  If the option is
 // used with no argument, the value will be the default, and
 // user_set_via_option will be true.
@@ -835,6 +850,10 @@ class General_options
 	      N_("(PowerPC only) Label linker stubs with a symbol"),
 	      N_("(PowerPC only) Do not label linker stubs with a symbol"));
 
+  DEFINE_optional_encoded_string(encoded_package_metadata, options::TWO_DASHES, '\0', NULL,
+			 N_("Generate package metadata note"),
+			 N_("[=PERCENT_ENCODED_JSON]"));
+
   DEFINE_string(entry, options::TWO_DASHES, 'e', NULL,
 		N_("Set program start address"), N_("ADDRESS"));
 
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 2f1348fd6e2..d28b1af5547 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -4454,3 +4454,9 @@ package_metadata_test.o: package_metadata_main.c
 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"}'
+package_metadata_test1b$(EXEEXT): package_metadata_test.o gcctestdir/ld
+	$(CXXLINK) package_metadata_test.o -Wl,--encoded-package-metadata=%7B%22foo%22%3A%22bar%22%7D
+	$(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
+package_metadata_test2$(EXEEXT): package_metadata_test.o gcctestdir/ld
+	$(CXXLINK) package_metadata_test.o -Wl,--encoded-package-metadata=%7B%22name%22:%22binutils%22%2C%22foo%22%3A%22bar%22%7d
+	$(TEST_READELF) --notes $@ | grep -q '{"name":"binutils","foo":"bar"}'
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 9cf21df8d7d..a22186d040c 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -10492,6 +10492,12 @@ package_metadata_test.o: package_metadata_main.c
 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"}'
+package_metadata_test1b$(EXEEXT): package_metadata_test.o gcctestdir/ld
+	$(CXXLINK) package_metadata_test.o -Wl,--encoded-package-metadata=%7B%22foo%22%3A%22bar%22%7D
+	$(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
+package_metadata_test2$(EXEEXT): package_metadata_test.o gcctestdir/ld
+	$(CXXLINK) package_metadata_test.o -Wl,--encoded-package-metadata=%7B%22name%22:%22binutils%22%2C%22foo%22%3A%22bar%22%7d
+	$(TEST_READELF) --notes $@ | grep -q '{"name":"binutils","foo":"bar"}'
 
 # Tell versions [3.59,3.63) of GNU make to not export all variables.
 # Otherwise a system limit (for SysV at least) may be exceeded.
-- 
2.43.0



More information about the Binutils mailing list