1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319
|
From benjamin.drung@canonical.com Tue Dec 17 15:27:46 2024
From: benjamin.drung@canonical.com (Benjamin Drung)
Date: Tue, 17 Dec 2024 15:27:46 +0000
Subject: [PATCH RESEND] gold: Support percent-encoded JSON in
--package-metadata
Message-ID: <20241217152746.98360-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.
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 <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(-)
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -255,6 +255,78 @@ parse_string(const char* option_name, co
*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)
{
--- a/gold/options.h
+++ b/gold/options.h
@@ -109,6 +109,10 @@ 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);
@@ -592,6 +596,17 @@ struct Struct_special : public Struct_va
// 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.
#define DEFINE_optional_string(varname__, dashes__, shortname__, \
default_value__, \
helpstring__, helparg__) \
@@ -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]"));
--- 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 %"}'
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -110,7 +110,11 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__E
$(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_overflo
@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) $(aar
$(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$(EX
--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.
|