On Tue, Dec 10, 2019 at 04:30:33PM +0000, Ed Maste wrote: > From: Ed Maste <emaste@xxxxxxxxxxx> > > When Valgrind is not available NO_VALGRIND is set in CFLAGS, and this > is needed during dependency generation as well as compilation. Sorry I've taken so long to respond to this. This is.. probably not exactly the right fix, but how to do it right is trickier than you'd think. In the end I've applied it, because it does fix a real problem and doesn't cause any real harm. The complications arise because we make a distinction between flags for the preprocessor, $(CPPFLAGS), and for the compiler proper, $(CFLAGS). We were putting -DNO_VALGRIND in CFLAGS, rather than CPPFLAGS, which is definitely wrong, since it sets a preproc directive. I've committed a patch to fix that as well. That change alone would suffice for your specific problem. However, when valgrind *is* enabled we're using pkgconfig --cflags, and we don't know what parts of its output belong in CPPFLAGS and which in CFLAGS. The output of pkg-config --cflags-only-I - which on my system is actually everything - definitely belongs in CPPFLAGS. But if there were other flags there, we don't know where to file them. But then thinking about that, I'm wondering why we even make the CFLAGS/CPPFLAGS distinction in the first place. We omit CFLAGS in the %.i targets (preprocessed C files, for debugging) - but I'm pretty sure including non preprocessor flags here would be harmless, if unnecessary. More significantly we leave them out when building from assembly files. That is I think actually necessary. But the only asm file we actually build is tests/trees.S, which is a really weird special case for asm anyway (there are no opcodes, only data-emitting pseudo-ops). We could probably special case the cflags thing here as well. In fact, I'd really kind of like to get rid of trees.S, (ab)using the assembler in this way has caused hassles from time to time. Unfortunately, I haven't yet come up with convenient alternative approach to building the odd testcases its used for. > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 4d88157..da6cf92 100644 > --- a/Makefile > +++ b/Makefile > @@ -344,7 +344,7 @@ clean: libfdt_clean pylibfdt_clean tests_clean > > %.d: %.c > @$(VECHO) DEP $< > - $(CC) $(CPPFLAGS) -MM -MG -MT "$*.o $@" $< > $@ > + $(CC) $(CPPFLAGS) $(CFLAGS) -MM -MG -MT "$*.o $@" $< > $@ > > %.d: %.S > @$(VECHO) DEP $< -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
