Page MenuHomeFreeBSD

crunchgen: remove -Wl,-dc
ClosedPublic

Authored by fbsd-phab_maskray.me on Feb 8 2022, 10:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 4:28 AM
Unknown Object (File)
Sat, Nov 2, 2:21 AM
Unknown Object (File)
Wed, Oct 30, 5:45 PM
Unknown Object (File)
Sat, Oct 26, 4:41 PM
Unknown Object (File)
Sat, Oct 26, 4:41 PM
Unknown Object (File)
Sat, Oct 26, 4:41 PM
Unknown Object (File)
Sat, Oct 26, 4:40 PM
Unknown Object (File)
Sat, Oct 26, 4:40 PM
Subscribers

Details

Summary

In GNU ld and ld.lld, -dc is used with -r to allocate space to COMMON symbols.
It is presumably to work around legacy code which cannot handle COMMON symbols
in relocatable output. -dc has no effect if -fno-common is used.

See https://maskray.me/blog/2022-02-06-all-about-common-symbols for detail.

ld.lld may remove -dc or make it a no-op for the 15.0.0 release.

See emaste's comment: -dc is just not needed, for both -fcommon and
-fno-common code.


Hope that someone tests this for me.

Note: -nostdlib can be removed if FreeBSD Clang picks https://reviews.llvm.org/D117388

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Needs to fix a comment.

Note that defaulting to -fno-common is a "recent" change. compared to the 25+ year history of this tool.

Thank you for your excellent blog post on the topic. AFAICT compiling with -fcommon and linking with -dc is functionally identical to compiling with -fno-common and no -dc. I believe the change in this review is good, but it will take me some time to be able to give it a try.

I think your explanation of the -dc comment is incorrect, probably because the comment isn't sufficiently clear. crunchide(1) modifies the object's symbol table (to "hide" symbols), and I suspect the technique it uses is (or, used to use prior to R10:7420b323a0148511e97e0467eec9919b2ec2c00e was) incompatible with common. IMO we might as well just remove the references to -dc entirely from the comment.

I managed to try it now:

$ cc -fcommon -c foo.c foostub.c
$ ld -r foo.o foostub.o -o foo.combined.o
$ crunchide -k foo_main foo.combined.o
$ readelf -s foo.combined.o
Symbol table (.symtab) contains 13 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS _$$hide$$ foo.combined.o foo.c
     2: 0000000000000000     0 SECTION LOCAL  DEFAULT    1 _$$hide$$ foo.combined.o 
     3: 0000000000000000     0 SECTION LOCAL  DEFAULT    5 _$$hide$$ foo.combined.o 
     4: 0000000000000000     0 SECTION LOCAL  DEFAULT    7 _$$hide$$ foo.combined.o 
     5: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS _$$hide$$ foo.combined.o foostub.c
     6: 0000000000000000     0 SECTION LOCAL  DEFAULT    3 _$$hide$$ foo.combined.o 
     7: 0000000000000000     0 SECTION LOCAL  DEFAULT    4 _$$hide$$ foo.combined.o 
     8: 0000000000000000    37 FUNC    GLOBAL DEFAULT    1 _$$hide$$ foo.combined.o fn
     9: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND printf
    10: 0000000000000030    31 FUNC    GLOBAL DEFAULT    1 _$$hide$$ foo.combined.o main
    11: 0000000000000004     4 OBJECT  GLOBAL DEFAULT  COM _$$hide$$ foo.combined.o i
    12: 0000000000000050    33 FUNC    GLOBAL DEFAULT    1 foo_main

Note that the COM symbol is mangled and I suspect that the crunch mechanism would actually work just fine now without -dc regardless of whether -fcommon or -fno-common is used or default.

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)
fbsd-phab_maskray.me removed a subscriber: dim.

remove -dc from comment

LGTM. I want to build w/ this change and confirm still.

Double-checking, Fangrui Song <i@maskray.me> is the correct author info?

Double-checking, Fangrui Song <i@maskray.me> is the correct author info?

Yes.

Looking more at R10:7420b323a0148511e97e0467eec9919b2ec2c00e confirms the above - crunchide previously operated by making the "hidden" symbols local, which was incompatible with common. -dc was unnecessary after that change.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 9 2022, 6:55 PM
This revision was automatically updated to reflect the committed changes.