Use @@@ instead of @@ in __sym_default

[rafael.espindola_gmail.com]

Summary

Using

.symver foo,foo@@VER

causes foo and foo@@VER to be output to the .o file. This requires foo to be weak since the linker handles foo@@VER as foo.

Using

.symver foo,foo@@@VER

causes just foo@@ver to be output and avoid the need for making foo weak.

It also reduces the constraint on how exactly a linker has to handle foo and foo@@VER being present.

[rafael.espindola_gmail.com]

For what it is worth I was able to buildword and install that. The libc seems to have the correct versions:

3087: 000000000004e8e0   203 FUNC    WEAK   DEFAULT   11 openat@FBSD_1.1 (3)          
3088: 000000000004e8e0   203 FUNC    GLOBAL DEFAULT   11 openat@@FBSD_1.2 (4)

[emaste]

Do you have a doc reference for @@@?

I've applied this to my WIP tree and will test with this change.

[emaste]

Do you have a doc reference for @@@?

https://sourceware.org/binutils/docs/as/Symver.html

[kib]

lib/libc/sys/setcontext.c

45 ↗

(On Diff #30942)

Several (most) syscall symbols are exported from libc as weak on purpose. This is explained because our rtld is not quite compliant and have a feature where non-weak definition is preferred over a weak symbols. Some important implementation details in our C runtime depend on this.

I prefer to not remove these pragma weaks, to keep the exports weak.

[rafael.espindola_gmail.com]

Keep #pragma weak.

I had assumed it was there just to avoid a duplicated symbol error during link.

[rafael.espindola_gmail.com]

What is the next step?

[emaste]

I have a test universe build running and will commit the change once that finishes.

[rafael.espindola_gmail.com]

In D11653#241651, @emaste wrote:

I have a test universe build running and will commit the change once that finishes.

Awesome, thanks!

[emaste]

With this patch my build fails in some applications that link against libc.a.

Failure:

--- all_subdir_rescue ---
/tank/emaste/obj/tank/emaste/src/wipbsd/tmp/usr/bin/ld: error: undefined symbol: openat
>>> referenced by /tank/emaste/src/wipbsd/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c
>>>               libzfs_import.o:(zpool_open_func) in archive /tank/emaste/obj/tank/emaste/src/wipbsd/tmp/usr/lib/libzfs.a

symbol table from svn head openat.o:

Symbol table '.symtab' contains 10 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS /tank/emaste/src/freebsd-
     2: 0000000000000000     0 SECTION LOCAL  DEFAULT    2 
     3: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 __impl_openat
     4: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN  UND __libc_interposing
     5: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __stack_chk_fail
     6: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __stack_chk_guard
     7: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 openat
     8: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 openat@@FBSD_1.2
     9: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 openat@FBSD_1.1

symbol table from openat.o from my patched branch:

Symbol table '.symtab' contains 9 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS /tank/emaste/src/wipbsd/l
     2: 0000000000000000     0 SECTION LOCAL  DEFAULT    2 
     3: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 __impl_openat
     4: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN  UND __libc_interposing
     5: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __stack_chk_fail
     6: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __stack_chk_guard
     7: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 openat@@FBSD_1.2
     8: 0000000000000000   199 FUNC    WEAK   DEFAULT    2 openat@FBSD_1.1

[emaste]

Although I wonder if we shouldn't arrange the build & sys/cdefs.h to activate all of the .symver machinery only for the objects that will be used by the shared lib, excluding it all from libc.a?

[rafael.espindola_gmail.com]

In D11653#241670, @emaste wrote:

Although I wonder if we shouldn't arrange the build & sys/cdefs.h to activate all of the .symver machinery only for the objects that will be used by the shared lib, excluding it all from libc.a?

It would be cleaner, but I was expecting this to still work for both. How do I reproduce the failure?

[emaste]

Oh, wait -- this failure was observed with the FreeBSD clang500-import branch, which has llvm/clang/lld/lldb/compiler-rt/libc++ at upstream r308421.

[emaste]

After merging LLVM r308492 to my wip branch (based on FreeBSD's clang500-import branch) the buildworld/buildkernel with this change is successful. If you can merge r308492 to the 5.0 branch we'll bring it into FreeBSD-HEAD from there, and could apply this change later.

For reference, my test branch can be seen here: https://github.com/emaste/freebsd/tree/wipbsd-12

[rafael.espindola_gmail.com]

r308492 was merged into the 5.0 branch in r308788.

[emaste]

5.0 or later is now in HEAD and 11.x. I intend to commit this to HEAD shortly.

[cem]

It looks like this was accidentally committed with the #pragma weaks removed? As a result, interposing openat (and setcontext, swapcontext) is broken. Or were they removed again on purpose?

We have some unit tests that interpose openat and were broken by this change. The straightforward solution seems to be restoring the #pragma weaks. Is there any reason not to? Thanks!

[kib]

In D11653#540521, @cem wrote:

It looks like this was accidentally committed with the #pragma weaks removed? As a result, interposing openat (and setcontext, swapcontext) is broken. Or were they removed again on purpose?

We have some unit tests that interpose openat and were broken by this change. The straightforward solution seems to be restoring the #pragma weaks. Is there any reason not to? Thanks!

weak should be restored.