Respect CPPFLAGS when generating stdio-lim.h
diff mbox series

Message ID 20190318092058.5531-1-palmer@sifive.com
State New
Headers show
Series
  • Respect CPPFLAGS when generating stdio-lim.h
Related show

Commit Message

Palmer Dabbelt March 18, 2019, 9:20 a.m. UTC
The RISC-V port expects that the C compiler provides a handful of
definitions that determine the ABI to build for, but some users go
through the glibc build process before there is a suitable cross
compiler and therefor are compiling with some arbitrary complier.  Since
we only look at the C preprocessor during the headers build process this
should be fine as long as users set CPPFLAGS on their own.

Unfortunately, stdio-lim.h isn't respecting CPPFLAGS and instead uses
CPPUNDEFS, a variable that isn't standard.  This variable is meant to
undefine _FORTIFY_SOURCE, and overriding it to also add definitions
seems ugly.  Instead I think the cleaner fix is to make the stdio-lib.h
generation process respect CPPFLAGS like the rest of the build process
does.

I'm checking via build-many-glibc.py to make sure this builds, but as
that never sets CPPFLAGS I don't think it'll find any potential issues.

        * Makerules (stdio-lim.h): Add $(CPPFLAGS) to CC invocation.
---
 Makerules | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andreas Schwab March 18, 2019, 10:52 a.m. UTC | #1
On Mär 18 2019, Palmer Dabbelt <palmer@sifive.com> wrote:

> diff --git a/Makerules b/Makerules
> index 83bdd3a44d0d..a81395b4b62e 100644
> --- a/Makerules
> +++ b/Makerules
> @@ -1423,7 +1423,7 @@ $(stdio_lim:h=st): $(..)stdio-common/stdio_lim.h.in $(..)Rules \
>  	{ echo '#include "$(..)posix/bits/posix1_lim.h"';		\
>  	} |								\
>  	$(CC) -E -dM -MD -MP -MF $(@:st=dT) -MT '$(@:st=h) $(@:st=d)' 	\
> -	      $(CPPUNDEFS) $(+includes) -xc - -o $(@:st=hT)
> +	      $(CPPUNDEFS) $(CPPFLAGS) $(+includes) -xc - -o $(@:st=hT)

CPPFLAGS already includes $(CPPUNDEFS), so there is no need to use both.

Andreas.
Palmer Dabbelt March 18, 2019, 11:06 a.m. UTC | #2
On Mon, 18 Mar 2019 03:52:53 PDT (-0700), schwab@suse.de wrote:
> On Mär 18 2019, Palmer Dabbelt <palmer@sifive.com> wrote:
>
>> diff --git a/Makerules b/Makerules
>> index 83bdd3a44d0d..a81395b4b62e 100644
>> --- a/Makerules
>> +++ b/Makerules
>> @@ -1423,7 +1423,7 @@ $(stdio_lim:h=st): $(..)stdio-common/stdio_lim.h.in $(..)Rules \
>>  	{ echo '#include "$(..)posix/bits/posix1_lim.h"';		\
>>  	} |								\
>>  	$(CC) -E -dM -MD -MP -MF $(@:st=dT) -MT '$(@:st=h) $(@:st=d)' 	\
>> -	      $(CPPUNDEFS) $(+includes) -xc - -o $(@:st=hT)
>> +	      $(CPPUNDEFS) $(CPPFLAGS) $(+includes) -xc - -o $(@:st=hT)
>
> CPPFLAGS already includes $(CPPUNDEFS), so there is no need to use both.

Thanks.  How does this look?

commit d9a1f56c29d7b18930ff6a75f43d52c7c3230546
gpg: Signature made Mon 18 Mar 2019 03:55:36 AM PDT
gpg:                using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41
gpg:                issuer "palmer@dabbelt.com"
gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate]
gpg:                 aka "Palmer Dabbelt <palmer@sifive.com>" [ultimate]
Author: Palmer Dabbelt <palmer@sifive.com>
Date:   Mon Mar 18 01:24:37 2019 -0700

    Respect CPPFLAGS when generating stdio-lim.h

    The RISC-V port expects that the C compiler provides a handful of
    definitions that determine the ABI to build for, but some users go
    through the glibc build process before there is a suitable cross
    compiler and therefor are compiling with some arbitrary complier.  Since
    we only look at the C preprocessor during the headers build process this
    should be fine as long as users set CPPFLAGS on their own.

    Unfortunately, stdio-lim.h isn't respecting CPPFLAGS and instead uses
    CPPUNDEFS, a variable that isn't standard.  This variable is meant to
    undefine _FORTIFY_SOURCE, and overriding it to also add definitions
    seems ugly.  Instead I think the cleaner fix is to make the stdio-lib.h
    generation process respect CPPFLAGS like the rest of the build process
    does.

    I'm checking via build-many-glibc.py to make sure this builds, but as
    that never sets CPPFLAGS I don't think it'll find any potential issues.

            * Makerules (stdio-lim.h): Use $(CPPFLAGS) instead of
            $(CPPUNDEFS) when invoking $(CC).

diff --git a/Makerules b/Makerules
index 83bdd3a44d0d..6d138eed613a 100644
--- a/Makerules
+++ b/Makerules
@@ -1423,7 +1423,7 @@ $(stdio_lim:h=st): $(..)stdio-common/stdio_lim.h.in $(..)Rules \
 	{ echo '#include "$(..)posix/bits/posix1_lim.h"';		\
 	} |								\
 	$(CC) -E -dM -MD -MP -MF $(@:st=dT) -MT '$(@:st=h) $(@:st=d)' 	\
-	      $(CPPUNDEFS) $(+includes) -xc - -o $(@:st=hT)
+	      $(CPPFLAGS) $(+includes) -xc - -o $(@:st=hT)
 	sed $(sed-remove-objpfx) $(sed-remove-dotdot)			\
 	    $(@:st=dT) > $(@:st=dt)
 	mv -f $(@:st=dt) $(@:st=d)

I'm having some trouble running build-many-glibcs.py, but that's par for the 
course on my end.
Andreas Schwab March 18, 2019, 11:47 a.m. UTC | #3
On Mär 18 2019, Palmer Dabbelt <palmer@sifive.com> wrote:

>    I'm checking via build-many-glibc.py to make sure this builds, but as
>    that never sets CPPFLAGS I don't think it'll find any potential issues.

Since CPPFLAGS is always set by our makefiles I don't understand this
comment.

Andreas.
Palmer Dabbelt March 18, 2019, 11:53 a.m. UTC | #4
On Mon, 18 Mar 2019 04:47:47 PDT (-0700), schwab@suse.de wrote:
> On Mär 18 2019, Palmer Dabbelt <palmer@sifive.com> wrote:
>
>>    I'm checking via build-many-glibc.py to make sure this builds, but as
>>    that never sets CPPFLAGS I don't think it'll find any potential issues.
>
> Since CPPFLAGS is always set by our makefiles I don't understand this
> comment.

My point is that bulid-many-glibcs.py isn't adding any additional CPPFLAGS, so 
we wouldn't notice anything that might crop up because of how users (who 
may have set some odd CPPFLAGS) build their glibc.

Maybe that's being too paranoid, though?
Andreas Schwab March 18, 2019, 12:11 p.m. UTC | #5
On Mär 18 2019, Palmer Dabbelt <palmer@sifive.com> wrote:

> My point is that bulid-many-glibcs.py isn't adding any additional
> CPPFLAGS, so we wouldn't notice anything that might crop up because of how
> users (who may have set some odd CPPFLAGS) build their glibc.
>
> Maybe that's being too paranoid, though?

Users setting CPPFLAGS need to be careful anyway.

Andreas.
Palmer Dabbelt March 18, 2019, 12:51 p.m. UTC | #6
On Mon, 18 Mar 2019 05:11:26 PDT (-0700), schwab@suse.de wrote:
> On Mär 18 2019, Palmer Dabbelt <palmer@sifive.com> wrote:
>
>> My point is that bulid-many-glibcs.py isn't adding any additional
>> CPPFLAGS, so we wouldn't notice anything that might crop up because of how
>> users (who may have set some odd CPPFLAGS) build their glibc.
>>
>> Maybe that's being too paranoid, though?
>
> Users setting CPPFLAGS need to be careful anyway.

OK, makes sense.  I'll figure out what's going on with build-many-glibcs.py.

Thanks!

Patch
diff mbox series

diff --git a/Makerules b/Makerules
index 83bdd3a44d0d..a81395b4b62e 100644
--- a/Makerules
+++ b/Makerules
@@ -1423,7 +1423,7 @@  $(stdio_lim:h=st): $(..)stdio-common/stdio_lim.h.in $(..)Rules \
 	{ echo '#include "$(..)posix/bits/posix1_lim.h"';		\
 	} |								\
 	$(CC) -E -dM -MD -MP -MF $(@:st=dT) -MT '$(@:st=h) $(@:st=d)' 	\
-	      $(CPPUNDEFS) $(+includes) -xc - -o $(@:st=hT)
+	      $(CPPUNDEFS) $(CPPFLAGS) $(+includes) -xc - -o $(@:st=hT)
 	sed $(sed-remove-objpfx) $(sed-remove-dotdot)			\
 	    $(@:st=dT) > $(@:st=dt)
 	mv -f $(@:st=dt) $(@:st=d)