diff mbox series

[2/2] gcc: make include poisoning fatal again in gcc/g++

Message ID 20250121182309.904188-2-ross.burton@arm.com
State Accepted, archived
Commit 56f21a02c009cb74072ee79467a5bcab3c4643a5
Headers show
Series [1/2] oeqa/poisoning: fix gcc include poisoning test | expand

Commit Message

Ross Burton Jan. 21, 2025, 6:23 p.m. UTC
We have a patch to allow us to 'poison' system include directories,
which are warnings by default but we make them fatal in cross builds.

However, in the 13.1 upgrade[1] the patch to make the warnings fatal was
dropped in the compiler invocation, so it only took effect for pure
preprocessor calls. This was not noticed at the time as the test case
was flawed, but this has now been fixed.

Add back the fatal poisoning, and restructure the patch slightly so it
is less invasive.

[1] oe-core bea46612fd9106cc5b46eb1d81623b6492563c13

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 .../0002-gcc-poison-system-directories.patch  | 35 +++++++++++++------
 1 file changed, 25 insertions(+), 10 deletions(-)

Comments

Richard Purdie Jan. 22, 2025, 8:15 a.m. UTC | #1
On Tue, 2025-01-21 at 18:23 +0000, Ross Burton via lists.openembedded.org wrote:
> We have a patch to allow us to 'poison' system include directories,
> which are warnings by default but we make them fatal in cross builds.
> 
> However, in the 13.1 upgrade[1] the patch to make the warnings fatal was
> dropped in the compiler invocation, so it only took effect for pure
> preprocessor calls. This was not noticed at the time as the test case
> was flawed, but this has now been fixed.
> 
> Add back the fatal poisoning, and restructure the patch slightly so it
> is less invasive.
> 
> [1] oe-core bea46612fd9106cc5b46eb1d81623b6492563c13
> 
> Signed-off-by: Ross Burton <ross.burton@arm.com>
> ---
>  .../0002-gcc-poison-system-directories.patch  | 35 +++++++++++++------
>  1 file changed, 25 insertions(+), 10 deletions(-)

Sadly this causes a ton of failures:

https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/857

The patch is right but we're going to have to dig in and clean up the
issues before it can merge.

Cheers,

Richard
Richard Purdie Jan. 22, 2025, 10:52 a.m. UTC | #2
On Tue, 2025-01-21 at 18:23 +0000, Ross Burton via lists.openembedded.org wrote:
> We have a patch to allow us to 'poison' system include directories,
> which are warnings by default but we make them fatal in cross builds.
> 
> However, in the 13.1 upgrade[1] the patch to make the warnings fatal was
> dropped in the compiler invocation, so it only took effect for pure
> preprocessor calls. This was not noticed at the time as the test case
> was flawed, but this has now been fixed.
> 
> Add back the fatal poisoning, and restructure the patch slightly so it
> is less invasive.
> 
> [1] oe-core bea46612fd9106cc5b46eb1d81623b6492563c13
> 
> Signed-off-by: Ross Burton <ross.burton@arm.com>
> ---
>  .../0002-gcc-poison-system-directories.patch  | 35 +++++++++++++------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch b/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch
> index 367c9e3821b..d8b3d4a9984 100644
> --- a/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch
> +++ b/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch
> @@ -160,10 +160,21 @@ index f82f7d2817b..1da91813b0e 100644
>   @opindex Wno-float-equal
>   @item -Wfloat-equal
>  diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> -index 728332b8153..343e4915097 100644
> +index 728332b8153..a63f128cb95 100644
>  --- a/gcc/gcc.cc
>  +++ b/gcc/gcc.cc
> -@@ -1159,6 +1159,8 @@ proper position among the other output files.  */
> +@@ -902,6 +902,10 @@ proper position among the other output files.  */
> + #define ASM_MAP ""
> + #endif
> + 
> ++#ifdef POISON_BY_DEFAULT
> ++#define POISON_IS_ERROR " -Werror=poison-system-directories"
> ++#endif
> ++

I think this needs to be:

+#ifdef POISON_BY_DEFAULT
+#define POISON_IS_ERROR " -Werror=poison-system-directories"
+#else
+#define POISON_IS_ERROR
+#endif

otherwwise POISON_IS_ERROR is undefined and leads to failures building
target gcc and gcc-cross-canadian. Fixup in -next I can squash in if
you agree.

Cheers,

Richard
Ross Burton Jan. 22, 2025, 11:44 a.m. UTC | #3
On 22 Jan 2025, at 10:52, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> I think this needs to be:
> 
> +#ifdef POISON_BY_DEFAULT
> +#define POISON_IS_ERROR " -Werror=poison-system-directories"
> +#else
> +#define POISON_IS_ERROR
> +#endif
> 
> otherwwise POISON_IS_ERROR is undefined and leads to failures building
> target gcc and gcc-cross-canadian. Fixup in -next I can squash in if
> you agree.

Please do. Apologies, my C is rustier than I thought. :(

Ross
diff mbox series

Patch

diff --git a/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch b/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch
index 367c9e3821b..d8b3d4a9984 100644
--- a/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch
+++ b/meta/recipes-devtools/gcc/gcc/0002-gcc-poison-system-directories.patch
@@ -160,10 +160,21 @@  index f82f7d2817b..1da91813b0e 100644
  @opindex Wno-float-equal
  @item -Wfloat-equal
 diff --git a/gcc/gcc.cc b/gcc/gcc.cc
-index 728332b8153..343e4915097 100644
+index 728332b8153..a63f128cb95 100644
 --- a/gcc/gcc.cc
 +++ b/gcc/gcc.cc
-@@ -1159,6 +1159,8 @@ proper position among the other output files.  */
+@@ -902,6 +902,10 @@ proper position among the other output files.  */
+ #define ASM_MAP ""
+ #endif
+ 
++#ifdef POISON_BY_DEFAULT
++#define POISON_IS_ERROR " -Werror=poison-system-directories"
++#endif
++
+ /* Assembler options for compressed debug sections.  */
+ #if HAVE_LD_COMPRESS_DEBUG == 0
+ /* Reject if the linker cannot write compressed debug sections.  */
+@@ -1159,6 +1163,8 @@ proper position among the other output files.  */
     "%{fuse-ld=*:-fuse-ld=%*} " LINK_COMPRESS_DEBUG_SPEC \
     "%X %{o*} %{e*} %{N} %{n} %{r}\
      %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!r:%{!nostartfiles:%S}}} \
@@ -172,20 +183,24 @@  index 728332b8153..343e4915097 100644
      %{static|no-pie|static-pie:} %@{L*} %(link_libgcc) " \
      VTABLE_VERIFICATION_SPEC " " SANITIZER_EARLY_SPEC " %o "" \
      %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1):\
-@@ -1260,8 +1262,11 @@ static const char *cpp_unique_options =
- static const char *cpp_options =
+@@ -1261,7 +1267,7 @@ static const char *cpp_options =
  "%(cpp_unique_options) %1 %{m*} %{std*&ansi&trigraphs} %{W*&pedantic*} %{w}\
   %{f*} %{g*:%{%:debug-level-gt(0):%{g*}\
-- %{!fno-working-directory:-fworking-directory}}} %{O*}\
+  %{!fno-working-directory:-fworking-directory}}} %{O*}\
 - %{undef} %{save-temps*:-fpch-preprocess}";
-+ %{!fno-working-directory:-fworking-directory}}} %{O*}"
-+#ifdef POISON_BY_DEFAULT
-+ " -Werror=poison-system-directories"
-+#endif
-+ " %{undef} %{save-temps*:-fpch-preprocess}";
++ %{undef} %{save-temps*:-fpch-preprocess}" POISON_IS_ERROR;
  
  /* Pass -d* flags, possibly modifying -dumpdir, -dumpbase et al.
  
+@@ -1290,7 +1296,7 @@ static const char *cc1_options =
+  %{coverage:-fprofile-arcs -ftest-coverage}\
+  %{fprofile-arcs|fcondition-coverage|fprofile-generate*|coverage:\
+    %{!fprofile-update=single:\
+-     %{pthread:-fprofile-update=prefer-atomic}}}";
++     %{pthread:-fprofile-update=prefer-atomic}}}" POISON_IS_ERROR;
+ 
+ static const char *asm_options =
+ "%{-target-help:%:print-asm-header()} "
 diff --git a/gcc/incpath.cc b/gcc/incpath.cc
 index 64cdd2f4a..89f42900d 100644
 --- a/gcc/incpath.cc