[ELF] --package-metadata: support %[0-9a-fA-F][0-9a-fA-F]

[MaskRay]

(This application-specific option is probably not appropriate as a
linker option (.o file offers more flexibility and decouples JSON
verification from linkers). However, the option has gained some traction
in Linux distributions, with support in GNU ld, gold, and mold.)

GNU ld has supported percent-encoded bytes and extensions like
%[comma] since November 2024. mold supports just percent-encoded
bytes. To prepare for potential adoption by Ubuntu, let's support
percent-encoded bytes.

Link: https://sourceware.org/bugzilla/show_bug.cgi?id=32003
Link: https://bugs.launchpad.net/ubuntu/+source/dpkg/+bug/2071468

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Fangrui Song (MaskRay)

Changes

(This application-specific option is probably not appropriate as a
linker option (.o file offers more flexibility and decouples JSON
verification from linkers). It has gained some traction, with support in GNU ld, gold, and mold.
)

GNU ld has supported percent-encoded bytes and extensions like
%[comma] since November 2024. mold supports just percent-encoded
bytes. To prepare for potential adoption by Ubuntu, let's support
percent-encoded bytes.

---

Full diff: https://github.com/llvm/llvm-project/pull/126396.diff

5 Files Affected:

• (modified) lld/ELF/Config.h (+1-1)
• (modified) lld/ELF/Driver.cpp (+22-1)
• (modified) lld/ELF/Options.td (+1-1)
• (modified) lld/docs/ld.lld.1 (+4)
• (modified) lld/test/ELF/package-metadata.s (+15-5)

diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 3cdb400e423fd95..f132b11b20c631e 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -411,7 +411,7 @@ struct Config {
   StringRef thinLTOJobs;
   unsigned timeTraceGranularity;
   int32_t splitStackAdjustSize;
-  StringRef packageMetadata;
+  SmallVector<uint8_t, 0> packageMetadata;
 
   // The following config options do not directly correspond to any
   // particular command line options.
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 3d6e022a89e5f68..7d14180a4992622 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -816,6 +816,26 @@ static ICFLevel getICF(opt::InputArgList &args) {
   return ICFLevel::All;
 }
 
+static void parsePackageMetadata(Ctx &ctx, const opt::Arg &arg) {
+  unsigned c0, c1;
+  SmallVector<uint8_t, 0> decoded;
+  StringRef s = arg.getValue();
+  for (size_t i = 0, e = s.size(); i != e; ++i) {
+    if (s[i] != '%') {
+      decoded.push_back(s[i]);
+    } else if (i + 2 < e && (c1 = hexDigitValue(s[i + 1])) != -1u &&
+               (c0 = hexDigitValue(s[i + 2])) != -1u) {
+      decoded.push_back(uint8_t(c1 * 16 + c0));
+      i += 2;
+    } else {
+      ErrAlways(ctx) << arg.getSpelling() << ": invalid % escape at byte " << i
+                     << "; supports only %[0-9a-fA-F][0-9a-fA-F]";
+      return;
+    }
+  }
+  ctx.arg.packageMetadata = std::move(decoded);
+}
+
 static StripPolicy getStrip(Ctx &ctx, opt::InputArgList &args) {
   if (args.hasArg(OPT_relocatable))
     return StripPolicy::None;
@@ -1425,7 +1445,8 @@ static void readConfigs(Ctx &ctx, opt::InputArgList &args) {
   ctx.arg.optimize = args::getInteger(args, OPT_O, 1);
   ctx.arg.orphanHandling = getOrphanHandling(ctx, args);
   ctx.arg.outputFile = args.getLastArgValue(OPT_o);
-  ctx.arg.packageMetadata = args.getLastArgValue(OPT_package_metadata);
+  if (auto *arg = args.getLastArg(OPT_package_metadata))
+    parsePackageMetadata(ctx, *arg);
   ctx.arg.pie = args.hasFlag(OPT_pie, OPT_no_pie, false);
   ctx.arg.printIcfSections =
       args.hasFlag(OPT_print_icf_sections, OPT_no_print_icf_sections, false);
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index 80032490da0de4b..b3b12a064687580 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -578,7 +578,7 @@ def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
 def visual_studio_diagnostics_format : FF<"vs-diagnostics">,
 HelpText<"Format diagnostics for Visual Studio compatibility">;
 
-def package_metadata: JJ<"package-metadata=">, HelpText<"Emit package metadata note">;
+def package_metadata: JJ<"package-metadata=">, HelpText<"Emit a percent-encoded string to the .note.package section">;
 
 // Aliases
 def: Separate<["-"], "dT">, Alias<default_script>, HelpText<"Alias for --default-script">;
diff --git a/lld/docs/ld.lld.1 b/lld/docs/ld.lld.1
index b28c6082f68b091..b5c1816ce6e5fde 100644
--- a/lld/docs/ld.lld.1
+++ b/lld/docs/ld.lld.1
@@ -493,6 +493,10 @@ If
 .Fl -use-android-relr-tags
 is specified, use SHT_ANDROID_RELR instead of SHT_RELR.
 .Pp
+.It Fl -package-metadata
+Emit a percent-encoded string to the
+.Cm .note.package
+section. For example, %25 decodes to a single %.
 .It Fl -pic-veneer
 Always generate position independent thunks.
 .It Fl -pie , Fl -pic-executable
diff --git a/lld/test/ELF/package-metadata.s b/lld/test/ELF/package-metadata.s
index 29df499d7e98d29..a70a8940d7c68bb 100644
--- a/lld/test/ELF/package-metadata.s
+++ b/lld/test/ELF/package-metadata.s
@@ -1,12 +1,15 @@
 # REQUIRES: x86
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
 
-# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: ld.lld a.o --package-metadata='{}'
+# RUN: llvm-readelf -n a.out | FileCheck %s --check-prefixes=NOTE,FIRST
 
-# RUN: ld.lld %t.o -o %t --package-metadata='{}'
-# RUN: llvm-readelf -n %t | FileCheck %s --check-prefixes=NOTE,FIRST
+# RUN: ld.lld a.o --package-metadata='{"abc":123}'
+# RUN: llvm-readelf -n a.out | FileCheck %s --check-prefixes=NOTE,SECOND
 
-# RUN: ld.lld %t.o -o %t --package-metadata='{"abc":123}'
-# RUN: llvm-readelf -n %t | FileCheck %s --check-prefixes=NOTE,SECOND
+# RUN: ld.lld a.o --package-metadata='%7b%22abc%22:123%7D'
+# RUN: llvm-readelf -n a.out | FileCheck %s --check-prefixes=NOTE,SECOND
 
 # NOTE: .note.package
 # NOTE-NEXT: Owner
@@ -14,6 +17,13 @@
 # FIRST-NEXT: description data: 7b 7d 00
 # SECOND-NEXT: description data: 7b 22 61 62 63 22 3a 31 32 33 7d 00
 
+# RUN: not ld.lld a.o --package-metadata='%7b%' 2>&1 | FileCheck %s --check-prefix=ERR
+# RUN: not ld.lld a.o --package-metadata='%7b%7' 2>&1 | FileCheck %s --check-prefix=ERR
+# RUN: not ld.lld a.o --package-metadata='%7b%7g' 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: error: --package-metadata=: invalid % escape at byte 3; supports only %[0-9a-fA-F][0-9a-fA-F]
+
+#--- a.s
 .globl _start
 _start:
   ret

[smithp35]

Looks good to me.

I thought it might be good to say that HTML named-characters are not supported in the help/man-page (https://sourceware.org/binutils/docs/ld/Options.html#index-_002d_002dpackage_002dmetadata_003dJSON) . But I've not got a strong opinion on that as the error message is clear on what is supported.

I agree that this would be better off passed in an object file, or even via a text file containing the raw JSON. I guess this would have meant too much faffing with one or more build-systems rather than just adding to the linker flags. However it looks like that if we want distros to be able to use LLD, we'll need to support it ... grumble.

[MaskRay]

Thanks! The feature is quite isolated. Creating a cherry-pick in case it does get more uses on Ubuntu.

/cherry-pick 0a470a9

[llvmbot]

/pull-request #126549

[bdrung]

I just wanted to implement that feature in lld and saw that it was already done. Thank you very much!