Page MenuHomeFreeBSD

Limit compressed debug to little-endian targets
ClosedPublic

Authored by emaste on Aug 7 2021, 2:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 8 2024, 10:14 PM
Unknown Object (File)
Feb 8 2024, 9:26 PM
Unknown Object (File)
Jan 17 2024, 3:42 PM
Unknown Object (File)
Dec 23 2023, 3:01 PM
Unknown Object (File)
Dec 23 2023, 9:52 AM
Unknown Object (File)
Dec 12 2023, 12:50 PM
Unknown Object (File)
Nov 1 2023, 4:28 PM
Unknown Object (File)
Oct 28 2023, 12:06 AM
Subscribers

Details

Summary
Older versions of LLD fail with big-endian compressed debug sections.
This was fixed in LLD upstream by c6ebc651b6fa and merged to FreeBSD
main in d69d07569ee2 by dim, but external toolchains will not yet have
the fix.  Limit to little-endian targets for now.

PR:             257638
Sponsored by:   The FreeBSD Foundation

Diff Detail

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

Event Timeline

emaste requested review of this revision.Aug 7 2021, 2:49 PM
share/mk/bsd.compiler.mk
244

Hm, I feel like this should be in both COMPILER_FEATURES and LINKER_FEATURES, and both must support it. Then you can leave this as-is and guard the LINKER_FEATURES by (LE || !lld || version > ...)?

Are there implications here for ports that link against static system libraries?

Are there implications here for ports that link against static system libraries?

AFAIK all non-obsolete GNU toolchains support compressed debug.

Perhaps this is an argument for leaving this disabled on BE (including in .o and .a) for the time being, as BE targets will continue to use llvm packages with broken lld for some time.

share/mk/bsd.compiler.mk
244

That's correct, although I'm not sure it's worth the added complexity in light of your point that having compressed debug in base system .a archives may pose a problem even if the base system linker is fixed. My preference is to go with this not-completely-correct change for now and just turn it on unconditionally once lld 13 is widely deployed.

imp added inline comments.
share/mk/bsd.compiler.mk
244

Do you need a generic kill switch for this? Otherwise it looks good to me. I agreed with both the pedantry and the belief that making it more complicated wouldn't buy much in terms of added functionality to 'pay' for that 'complexity'.

share/mk/bsd.compiler.mk
244

Are you suggesting something like WITH_/WITHOUT_COMPRESSED_DEBUG? I'm not really sure it's necessary, and setting DEBUG_FILES_CFLAGS?= -g should work to disable it if desired.

A WITH_/WITHOUT_ build knob would probably entail including src.opts.mk into bsd.sys.mk (where DEBUG_FILES_CFLAGS is set).

share/mk/bsd.compiler.mk
244

Including src.opt.mk anywhere in the bsd.*.mk files is likely bad... which was why I was asking if we needed one or not... I'm thinking not since there's ways to work around this, as you say, even if they aren't the most convenient... and the need would likely be niche enough to not give enough benefit to having a full WITHOUT_foo flag.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 8 2021, 5:36 PM
This revision was automatically updated to reflect the committed changes.
share/mk/bsd.compiler.mk
238–244

Shouldn't all of these COMPILER_FEATURES depend on a compiler version? If I use an external clang is it guaranteed to have retpoline, init-all, and compressed-debug? I might be forgetting or missing something here that avoids external toolchains getting to this point.

share/mk/bsd.compiler.mk
238–244

retpoline was in 7.0 and backported to 6.0 and 5.0. init-all looks like it was added for 8.0 (and by default is unused, you need one of the WITH_INIT_ALL_* options enabled). compressed-debug looks like it was added around 3.5.

So technically yes there are missing version checks, but in practice there's basically no use case for many-years-old versions of Clang being used to build FreeBSD, especially when some of them, like retpoline, are annoying to check for and also really should just be required these days.

share/mk/bsd.compiler.mk
238–244

True. Works for me.

Yeah, for the ones I introduced the feature was already available in all relevant Clang versions so did not bother with a version check. Assuming this is now also true for GCC we might want to simplify things by removing the test and special cases.

Yeah, for the ones I introduced the feature was already available in all relevant Clang versions so did not bother with a version check. Assuming this is now also true for GCC we might want to simplify things by removing the test and special cases.

We have enough special cases that would be great!

I've been slogging through sys/cdefs.h with this in mind, but since it's included by everything, it kinda begs the question of how far back do we, as a project, really support when it's just a compiler that's used....