[PATCH] D40849: CodeGen: support an extension to pass linker options on ELF

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: test/Feature/elf-linker-options.ll:5-7
+!0 = !{!"lib", !"iberty"}
+!1 = !{!"path", !"/Users/compnerd/DerivedData/binutils-gdb-linux-x86_64/libiberty"}
+!2 = !{!"rpath", !"/var/empty"}
----------------
"path" and "rpath" look too real and misleading. Can you use meaningless words such as foo or bar so that this test doesn't look like a guidance?


================
Comment at: test/tools/llvm-readobj/elf-linker-options.ll:6-7
+!0 = !{!"lib", !"iberty"}
+!1 = !{!"path", !"/Users/compnerd/DerivedData/binutils-gdb-linux-x86_64/libiberty"}
+!2 = !{!"rpath", !"/var/empty"}
+
----------------
jhenderson wrote:
> As with the other test, I'd suggest adding a space to one of these.
Ditto


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:4085-4086
+    if (Name != ".linker-options")
+      continue;
+
+    ArrayRef<uint8_t> Contents = unwrapOrError(Obj->getSectionContents(&Shdr));
----------------
It is one of the design principles of ELF that section names are not significant. Section should be identified by type and not by name. Can you define a new section type and use it?


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:4093-4095
+                        .Case("lib", "library")
+                        .Case("path", "library search path")
+                        .Default(Key),
----------------
I prefer just printing these strings as-is instead of converting "lib" to "library" and "path" to "library search path".


Repository:
  rL LLVM

https://reviews.llvm.org/D40849