diff mbox

kbuild: Use -nostdinc in compile tests

Message ID 1415235534.3398.35.camel@decadent.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Hutchings Nov. 6, 2014, 12:58 a.m. UTC
gcc 4.8 and later include <stdc-predef.h> by default.  In some
versions of eglibc that includes <bits/predefs.h>, but that may be
missing when building with a biarch compiler.  Also <stdc-predef.h>
itself could be missing as we are only trying to build a kernel, not
userland.

The -nostdinc option disables this, though it isn't explicitly
documented.  This option is already used when actually building
the kernel, but not by cc-option and other tests.  This can result
in silently miscompiling the kernel.

References: https://bugs.debian.org/717557
References: https://bugs.debian.org/726861
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
This specific issue appeared and then was fixed in Debian about a year
ago, but it seems like it can also affect cross-builds.

It may also be worth adding some sort of sanity test, so we stop
immediately if $(cc-option $(empty),broken) yields 'broken'.

Ben.

 Makefile               |  4 +++-
 scripts/Kbuild.include | 15 ++++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

Comments

Michal Marek Nov. 25, 2014, 9:53 p.m. UTC | #1
Dne 6.11.2014 v 01:58 Ben Hutchings napsal(a):
> gcc 4.8 and later include <stdc-predef.h> by default.  In some
> versions of eglibc that includes <bits/predefs.h>, but that may be
> missing when building with a biarch compiler.  Also <stdc-predef.h>
> itself could be missing as we are only trying to build a kernel, not
> userland.
> 
> The -nostdinc option disables this, though it isn't explicitly
> documented.  This option is already used when actually building
> the kernel, but not by cc-option and other tests.  This can result
> in silently miscompiling the kernel.
> 
> References: https://bugs.debian.org/717557
> References: https://bugs.debian.org/726861
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> This specific issue appeared and then was fixed in Debian about a year
> ago, but it seems like it can also affect cross-builds.
> 
> It may also be worth adding some sort of sanity test, so we stop
> immediately if $(cc-option $(empty),broken) yields 'broken'.
> 
> Ben.
> 
>  Makefile               |  4 +++-
>  scripts/Kbuild.include | 15 ++++++++-------
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ffc1ce2..c0fbf67 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -619,6 +619,8 @@ endif
>  # Tell gcc to never replace conditional load with a non-conditional one
>  KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
>  
> +NOSTDINC_FLAGS += -nostdinc
> +
>  ifdef CONFIG_READABLE_ASM
>  # Disable optimizations that make assembler listings hard to read.
>  # reorder blocks reorders the control in the function
> @@ -742,7 +744,7 @@ KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)
>  endif
>  
>  # arch Makefile may override CC so keep this after arch Makefile is included
> -NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
> +NOSTDINC_FLAGS += -isystem $(shell $(CC) -print-file-name=include)

Why not simply move the whole assignment up? Also, there are two
cc-option tests and one cc-disable-warning test before the first part
assignment. Looks like a result of a mismerge.

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek Nov. 26, 2014, 1:47 p.m. UTC | #2
On 2014-11-25 22:53, Michal Marek wrote:
> Dne 6.11.2014 v 01:58 Ben Hutchings napsal(a):
>> gcc 4.8 and later include <stdc-predef.h> by default.  In some
>> versions of eglibc that includes <bits/predefs.h>, but that may be
>> missing when building with a biarch compiler.  Also <stdc-predef.h>
>> itself could be missing as we are only trying to build a kernel, not
>> userland.
>>
>> The -nostdinc option disables this, though it isn't explicitly
>> documented.  This option is already used when actually building
>> the kernel, but not by cc-option and other tests.  This can result
>> in silently miscompiling the kernel.
>>
>> References: https://bugs.debian.org/717557
>> References: https://bugs.debian.org/726861
>> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>> ---
>> This specific issue appeared and then was fixed in Debian about a year
>> ago, but it seems like it can also affect cross-builds.
>>
>> It may also be worth adding some sort of sanity test, so we stop
>> immediately if $(cc-option $(empty),broken) yields 'broken'.
>>
>> Ben.
>>
>>  Makefile               |  4 +++-
>>  scripts/Kbuild.include | 15 ++++++++-------
>>  2 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index ffc1ce2..c0fbf67 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -619,6 +619,8 @@ endif
>>  # Tell gcc to never replace conditional load with a non-conditional one
>>  KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
>>  
>> +NOSTDINC_FLAGS += -nostdinc
>> +
>>  ifdef CONFIG_READABLE_ASM
>>  # Disable optimizations that make assembler listings hard to read.
>>  # reorder blocks reorders the control in the function
>> @@ -742,7 +744,7 @@ KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)
>>  endif
>>  
>>  # arch Makefile may override CC so keep this after arch Makefile is included
>> -NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>> +NOSTDINC_FLAGS += -isystem $(shell $(CC) -print-file-name=include)
> 
> Why not simply move the whole assignment up?

Ah, I see the reason now -- we need $(CC) for the second part of the
assignment.


> Also, there are two
> cc-option tests and one cc-disable-warning test before the first part
> assignment. Looks like a result of a mismerge.

But this still looks like a bug to me.

Michal

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Makefile b/Makefile
index ffc1ce2..c0fbf67 100644
--- a/Makefile
+++ b/Makefile
@@ -619,6 +619,8 @@  endif
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
+NOSTDINC_FLAGS += -nostdinc
+
 ifdef CONFIG_READABLE_ASM
 # Disable optimizations that make assembler listings hard to read.
 # reorder blocks reorders the control in the function
@@ -742,7 +744,7 @@  KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)
 endif
 
 # arch Makefile may override CC so keep this after arch Makefile is included
-NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
+NOSTDINC_FLAGS += -isystem $(shell $(CC) -print-file-name=include)
 CHECKFLAGS     += $(NOSTDINC_FLAGS)
 
 # warn about C99 declaration after statement
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 65e7b08..e87fc08 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -111,12 +111,12 @@  as-instr = $(call try-run,\
 # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
 
 cc-option = $(call try-run,\
-	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
+	$(CC) $(NOSTDINC_FLAGS) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
 
 # cc-option-yn
 # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
 cc-option-yn = $(call try-run,\
-	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n)
+	$(CC) $(NOSTDINC_FLAGS) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n)
 
 # cc-option-align
 # Prefix align with either -falign or -malign
@@ -126,16 +126,17 @@  cc-option-align = $(subst -functions=0,,\
 # cc-disable-warning
 # Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
 cc-disable-warning = $(call try-run,\
-	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
+	$(CC) $(NOSTDINC_FLAGS) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
 
 # cc-version
 # Usage gcc-ver := $(call cc-version)
-cc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))
+cc-version = $(shell $(CONFIG_SHELL) \
+	$(srctree)/scripts/gcc-version.sh $(CC) $(NOSTDINC_FLAGS))
 
 # cc-fullversion
 # Usage gcc-ver := $(call cc-fullversion)
 cc-fullversion = $(shell $(CONFIG_SHELL) \
-	$(srctree)/scripts/gcc-version.sh -p $(CC))
+	$(srctree)/scripts/gcc-version.sh -p $(CC) $(NOSTDINC_FLAGS))
 
 # cc-ifversion
 # Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
@@ -144,12 +145,12 @@  cc-ifversion = $(shell [ $(call cc-version, $(CC)) $(1) $(2) ] && echo $(3))
 # cc-ldoption
 # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
 cc-ldoption = $(call try-run,\
-	$(CC) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
+	$(CC) $(NOSTDINC_FLAGS) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
 
 # ld-option
 # Usage: LDFLAGS += $(call ld-option, -X)
 ld-option = $(call try-run,\
-	$(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
+	$(CC) $(NOSTDINC_FLAGS) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
 
 # ar-option
 # Usage: KBUILD_ARFLAGS := $(call ar-option,D)