[PATCH] D92258: [ELF][test] Add some tests for versioned symbols in object files

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: lld/test/ELF/symver.s:1
+# REQUIRES: x86
+# RUN: split-file %s %t
----------------
Perhaps worth a top-level comment saying what this test is intended to cover?


================
Comment at: lld/test/ELF/symver.s:15
+
+# TODO Report an duplicate definition error for foo at v1 and foo@@v1.
+# RUN: ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1.o -o /dev/null
----------------



================
Comment at: lld/test/ELF/symver.s:22
+
+# TODO foo@@v1 should resolve undefined foo at v1.
+# RUN: not ld.lld -shared --version-script=%t/ver %t/ref1p.o %t/def1.o -o /dev/null
----------------



================
Comment at: lld/test/ELF/symver.s:27
+## foo@@v1 resolves both undefined foo and foo at v1. There is one single definition.
+## Note, set soname so that the name string referenced by .gnu.version_d is fixed.
+# RUN: ld.lld -shared --soname=t1 --version-script=%t/ver %t/ref.o %t/ref1.o %t/def1.o -o %t1
----------------
(I think this is more traditional punctuation for this style of message)


================
Comment at: lld/test/ELF/symver.s:89-90
+
+## Unversioned foo is redirected. foo at v1 is not.
+# RUN: ld.lld -shared --soname=t6 --version-script=%t/ver --wrap=foo %t/ref.o %t/ref1.o %t/def1.so %t/wrap.o -o %t6
+# RUN: llvm-objdump -d --no-show-raw-insn %t6 | FileCheck %s --check-prefix=DIS6
----------------
Do you need a similar test-case for `--wrap=foo at v1` (and/or `--wrap=foo@@v1`)? There's an interesting interaction between the two things with your follow-up patch, which probably needs exploring in tests.


================
Comment at: lld/test/ELF/symver.s:93-96
+# DIS6-LABEL: <.text>:
+# DIS6-NEXT:    callq {{.*}} <__wrap_foo at plt>
+# DIS6-COUNT-3: int3
+# DIS6-NEXT:    callq {{.*}} <foo at plt>
----------------
Not that it really matters, but in all the other cases, the prefixes are `CHECK*`. Should this be `CHECK6` for consistency?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92258