We build a number of build tools using the host's toolchain, and the host may not be FreeBSD. Add a relro linker feature, set for lld and bfd, and add -zrelro or -znorelro only if the feature exists. This fixes cross-build from MacOS.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
This one is a bit of a special case compared to e.g. -znow, because my change always passes either -zrelro or -znorelro as different linkers have different defaults.
| share/mk/bsd.linker.mk | ||
|---|---|---|
| 18 ↗ | (On Diff #107348) | will resort in alpha order |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 86 | Do we want something akin to the warning below for retpoline? It doesn't seem nice to silently ignore a build configuration. | |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 89 | If the linker doesn't support RELRO, then we print this warning unconditionally. For retpoline, the warning is printed only if a non-default configuration is specified. The downside of that is that the default value of the configuration variable is hard-coded in two places, so I'm not sure what the best solution is. At least I think the warning should directly reference the name of the WITH_ variable. | |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 89 | This is why I didn't include a warning originally, because there are various combinations of the option being on/off and the linker's default being on/off. WITH_RELRO is the default option and relro is on by default in lld, so maybe the warning should only be emitted in the case that MK_RELRO == no and the linker doesn't support it. | |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 89 |
Yeah, that's what we do below for retpoline. I think. It's just a bit ugly since we end up hard-coding the fact that MK_RELRO != no is the default. | |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 89 | Ideally what you'd do is not necessarily hardcode the default, but only warn when the linker's behavior doesn't match the requested behavior, but that seems messy. Either we assume that all linkers that don't support the option use a specific behavior (and then make the warning conditional that way), or we have more complicated gymnastics to figure out the default based on the linker version, etc. and then warn on a mismatch. Do we know what macOS's native linker does btw? | |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 89 | I am not really sure how the macho format works; I believe relro is ELF specific but perhaps macho has something similar. It's complicated by the fact that bfd and lld have opposite defaults, but both support the option. Either way I think it doesn't matter much for the build tools, and I do not want to overcomplicate the warning. What about just leaving the logic as is but making it "does not support WITH_RELRO / WITHOUT_RELRO"? | |
Comment Actions
complicated gymnastics to figure out the default based on the linker version
yes, we could add a relro-default linker feature that is set for lld, and then do something like
.if ${MK_RELRO} != "no"
.if ${LINKER_FEATURES:Mrelro-default} == ""
.if ${LINKER_FEATURES:Mrelro}
LDFLAGS+= -Wl,-zrelro
.else
.warning Linker does not support WITH_RELRO, option ignored
.endif
.endif
.else
.if ${LINKER_FEATURES:Mrelro-default}
.if ${LINKER_FEATURES:Mrelro}
LDFLAGS+= -Wl,-znorelro
.else
.warning Linker does not support WITHOUT_RELRO, option ignored
.endif
.endif
but IMO this complexity is unwarrantedComment Actions
Seems fine to me. Alternatively we could add a "gnu-like-arguments" (or some better name) feature for bfd compatible syntax.
| share/mk/bsd.prog.mk | ||
|---|---|---|
| 41 | Another option would be to put all the -z options inside a LINKER_TYPE != macos block, I guess the the only reason we aren't seeing the above line fail is that bind_now is off by default (at least for bootstrap tools). | |