[PATCH] D55682: [ELF] Support for setting __start/__stop symbol visibility

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D55682#2094089 <https://reviews.llvm.org/D55682#2094089>, @MaskRay wrote:

> Please add a test, `startstop-visibility.s` (I would like start-stop-visibility.s, but some existing tests use the startstop- prefix).
>
> Use `-triple=x86_64` and use `llvm-readelf -s` to check visibility. Check both `__start_foo` and `__stop_foo`. Don't add another `__start_bar`.
>
> Please mention that GNU ld and gold from binutils 2.35 will support the option (
>  https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=cae64165f47b64898c4f1982d294862cfae89a47 )


Probably call out the option name in the subject, i.e. `Add -z start-stop-visibility= to set __start_/__stop_ symbol visibility`



================
Comment at: lld/ELF/Driver.cpp:418
+    if (StringRef("start-stop-visibility") == kv.first) {
+      if (StringRef("default") == kv.second) {
+        return STV_DEFAULT;
----------------
if (kv.second == "default")

The braces can be omitted.


================
Comment at: lld/test/ELF/startstop-visibility.s:29
+
+// RUN: not ld.lld -z start-stop-visibility=aaa %t.o -o %t
+// CHECK-ERROR: error: unknown -z start-stop-visibility= value: aaa
----------------
Use -o /dev/null if the output is not used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55682/new/

https://reviews.llvm.org/D55682