Page MenuHomeFreeBSD

Re-add ALLOW_MIPS_SHARED_TEXTREL, sprinkle it around
ClosedPublic

Authored by kevans on Sep 18 2019, 2:11 AM.

Details

Summary

Diff partially stolen from CheriBSD; these bits need -Wl,-z,notext in order to build in an LLVM world. They are needed for all flavors/sizes of MIPS. This will eventually get fixed in LLVM, but it's unclear when.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 26542

Event Timeline

kevans created this revision.Sep 18 2019, 2:11 AM
dim added inline comments.Sep 18 2019, 6:02 AM
share/mk/bsd.lib.mk
286

Maybe rename this to ALLOW_MIPS_SHARED_TEXTREL or something, if it really only applies to mips?

290

I wonder if there is a way to really "fix" this, in the sense that we could get clang and lld to stop attempting to make those relocations? :)

297

Looking at this, I think that you should simply check if ALLOW_SHARED_TEXTREL is defined or not, and stop using the "yes" and "no" values, as this is not a regular MK_FOO knob.

Otherwise, you would have to check for:

.if !defined(ALLOW_SHARED_TEXTREL) || ${ALLOW_SHARED_TEXTREL} != "yes"

instead.

301

Maybe best to decouple this from the -z,notext handling, as people might still want to have linker warnings on mips.

I'll address the rest of this later this morning.

share/mk/bsd.lib.mk
290

Simon plans to fix it, but he was not able to get me definitive timeline -- which is OK, since he does so much. =)

kevans updated this revision to Diff 62533.Wed, Sep 25, 2:27 AM
kevans marked 3 inline comments as done.
kevans retitled this revision from Re-add ALLOW_SHARED_TEXTREL, sprinkle it around to Re-add ALLOW_MIPS_SHARED_TEXTREL, sprinkle it around.

Rename to ALLOW_MIPS_SHARED_TEXTREL and clean up the bsd.lib.mk segment quite a bit. Stop with silly yes/no, check for defined and only do something if mips.

Looks good. Although I'm not sure if we want this to be conditional on clang/lld since modern GCC+bfd should work (and maybe even clang+new bfd).

Looks good. Although I'm not sure if we want this to be conditional on clang/lld since modern GCC+bfd should work (and maybe even clang+new bfd).

IIRC clang+new bfd still fails as bfd checks object files and catches the relocation in read-only segment. I'll retest that, though, because my memory is dicy here.

emaste added inline comments.Fri, Sep 27, 1:04 PM
share/mk/bsd.lib.mk
285

why yes here and empty string elsewhere?

309–311

Can we extend this comment slightly to indicate the conditions for eventual revert? Presumably a later version of lld would address this?

kevans updated this revision to Diff 62847.Wed, Oct 2, 5:53 PM
kevans marked an inline comment as done.
  • One last instance of "yes" where the value doesn't matter has been fixed.
  • Add enough explanation to make it clear that this won't spontaneously go away due to work in the FreeBSD tree, but rather from clang/lld revisions.
emaste accepted this revision.Wed, Oct 2, 6:23 PM

looks ok to me

This revision is now accepted and ready to land.Wed, Oct 2, 6:23 PM
arichardson accepted this revision.Wed, Oct 2, 6:57 PM
This revision was automatically updated to reflect the committed changes.