[lld][test] Expand testing for dynamic-list and export-dynamic

[gbreynoo]

Summary

• Expanded testing for export-dynamic and dynamic list
• Added cases that have no matching symbol
• Added uses of fatal-warnings to ensure warnings are not output unexpectedly
• Rename dynamic-list-wildcard.s to dynamic-list-patterns.s and expanded the test
• Added export-symbol-types.s
• Fixed invalid-dynamic-list, previously the same list was used in each case. When looking at this test on I found that the 5th case was failing to echo successfully leading to the previous case being tested again.
• Allow invalid-dynamic-list to run on Windows after removing the use of echo with quoted strings.

[MaskRay]

Interesting. Oh, did you know that I have an ongoing discussion with binutils to refine the semantics of --export-dynamic-symbol --dynamic-symbol -Bsymbolic :) https://sourceware.org/pipermail/binutils/2020-May/111019.html

[grimar]

Generally LG. Few comments below.

lld/test/ELF/export-dynamic-symbol.s

8 ↗

(On Diff #265273)

##

9 ↗

(On Diff #265273)

A double space after --fatal-warnings (and below).

lld/test/ELF/export-symbol-types.s

1 ↗

(On Diff #265273)

It probably missing a description of what this test case generally checks.

17 ↗

(On Diff #265273)

export-symbol-types.s is short. Could you use echo instead?

21 ↗

(On Diff #265273)

##

Also, full stop is missing.

23 ↗

(On Diff #265273)

This empty line looks excessive.

[gbreynoo]

Changes based on grimars comments:

• Corrected white space and comments
• Replaced the use of input file "Inputs/export-symbol-types.s" with use of echo

[gbreynoo]

Thanks MaskRay, I was not aware of that.

[MaskRay]

lld/test/ELF/dynamic-list-patterns.s

7 ↗

(On Diff #265509)

-unknown-linux can be dropped (this tests generic ELF behavior)

9 ↗

(On Diff #265509)

Use ' for outermost quotes

11 ↗

(On Diff #265509)

--dyn-syms

You can also use llvm-nm -D

lld/test/ELF/export-symbol-types.s

31 ↗

(On Diff #265509)

--dyn-syms

32 ↗

(On Diff #265509)

Can you find existing tests which can be deleted after you add the more comprehensive test here?

[gbreynoo]

Changes based on MaskRay's comments:

• Replace use of --dyn-symbols with --dynsyms
• Remove use of -unknown-linux
• Correct use of quotes

[gbreynoo]

lld/test/ELF/dynamic-list-patterns.s

11 ↗

(On Diff #265509)

The other tests use llvm-readelf so thought it best to be consistent, also I think confirming the number of entries is easier with readelf. If llvm-nm is preferred I could update the other tests too?

lld/test/ELF/export-symbol-types.s

32 ↗

(On Diff #265509)

I had a look at the other tests for --dynamic-list and --export-dynamic and believe they each add coverage e.g. they test for a particular error message or are case when multiple symbols of the same type are exported together etc. If there is a test you have in mind I could add it to the review.

[MaskRay]

In D80311#2049142, @gbreynoo wrote:

Thanks MaskRay, I was not aware of that.

--dynamic-list is an obscure option. I just wondered how you get interested in it:)

There are fuzzy things about its semantics and relationship with -Bsymbolic, --export-dynamic, etc. That was the motivation behind my GNU ld patches.
I hope they can accept reasonable order-independent semantics.
https://sourceware.org/pipermail/binutils/2020-May/111211.html (I am happy with our our --dynamic-list semantics)
https://sourceware.org/pipermail/binutils/2020-May/111019.html (we may need to change --export-dynamic-symbol a bit)

[MaskRay]

lld/test/ELF/export-symbol-types.s

3 ↗

(On Diff #265745)

For -shared, --dynamic-list does not affect the exported symbols.

[MaskRay]

lld/test/ELF/export-symbol-types.s

4 ↗

(On Diff #265745)

I think the test should be named dynamic-list-* instead, instead of export-symbol

--dynamic-list work for both executables and shared libraries.
For an executable, export some symbols.
For a shared library, specify preemptible symbols.

We should check both executable and so cases.

[MaskRay]

Created D80487 for --export-dynamic-symbol

lld/test/ELF/Inputs/invalid-dynamic-list-1.list

1 ↗

(On Diff #265745)

echo '.........' | llvm-mc -filetype=obj -triple=x86_64 - -o %t1.o for simple tests

lld/test/ELF/export-symbol-types.s

1 ↗

(On Diff #265745)

REQUIRES: x86

The test duplicates partially with dynsym-pie.s, symbols.s and visibility.s

Check if they can be reorganized.

11 ↗

(On Diff #265745)

imported symbols -> shared symbols

18 ↗

(On Diff #265745)

echo '{ *; };' > %t.list

20 ↗

(On Diff #265745)

write on one line and separate statements with ;

24 ↗

(On Diff #265745)

Use the same basename for .o and .so if applicable

36 ↗

(On Diff #265745)

Just use CHECK-NEXT:

The diagnostic will be better if things go off. This does rely on the symbol table order and will require extra efforts when the order does change, but I think the cost pays off.

38 ↗

(On Diff #265745)

Use ABS for abs

43 ↗

(On Diff #265745)

technically this is not referenced. You may name it undef_weak (yes, the order undefined weak (instead of weak undefined) is used in the ELF spec)

[gbreynoo]

Made the following changes:

• Fixed up a comment
• Added x86 requirement
• Changed the creation of the dynamic list in export-symbol-types.s
• Made file names more consistent
• Use CHECK-NEXT
• Use ABS for abs
• rename symbol

[gbreynoo]

Fixed use of echo

[gbreynoo]

Thanks for the comments Maskray. I had some questions regarding the comments for expanding the test for use of -shared and the similarities with dynsym-pie.s, symbols.s and visibility.s. My intention with the test export-symbol-types.s was to confirm the symbols types that are placed in the dynamic symbol table with use of --export-dynamic and --dynamic-list. I considered the other tests mentioned as covering related but different functionality using similar methods. For example symbol.s being for correct symbol output as a whole, not specifically exports etc.

I can see there being a reason to expand the coverage of the existing test to cover -shared, this would require the rename and potentially splitting the existing test in two. It may also be worth creating one input assembly file that can be used for all these tests mentioned to ensure coverage. Could this wait for a future change however?

Also, with your comment were you suggesting removing some of the existing tests and folding them into the one added here? As they test different functionality I think it may be best to keep them separate.

[MaskRay]

lld/test/ELF/dynamic-list-patterns.s

1 ↗

(On Diff #267671)

dynamic-list-glob.s may be better.

The recognized patterns are globs, not regex.

3 ↗

(On Diff #267671)

Thanks for deleting dynamic-list-wildcard.s

Whether an entry is a glob depends on whether double quotes are used. It is not unconditional glob, so the sentence is not accurate.

Confirm --dynamic-list identifies symbols by patterns, including wildcards.

10 ↗

(On Diff #267671)

s/%t.elf/%t/

lld/test/ELF/export-dynamic-symbol.s

3 ↗

(On Diff #267671)

I committed D80487 which came with a bunch of tests. These cases should have been covered.

lld/test/ELF/export-symbol-types.s

9 ↗

(On Diff #267671)

undefined symbols and shared symbols are exported.

--export-dynamic, --dynamic-list and --export-dynamic-symbol can export additional symbols: non-local STV_DEFAULT symbols

The list is not accurate because there is overlap between hidden symbols and weak symbols, for example.

weak symbols are not unconditionally exported.

Many symbols generated by the linker are not exported (hidden).

24 ↗

(On Diff #267671)

Drop -shared. .so obviously identifies a shared object.

26 ↗

(On Diff #267671)

Drop -elf. It carries no additional information. All object files discussed here are ELF.

26 ↗

(On Diff #267671)

Delete llvm-mc -filetype=obj -triple=x86_64 %s -o %t-elf.o

the object file is the same

lld/test/ELF/invalid-dynamic-list.test

9–10

You can update the quotes by the way.

[MaskRay]

In D80311#2066890, @gbreynoo wrote:

Thanks for the comments Maskray. I had some questions regarding the comments for expanding the test for use of -shared and the similarities with dynsym-pie.s, symbols.s and visibility.s. My intention with the test export-symbol-types.s was to confirm the symbols types that are placed in the dynamic symbol table with use of --export-dynamic and --dynamic-list. I considered the other tests mentioned as covering related but different functionality using similar methods. For example symbol.s being for correct symbol output as a whole, not specifically exports etc.

I carefully checked. export-symbol-types.s makes sense, but the name may be confusing. symbol types refers specifically to STT_*, while in the test we are checking other properties (visibility/binding/etc).

I can see there being a reason to expand the coverage of the existing test to cover -shared, this would require the rename and potentially splitting the existing test in two. It may also be worth creating one input assembly file that can be used for all these tests mentioned to ensure coverage. Could this wait for a future change however?

Also, with your comment were you suggesting removing some of the existing tests and folding them into the one added here? As they test different functionality I think it may be best to keep them separate.

-shared tests require to check preemptibility. This can usually tested by checking dynamic relocations. You can test them with movl foo@gotpcrel(%rip), %rax
They can be added in this patch. You can add -shared tests in a future change.

[gbreynoo]

• Corrected use of pattern with entry
• Renamed dynamic-list-patterns.s
• Removed changes to export-dynamic-symbol.s
• Change input file names
• Updated quotes

[gbreynoo]

• Fixed use of %t.elf

[gbreynoo]

Thanks again MaskRay, to avoid confusion surrounding the term "type" I have renamed the test export-symbols and removed the list of symbols, a number of new uses of weak symbols have also been added to the test. I have also split dynamic-list-cpp into a new test due to the dynamic-list-glob.s name change.

Regarding the testing of shared and preemption I have seen that export-dynamic-symbol.s has testing for this regarding --export-dynamic-symbol, whilst --dynamic-list has testing in dynamic-list-preempt-replace-symbol.s and others. Whilst looking into the behaviour of --dynamic-export I struggled to see the same behaviour. Should it also be affecting shared objects in the same way?

I think testing of shared and preemption could be better expanded in similar tests as I described above. I should better describe the initial reasoning for export-symbols.s as the test is a little vague in what exactly it is covering and the misuse of the term type did not help. The idea was that this would be a test for a number of different symbols to ensure they were being exported, this didn't seem to fit into the existing tests and was not exhaustive but could be good additional coverage. Although a shared object is created in the test, this was only used to check for a symbol that found it's definition elsewhere.

lld/test/ELF/export-symbol-types.s

9 ↗

(On Diff #267671)

You are right that this list was more confusing than helpful, I have removed it.

26 ↗

(On Diff #267671)

The object file created here is different to the one above, one uses the echo as input, the other this file.

[MaskRay]

Looks great!

lld/test/ELF/export-symbols.s

30

Will be nice to make the table aligned.

[MaskRay]

FWIW binutils's --dynamic-list and --export-dynamic-symbol are updated a bit this month. The new semantics should match us. https://sourceware.org/pipermail/binutils/2020-June/

lld/test/ELF/dynamic-list-cpp.s

+# REQUIRES: x86
+
+## Confirm both mangled and unmangled names may appear in
+## the --dynamic-list file.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+
+# RUN: echo '{ _Z1fv; extern "C++" { "g()"; }; };' > %t.list
+# RUN: ld.lld -pie --dynamic-list %t.list %t.o -o %t
+# RUN: llvm-readelf --dyn-syms %t | FileCheck %s
+
+# CHECK:      Symbol table '.dynsym' contains 3 entries:
+# CHECK:      _Z1fv
+# CHECK-NEXT: _Z1gv
+
+.globl _Z1fv, _Z1gv
+_Z1fv:
+_Z1gv:

lld/test/ELF/dynamic-list-glob.s

+# REQUIRES: x86
+
+## Confirm --dynamic-list identifies symbols by entries, including wildcards.
+## Entries need not match a symbol. 
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+
+# RUN: echo '{ [fb]o?1*; };' > %t.list
+# RUN: ld.lld -pie --dynamic-list %t.list %t.o -o %t
+# RUN: llvm-readelf --dyn-syms %t | FileCheck %s
+
+# CHECK:      Symbol table '.dynsym' contains 4 entries:
+# CHECK:      boo1
+# CHECK-NEXT: foo1
+# CHECK-NEXT: foo11
+
+.globl _start, foo1, foo11, foo2, boo1
+foo1:
+foo11:
+foo2:
+boo1:
+_start:

lld/test/ELF/dynamic-list-wildcard.s

-# REQUIRES: x86
-
-# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
-
-# RUN: echo "{ foo1*; };" > %t.list
-# RUN: ld.lld -pie --dynamic-list %t.list %t -o %t
-# RUN: llvm-nm -D %t | FileCheck %s
-
-# CHECK:      foo1
-# CHECK-NEXT: foo11
-# CHECK-NOT:  {{.}}
-
-.globl _start, foo1, foo11, foo2
-foo1:
-foo11:
-foo2:
-_start:

lld/test/ELF/export-symbols.s

+# REQUIRES: x86
+## Verify that the arguments --export-dynamic and --dynamic-list
+## put the correct symbols in the dynamic symbol table.
+
+# RUN: echo "{ *; };" > %t.list
+
+# RUN: echo ".globl shared" > %t.s ; echo "shared = 0xDEADBEEF" >> %t.s
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t.s -o %t-shared.o
+# RUN: ld.lld --shared %t-shared.o -o %t.so
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+## Use --fatal-warnings to confirm no diagnostics are emitted.
+# RUN: ld.lld --fatal-warnings --defsym=defsym=_start %t.so %t.o -o %t.out
+# RUN: ld.lld --fatal-warnings --defsym=defsym=_start %t.so %t.o -o %texport.out --export-dynamic
+# RUN: ld.lld --fatal-warnings --defsym=defsym=_start %t.so %t.o -o %tlist.out --dynamic-list %t.list
+
+# RUN: llvm-readelf --dyn-syms %t.out | FileCheck %s --check-prefix=NO-EXPORT
+# RUN: llvm-readelf --dyn-syms %texport.out | FileCheck %s --check-prefix=EXPORT
+# RUN: llvm-readelf --dyn-syms %tlist.out | FileCheck %s --check-prefix=EXPORT
+
+# NO-EXPORT:      Symbol table '.dynsym' contains 3 entries:
+# NO-EXPORT:      GLOBAL DEFAULT {{.*}} shared
+# NO-EXPORT-NEXT: WEAK   DEFAULT {{.*}} undef_weak
+
+# EXPORT:      Symbol table '.dynsym' contains 8 entries:
+# EXPORT:      GLOBAL DEFAULT   {{.*}} shared
+# EXPORT-NEXT: WEAK   DEFAULT   {{.*}} undef_weak
+# EXPORT-NEXT: GLOBAL PROTECTED {{.*}} _start
+# EXPORT-NEXT: GLOBAL DEFAULT   ABS    abs
+# EXPORT-NEXT: GLOBAL DEFAULT   {{.*}} common
+# EXPORT-NEXT: WEAK   DEFAULT   {{.*}} weak_default
+# EXPORT-NEXT: GLOBAL DEFAULT   {{.*}} defsym
+
+.weak undef_weak
+
+.weak weak_default
+weak_default:
+
+.weak weak_internal
+.internal weak_internal
+weak_internal:
+
+.weak weak_hidden
+.internal weak_hidden
+weak_hidden:
+
+.weak weak_protected
+.internal weak_protected
+weak_protected:
+
+.globl shared
+
+.local local
+local:
+
+.comm common, 10
+
+.globl abs
+abs = 0xDEADBEEF
+
+.globl hidden
+.hidden hidden
+hidden:
+
+.globl _start
+.protected _start
+_start:

lld/test/ELF/invalid-dynamic-list.test

-## Different "echo" commands on Windows interpret quoted strings and
-## wildcards in similar but different way (On Windows, ARGV tokenization
-## and wildcard expansion are not done by the shell but by each command.)
-## Because of that reason, this test fails on some Windows environment.
-## We can't write quoted strings that are interpreted the same way
-## by all echo commands. So, we don't want to run this on Windows.
-
-# REQUIRES: shell
-
-# RUN: mkdir -p %t.dir
-
 # RUN: echo > %tempty.list
 # RUN: not ld.lld --dynamic-list %tempty.list 2>&1 | FileCheck --check-prefix=EMPTY %s
 # EMPTY: error: {{.*}}.list:1: unexpected EOF
 
-# RUN: echo foobar > %t1
+# RUN: echo 'foobar' > %t1
 # RUN: not ld.lld --dynamic-list %t1 2>&1 | FileCheck -check-prefix=ERR1 %s
 # ERR1: {{.*}}:1: { expected, but got foobar
 
-# RUN: echo "{ foobar;" > %t1
-# RUN: not ld.lld --dynamic-list %t1 2>&1 | FileCheck -check-prefix=ERR2 %s
+# RUN: echo '{ foobar;' > %t2
+# RUN: not ld.lld --dynamic-list %t2 2>&1 | FileCheck -check-prefix=ERR2 %s
 # ERR2: {{.*}}:1: unexpected EOF
 
 ## Missing ';' before '}'
-# RUN: echo "{ foobar }" > %t1
-# RUN: not ld.lld --dynamic-list %t1 2>&1 | FileCheck -check-prefix=ERR3 %s
+# RUN: echo '{ foobar }' > %t3
+# RUN: not ld.lld --dynamic-list %t3 2>&1 | FileCheck -check-prefix=ERR3 %s
 # ERR3: {{.*}}:1: ; expected, but got }
 
 ## Missing final ';'
-# RUN: echo "{ foobar; }" > %t1
-# RUN: not ld.lld --dynamic-list %t1 2>&1 | FileCheck -check-prefix=ERR4 %s
+# RUN: echo '{ foobar; }' > %t4
+# RUN: not ld.lld --dynamic-list %t4 2>&1 | FileCheck -check-prefix=ERR4 %s
 # ERR4: {{.*}}:1: unexpected EOF
 
-## Missing \" in foobar definition
-# RUN echo "{ \"foobar; };" > %t1
-# RUN: not ld.lld --dynamic-list %t1 2>&1 | FileCheck -check-prefix=ERR5 %s
-# ERR5: {{.*}}:1: unexpected EOF
+## Missing " in foobar definition
+# RUN: echo '{ "foobar; };' > %t5
+# RUN: not ld.lld --dynamic-list %t5 2>&1 | FileCheck -check-prefix=ERR5 %s
+# ERR5: {{.*}}:1: unclosed quote
 
-# RUN: echo "{ extern \"BOGUS\" { test }; };" > %t1
-# RUN: not ld.lld --dynamic-list %t1 2>&1 | FileCheck -check-prefix=ERR6 %s
+# RUN: echo '{ extern "BOGUS" { test }; };' > %t6
+# RUN: not ld.lld --dynamic-list %t6 2>&1 | FileCheck -check-prefix=ERR6 %s
 # ERR6: {{.*}}:1: Unknown language