From c257628f0d0df344933a90c1ada2f63ddf7291ad Mon Sep 17 00:00:00 2001 From: Benjamin Drung Date: Tue, 17 Dec 2024 15:27:46 +0000 Subject: [PATCH] [PATCH RESEND] gold: Support percent-encoded JSON in --package-metadata From benjamin.drung@canonical.com Tue Dec 17 15:27:46 2024 Message-ID: <20241217152746.98360-1-benjamin.drung@canonical.com> Specifying the compiler flag `-Wl,--package-metadata=` 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. Following the same format as the implementation in ld: b0cc81e87087bb8a6b12dc1e4fd7f2591927977b 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 Gbp-Pq: Name gold-package-metadata.diff --- 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 545fb3da4..1f5b12053 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 3e5f7122e..dfa8d4f65 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. @@ -1107,7 +1122,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 8f158ba20..d7f496c75 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 357dec0d4..6cf619fd9 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.30.2