From: Benjamin Drung <benjamin.drung@canonical.com>
To: binutils@sourceware.org
Cc: Benjamin Drung <benjamin.drung@canonical.com>
Subject: [PATCH 2/2] gold: Support percent-encoded JSON in --package-metadata
Date: Mon, 11 Nov 2024 10:02:07 +0000 [thread overview]
Message-ID: <20241111100207.219703-2-benjamin.drung@canonical.com> (raw)
In-Reply-To: <20241111100207.219703-1-benjamin.drung@canonical.com>
Specifying the compiler flag `-Wl,--package-metadata=<JSON>` will not
work in case the JSON contains a comma, because compiler drivers eat
commas. Example:
```
$ echo "void main() { }" > test.c
$ gcc -fuse-ld=gold '-Wl,--package-metadata={"type":"deb","os":"ubuntu"}' test.c
/usr/bin/ld.gold: error: cannot open "os":"ubuntu"}: No such file or directory
/usr/bin/ld.gold: fatal error: error: --package-metadata={"type":"deb" does not contain valid JSON: '}' expected near end of file
collect2: error: ld returned 1 exit status
```
The quotation marks in the JSON value do not work well with shell nor
make. Specifying the `--package-metadata` linker flag in a `LDFLAGS`
environment variable might loose its quotation marks when it hits the
final compiler call.
So support percent-encoded and %[string] encoded JSON data in the
`--package-metadata` linker flag. Percent-encoding is used because it is
a standard, simple to implement, and does take too many additional
characters. %[string] encoding is supported for having a more readable
encoding.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32003
Bug-Ubutru: https://bugs.launchpad.net/bugs/2071468
Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
---
gold/options.cc | 72 +++++++++++++++++++++++++++++++++
gold/options.h | 17 +++++++-
gold/testsuite/Makefile.am | 16 +++++++-
gold/testsuite/Makefile.in | 82 +++++++++++++++++++++++++++++++++-----
4 files changed, 174 insertions(+), 13 deletions(-)
diff --git a/gold/options.cc b/gold/options.cc
index 46f067fa72f..4f615d44e28 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -255,6 +255,78 @@ parse_string(const char* option_name, const char* arg, const char** retval)
*retval = arg;
}
+// Decode a percent and/or %[string] encoded string. Following %[string]
+// encodings are supported:
+//
+// %[comma] for ,
+// %[lbrace] for {
+// %[quot] for "
+// %[rbrace] for }
+// %[space] for ' '
+//
+// The percent decoding behaves the same as Python's urllib.parse.unquote.
+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 == '%')
+ {
+ char next1 = *src;
+ if (next1 != '\0' && hex_p(next1))
+ {
+ char next2 = *(src + 1);
+ if (next2 != '\0' && hex_p(next2))
+ {
+ c = (char)((hex_value(next1) << 4) + hex_value(next2));
+ src += 2;
+ }
+ }
+ else if (next1 == '[')
+ {
+ if (strncmp(src + 1, "comma]", 6) == 0)
+ {
+ c = ',';
+ src += 7;
+ }
+ else if (strncmp(src + 1, "lbrace]", 7) == 0)
+ {
+ c = '{';
+ src += 8;
+ }
+ else if (strncmp(src + 1, "quot]", 5) == 0)
+ {
+ c = '"';
+ src += 6;
+ }
+ else if (strncmp(src + 1, "rbrace]", 7) == 0)
+ {
+ c = '}';
+ src += 8;
+ }
+ else if (strncmp(src + 1, "space]", 6) == 0)
+ {
+ c = ' ';
+ src += 7;
+ }
+ }
+ }
+ *dst++ = c;
+ }
+ *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..78766c1fc6b 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.
@@ -1106,7 +1121,7 @@ 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,
+ DEFINE_optional_encoded_string(package_metadata, options::TWO_DASHES, '\0', NULL,
N_("Generate package metadata note"),
N_("[=JSON]"));
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 8f158ba20cc..d7f496c75c1 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -4470,9 +4470,21 @@ retain_2.o: retain_2.s
endif DEFAULT_TARGET_X86_64
-check_PROGRAMS += package_metadata_test
+check_PROGRAMS += package_metadata_test1 package_metadata_test1b package_metadata_test1c package_metadata_test2 package_metadata_test2b
package_metadata_test.o: package_metadata_main.c
$(COMPILE) -c -o $@ $<
-package_metadata_test$(EXEEXT): package_metadata_test.o gcctestdir/ld
+package_metadata_test1$(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,--package-metadata=%7B%22foo%22%3A%22bar%22%7D
+ $(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
+package_metadata_test1c$(EXEEXT): package_metadata_test.o gcctestdir/ld
+ $(CXXLINK) package_metadata_test.o -Wl,--package-metadata='%[lbrace]%[quot]foo%[quot]:%[quot]bar%[quot]%[rbrace]'
+ $(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
+package_metadata_test2$(EXEEXT): package_metadata_test.o gcctestdir/ld
+ $(CXXLINK) package_metadata_test.o -Wl,--package-metadata=%7B%22name%22:%22binutils%22%2C%22ver%22%3A%22x%20%%22%7d
+ $(TEST_READELF) --notes $@ | grep -q '{"name":"binutils","ver":"x %"}'
+package_metadata_test2b$(EXEEXT): package_metadata_test.o gcctestdir/ld
+ $(CXXLINK) package_metadata_test.o -Wl,--package-metadata='{%[quot]name%[quot]:%[quot]binutils%[quot]%[comma]%[quot]ver%[quot]:%[quot]x%[space]%%[quot]}'
+ $(TEST_READELF) --notes $@ | grep -q '{"name":"binutils","ver":"x %"}'
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 357dec0d4f9..6cf619fd974 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -110,7 +110,11 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
$(am__EXEEXT_40) $(am__EXEEXT_41) $(am__EXEEXT_42) \
$(am__EXEEXT_43) $(am__EXEEXT_44) $(am__EXEEXT_45) \
$(am__EXEEXT_46) $(am__EXEEXT_47) $(am__EXEEXT_48) \
- package_metadata_test$(EXEEXT)
+ package_metadata_test1$(EXEEXT) \
+ package_metadata_test1b$(EXEEXT) \
+ package_metadata_test1c$(EXEEXT) \
+ package_metadata_test2$(EXEEXT) \
+ package_metadata_test2b$(EXEEXT)
@NATIVE_OR_CROSS_LINKER_TRUE@am__append_1 = object_unittest \
@NATIVE_OR_CROSS_LINKER_TRUE@ binary_unittest leb128_unittest \
@NATIVE_OR_CROSS_LINKER_TRUE@ overflow_unittest
@@ -1799,9 +1803,21 @@ overflow_unittest_OBJECTS = $(am_overflow_unittest_OBJECTS)
@NATIVE_OR_CROSS_LINKER_TRUE@ $(am__DEPENDENCIES_1)
overflow_unittest_LINK = $(CXXLD) $(AM_CXXFLAGS) $(CXXFLAGS) \
$(overflow_unittest_LDFLAGS) $(LDFLAGS) -o $@
-package_metadata_test_SOURCES = package_metadata_test.c
-package_metadata_test_OBJECTS = package_metadata_test.$(OBJEXT)
-package_metadata_test_LDADD = $(LDADD)
+package_metadata_test1_SOURCES = package_metadata_test1.c
+package_metadata_test1_OBJECTS = package_metadata_test1.$(OBJEXT)
+package_metadata_test1_LDADD = $(LDADD)
+package_metadata_test1b_SOURCES = package_metadata_test1b.c
+package_metadata_test1b_OBJECTS = package_metadata_test1b.$(OBJEXT)
+package_metadata_test1b_LDADD = $(LDADD)
+package_metadata_test1c_SOURCES = package_metadata_test1c.c
+package_metadata_test1c_OBJECTS = package_metadata_test1c.$(OBJEXT)
+package_metadata_test1c_LDADD = $(LDADD)
+package_metadata_test2_SOURCES = package_metadata_test2.c
+package_metadata_test2_OBJECTS = package_metadata_test2.$(OBJEXT)
+package_metadata_test2_LDADD = $(LDADD)
+package_metadata_test2b_SOURCES = package_metadata_test2b.c
+package_metadata_test2b_OBJECTS = package_metadata_test2b.$(OBJEXT)
+package_metadata_test2b_LDADD = $(LDADD)
permission_test_SOURCES = permission_test.c
permission_test_OBJECTS = permission_test.$(OBJEXT)
permission_test_LDADD = $(LDADD)
@@ -2355,7 +2371,9 @@ SOURCES = $(libgoldtest_a_SOURCES) $(aarch64_pr23870_SOURCES) \
$(large_symbol_alignment_SOURCES) $(leb128_unittest_SOURCES) \
local_labels_test.c many_sections_r_test.c \
$(many_sections_test_SOURCES) $(object_unittest_SOURCES) \
- $(overflow_unittest_SOURCES) package_metadata_test.c \
+ $(overflow_unittest_SOURCES) package_metadata_test1.c \
+ package_metadata_test1b.c package_metadata_test1c.c \
+ package_metadata_test2.c package_metadata_test2b.c \
permission_test.c $(pie_copyrelocs_test_SOURCES) \
plugin_test_1.c plugin_test_10.c plugin_test_11.c \
plugin_test_12.c plugin_test_2.c plugin_test_3.c \
@@ -4869,7 +4887,11 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/many_sections_test.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/object_unittest.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/overflow_unittest.Po@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/package_metadata_test.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/package_metadata_test1.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/package_metadata_test1b.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/package_metadata_test1c.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/package_metadata_test2.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/package_metadata_test2b.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/permission_test.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pie_copyrelocs_test-pie_copyrelocs_test.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_1.Po@am__quote@
@@ -7854,9 +7876,37 @@ aarch64_pr23870.log: aarch64_pr23870$(EXEEXT)
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
"$$tst" $(AM_TESTS_FD_REDIRECT)
-package_metadata_test.log: package_metadata_test$(EXEEXT)
- @p='package_metadata_test$(EXEEXT)'; \
- b='package_metadata_test'; \
+package_metadata_test1.log: package_metadata_test1$(EXEEXT)
+ @p='package_metadata_test1$(EXEEXT)'; \
+ b='package_metadata_test1'; \
+ $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
+ --log-file $$b.log --trs-file $$b.trs \
+ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+ "$$tst" $(AM_TESTS_FD_REDIRECT)
+package_metadata_test1b.log: package_metadata_test1b$(EXEEXT)
+ @p='package_metadata_test1b$(EXEEXT)'; \
+ b='package_metadata_test1b'; \
+ $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
+ --log-file $$b.log --trs-file $$b.trs \
+ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+ "$$tst" $(AM_TESTS_FD_REDIRECT)
+package_metadata_test1c.log: package_metadata_test1c$(EXEEXT)
+ @p='package_metadata_test1c$(EXEEXT)'; \
+ b='package_metadata_test1c'; \
+ $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
+ --log-file $$b.log --trs-file $$b.trs \
+ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+ "$$tst" $(AM_TESTS_FD_REDIRECT)
+package_metadata_test2.log: package_metadata_test2$(EXEEXT)
+ @p='package_metadata_test2$(EXEEXT)'; \
+ b='package_metadata_test2'; \
+ $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
+ --log-file $$b.log --trs-file $$b.trs \
+ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+ "$$tst" $(AM_TESTS_FD_REDIRECT)
+package_metadata_test2b.log: package_metadata_test2b$(EXEEXT)
+ @p='package_metadata_test2b$(EXEEXT)'; \
+ b='package_metadata_test2b'; \
$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
@@ -10521,9 +10571,21 @@ uninstall-am:
@DEFAULT_TARGET_X86_64_TRUE@ $(TEST_AS) -o $@ $<
package_metadata_test.o: package_metadata_main.c
$(COMPILE) -c -o $@ $<
-package_metadata_test$(EXEEXT): package_metadata_test.o gcctestdir/ld
+package_metadata_test1$(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,--package-metadata=%7B%22foo%22%3A%22bar%22%7D
+ $(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
+package_metadata_test1c$(EXEEXT): package_metadata_test.o gcctestdir/ld
+ $(CXXLINK) package_metadata_test.o -Wl,--package-metadata='%[lbrace]%[quot]foo%[quot]:%[quot]bar%[quot]%[rbrace]'
+ $(TEST_READELF) --notes $@ | grep -q '{"foo":"bar"}'
+package_metadata_test2$(EXEEXT): package_metadata_test.o gcctestdir/ld
+ $(CXXLINK) package_metadata_test.o -Wl,--package-metadata=%7B%22name%22:%22binutils%22%2C%22ver%22%3A%22x%20%%22%7d
+ $(TEST_READELF) --notes $@ | grep -q '{"name":"binutils","ver":"x %"}'
+package_metadata_test2b$(EXEEXT): package_metadata_test.o gcctestdir/ld
+ $(CXXLINK) package_metadata_test.o -Wl,--package-metadata='{%[quot]name%[quot]:%[quot]binutils%[quot]%[comma]%[quot]ver%[quot]:%[quot]x%[space]%%[quot]}'
+ $(TEST_READELF) --notes $@ | grep -q '{"name":"binutils","ver":"x %"}'
# 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 prev parent reply other threads:[~2024-11-11 10:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-11 10:02 [PATCH 1/2] ld: " Benjamin Drung
2024-11-11 10:02 ` Benjamin Drung [this message]
2024-11-12 13:35 ` Jan Beulich
2024-11-12 15:28 ` Benjamin Drung
2024-11-12 15:43 ` Jan Beulich
2024-11-18 10:41 ` 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=20241111100207.219703-2-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).