From: Benjamin Drung <benjamin.drung@canonical.com>
To: binutils@sourceware.org
Cc: Benjamin Drung <benjamin.drung@canonical.com>
Subject: [PATCH] gold: add --encoded-package-metadata
Date: Thu, 18 Jul 2024 15:55:24 +0000 [thread overview]
Message-ID: <20240718155524.17623-1-benjamin.drung@canonical.com> (raw)
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
next reply other threads:[~2024-07-18 15:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-18 15:55 Benjamin Drung [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-07-16 16:18 Benjamin Drung
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240718155524.17623-1-benjamin.drung@canonical.com \
--to=benjamin.drung@canonical.com \
--cc=binutils@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).