clang: Support building with GCC and DEBUG_FILES disabled
ClosedPublic
Actions

Authored by jrtc27 on Aug 13 2021, 5:57 PM.

Details

Reviewers

dim
imp
sjg
emaste
bdrewery

Commits

rGf28517db4ed0: clang: Support building with GCC and DEBUG_FILES disabled
rG523f2486d54a: clang: Support building with GCC and DEBUG_FILES disabled
rGc8edd0542647: clang: Support building with GCC and DEBUG_FILES disabled

Summary

If MK_DEBUG_FILES=no then the Clang link rule has clang as .TARGET,
rather than clang.full, causing the implicit ${CFLAGS.${.TARGET:T}} to
be CFLAGS.clang, and thus pull in flags intended for when your compiler
is Clang, not when linking Clang itself. This doesn't matter if your
compiler is in fact Clang, but it breaks using GCC as, for example,
bsd.sys.mk adds -Qunused-arguments to CFLAGS.clang. This is seen when
trying to build a bootstrap toolchain on Linux where GCC is the system
compiler.

Thus, introduce a new internal NO_TARGET_FLAGS variable that is set by
Clang to disable the addition of these implicit flags. This is a bigger
hammer than necessary, as flags for .o files would be safe, but that is
not needed for Clang.

Note that the same problem does not arise for LDFLAGS when building LLD
with BFD, since our build produces a program called ld.lld, not plain
lld (unlike upstream, where ld.lld is a symlink to lld so they can
support multiple different flavours in one binary).

Suggested by: sjg
Fixes: 31ba4ce8898f ("Allow bootstrapping llvm-tblgen on macOS and Linux")
MFC after: 1 week

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jrtc27 created this revision.Aug 13 2021, 5:57 PM

Herald added subscribers: emaste, bdrewery. · View Herald TranscriptAug 13 2021, 5:57 PM

jrtc27 requested review of this revision.Aug 13 2021, 5:57 PM

Harbormaster completed remote builds in B41027: Diff 93654.Aug 13 2021, 5:57 PM

emaste added inline comments.Aug 13 2021, 6:09 PM

share/mk/bsd.sys.mk
277	I might expand this comment slightly, it's really a bug (but one that's probably too awkward to fix) that we put the compiler type and the target in the same namespace.

jrtc27 added inline comments.Aug 13 2021, 6:10 PM

share/mk/bsd.sys.mk
277	Ack. I also note it's stale from when I thought I'd need it for LLD too...

Reword comment to drop stale mention of lld and add that this is a workaround for past regrettable design decisions

Harbormaster completed remote builds in B41029: Diff 93657.Aug 13 2021, 6:18 PM

jrtc27 marked an inline comment as done.Aug 13 2021, 6:19 PM

Thanks, LGTM

This revision is now accepted and ready to land.Aug 13 2021, 6:20 PM

causing the implicit ${CFLAGS.${.TARGET:T}} to be CFLAGS.clang, and thus pull in flags intended for when your compiler is Clang, not when linking Clang itself

Whoa, that's a nasty unintended side-effect! To me the CFLAGS.compiler-name trick always looked handy, but a little iffy, and now I realize why. :)

But if this works around it, then it's fine. We won't have to do this for CFLAGS.gcc either, since we're (hopefully) not building gcc!

Yeah, we could have done something like CFLAGS.compiler.clang, CFLAGS.target.clang, CFLAGS.src.foo.o, but practically speaking the conflict will only arise from tool chain component and target, and moving one of them out would have have avoided this.

In D31532#710873, @emaste wrote:

Yeah, we could have done something like CFLAGS.compiler.clang, CFLAGS.target.clang, CFLAGS.src.foo.o, but practically speaking the conflict will only arise from tool chain component and target, and moving one of them out would have have avoided this.

In theory you could have a target that itself is a source of something else and have some weird overlap too... :)

jrtc27 added a reviewer: bdrewery.Aug 18 2021, 1:30 PM

imp added inline comments.Aug 18 2021, 2:58 PM