[PATCH] D115041: [ELF] Do not report undefined weak references in shared libraries

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

(I am in a trip and have limited Internet connection.)

This is a good candidate for 13.0.1



================
Comment at: lld/ELF/InputFiles.cpp:1539
       s->exportDynamic = true;
-      if (s->isUndefined() && !s->isWeak() &&
+      if (s->isUndefined() && sym.getBinding() == STB_GLOBAL &&
           config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore)
----------------
I just noticed that we did not (before this patch) have test coverage for `&& !s->isWeak()`. 

If the binding check is tested here, Writer.cpp:1978 binding check may be deleted.


================
Comment at: lld/test/ELF/allow-shlib-undefined-weak.s:1
+REQUIRES: x86
+
----------------
While the suffix name is .s, it is customary to use `#` for RUN/CHECK lines to not render them as instructions which may look weird in some editors.


================
Comment at: lld/test/ELF/allow-shlib-undefined-weak.s:21
+RUN: ld.lld -shared def.o -o def.so
+RUN: ld.lld -shared wrap.o def.so -o wrap.so
+
----------------
When a .so is linked into another output, specify -soname= to ensure the final output has a deterministic DT_NEEDED string. It doesn't matter for this test, but can avoid a problem in case the test gets more checks.


================
Comment at: lld/test/ELF/allow-shlib-undefined-weak.s:27
+
+CHECK-NOT: undefined reference to foo [--no-allow-shlib-undefined]
+
----------------
Checking just the diagnostic can easily go stale. Use `| count 0` if the output is empty.


================
Comment at: lld/test/ELF/allow-shlib-undefined-weak.s:39
+  movq foo at GOTPCREL(%rip), %rax
+  movl (%rax), %eax
+  retq
----------------
You can remove `movl (%rax), %eax`. The semantic will be to get the address, which still looks like a real world example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115041