Re: [PATCH v2] analyzer: Move gcc.dg/analyzer tests to c-c++-common (1) [PR96395]

2023-08-29 Thread Benjamin Priour via Gcc-patches
Hi Prathamesh,

Thanks for the report, I am running on a x86_64_linux_gnu and never had to
cross-compile before.
I've tried to set it up and reproduce your errors, please tell me if I'm
off the mark.

I configured gcc as:

../gcc/configure --prefix $CONTROL_PFX --disable-bootstrap
--host=x86_64-linux-gnu --target=arm-eabi --enable-languages=c --with-newlib

Then from the patch 7997f0d35efca8a24d1b0ceae5066b1019d633d7 prior to mine,
*I ran the tests manually as arm-sim refused to work*:
$GCC_SRC/control/eabi_arm_build/gcc/xgcc
-B$GCC_SRC/control//eabi_arm_build/gcc \

../../gcc/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c
\
  -fanalyzer -Wanalyzer-too-complex -fanalyzer-call-summaries
and
$GCC_SRC/control/eabi_arm_build/gcc/xgcc
-B$GCC_SRC/control//eabi_arm_build/gcc \

../../gcc/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
\
  -fanalyzer -Wanalyzer-too-complex -fanalyzer-call-summaries -O2


>From my patch, I ran similar commands, only s/gcc.dg/c-c++-common/ as the
patch
moved the tests there.

The output were *identical*, for both pre/post-patched.
I do agree with you that there is an excess error compared to
x86_64-linux-gnu and the test expectations.

../../gcc/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c:63:65:
warning: converting a packed ‘enum obj_type’ pointer (alignment 1) to a
‘struct connection’ pointer (alignment 4) may result in an unaligned
pointer value [-Waddress-of-packed-member]
   63 |  return ((struct connection *)(((void *)(t)) - ((long)&((struct
connection *)0)->obj_type)));
  |
^~
../../gcc/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c:31:6:
note: defined here
   31 | enum obj_type {
  |  ^~~~
../../gcc/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c:47:8:
note: defined here
   47 | struct connection {
  |^~

Therefore I do also see a regression, but it does not come from
55f6a7d949abc708d1c6ebc01eb3053f96d1472b.
It seems like an easy fix though and I will add it to the second part of
this patch.

However I did run the tests manually, rather than with make check-gcc as it
failed to run - an issue with stdio not found.
I'm attending this issue and will further update you in case the conclusion
of this mail changes.

Thanks again,
Benjamin.

On Tue, Aug 29, 2023 at 8:47 AM Prathamesh Kulkarni <
prathamesh.kulka...@linaro.org> wrote:

> On Sat, 26 Aug 2023 at 18:02, Benjamin Priour via Gcc-patches
>  wrote:
> >
> > From: benjamin priour 
> >
> > Hi,
> >
> > Updated version of the patch, regstrapping the changes described in
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628455.html.
> >
> > Regstrapped off trunk 66be6ed81f369573824f1a8f5a3538a63472292f
> > on x86_64-linux-gnu.
> Hi Benjamin,
> It seems your patch committed in 55f6a7d949abc708d1c6ebc01eb3053f96d1472b
> caused following regressions on arm-eabi config:
> Running gcc:gcc.dg/analyzer/analyzer.exp ...
> FAIL:
> c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> (test for excess errors)
> FAIL:
> c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c
> (test for excess errors)
>
> Please let me know if you need help to reproduce these failures.
>
> Thanks,
> Prathamesh
>
>


Re: [PATCH v2] analyzer: Move gcc.dg/analyzer tests to c-c++-common (1) [PR96395]

2023-08-26 Thread David Malcolm via Gcc-patches
On Sat, 2023-08-26 at 14:22 +0200, priour...@gmail.com wrote:


> From: benjamin priour 
> 
> Hi,
> 
> Updated version of the patch, regstrapping the changes described in
> https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628455.html.
> 
> Regstrapped off trunk 66be6ed81f369573824f1a8f5a3538a63472292f
> on x86_64-linux-gnu.
> 
> OK for trunk ?

Thanks for the v2 patch.

This is almost ready, some minor nits below...

[...snip...]

> 
> gcc/analyzer/ChangeLog:
> 
>   analyzer/PR 96395

This should be PR analyzer/96395.

>   * analyzer.h (class known_function): Add virtual casts
>   to builtin_known_function.
>   (class builtin_known_function): New subclass of known_function
>   for builtins.

[...snip...]
 
> 
> gcc/testsuite/ChangeLog:
> 
>   analyzer/PR 96395

Likewise here.

>   * gcc.dg/analyzer/aliasing-3.c: Moved to...
>   * c-c++-common/analyzer/aliasing-3.c: ...here.
>   * gcc.dg/analyzer/aliasing-pr106473.c: Moved to...
>   * c-c++-common/analyzer/aliasing-pr106473.c: ...here.

[...snip...]
 
> diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
> index 93a28b4b5cf..d42771f1e20 100644
> --- a/gcc/analyzer/analyzer.h
> +++ b/gcc/analyzer/analyzer.h

[...snip...]

> @@ -279,6 +283,26 @@ public:
>{
>  return;
>}
> +
> +  virtual const builtin_known_function *
> +  dyn_cast_builtin_kf () const { return NULL; }
> +  virtual builtin_known_function *
> +  dyn_cast_builtin_kf () { return NULL; }

As noted in the review of v1, I don't think we ever work with non-const
known_function pointers, so we don't need the non-const version of the
vfunc.

[...snip...]

> diff --git a/gcc/testsuite/c-c++-common/analyzer/pr61861.c 
> b/gcc/testsuite/c-c++-common/analyzer/pr61861.c
> new file mode 100644
> index 000..bb9e039ebd5
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/analyzer/pr61861.c
> @@ -0,0 +1,3 @@
> +/* { dg-additional-options "-Wno-int-conversion" } */
> +/* { dg-skip-if "-Wno-int-conversion for C++" { c++ } } */
> +#include "../../gcc.dg/pr61861.c"

For tests like this that aren't going to be portable to C++, let's keep
it in the gcc.dg subdirectory, with a suitable comment, rather than
moving them and having a dg-skip-if on it.

Perhaps:

/* C only: -Wno-int-conversion is not valid for C++.  */

That way we can easily grep for the absence of "C only" when checking
to see which analyzer tests below gcc.dg still need considering for
moving below c-c++-common.  The string "C only" is conveniently short.

[...snip...]

> diff --git a/gcc/testsuite/gcc.dg/analyzer/pr95152-4.c 
> b/gcc/testsuite/c-c++-common/analyzer/pr95152-4.c
> similarity index 55%
> rename from gcc/testsuite/gcc.dg/analyzer/pr95152-4.c
> rename to gcc/testsuite/c-c++-common/analyzer/pr95152-4.c
> index f2a72cad01c..5ebbae85aee 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/pr95152-4.c
> +++ b/gcc/testsuite/c-c++-common/analyzer/pr95152-4.c
> @@ -1,4 +1,6 @@
> +/* { dg-skip-if "'-Wno-pointer-to-int-cast' invalid for C++" { c++ } } */
>  /* { dg-additional-options "-Wno-pointer-to-int-cast" } */

Likewise here.

[...snip...]

> diff --git a/gcc/testsuite/gcc.dg/analyzer/pr95152-5.c 
> b/gcc/testsuite/c-c++-common/analyzer/pr95152-5.c
> similarity index 61%
> rename from gcc/testsuite/gcc.dg/analyzer/pr95152-5.c
> rename to gcc/testsuite/c-c++-common/analyzer/pr95152-5.c
> index 604b78458c7..fbc4753e0b4 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/pr95152-5.c
> +++ b/gcc/testsuite/c-c++-common/analyzer/pr95152-5.c
> @@ -1,3 +1,4 @@
> +/* { dg-skip-if "'-Wno-incompatible-pointer-types' invalid for C++" { c++ } 
> } */
>  /* { dg-additional-options "-Wno-incompatible-pointer-types" } */
>  void foo(void)
>  {

Likewise here.

[...snip...]

> diff --git a/gcc/testsuite/gcc.dg/analyzer/write-to-function-1.c 
> b/gcc/testsuite/c-c++-common/analyzer/write-to-function-1.c
> similarity index 81%
> rename from gcc/testsuite/gcc.dg/analyzer/write-to-function-1.c
> rename to gcc/testsuite/c-c++-common/analyzer/write-to-function-1.c
> index c1bece632ce..dd4adc13141 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/write-to-function-1.c
> +++ b/gcc/testsuite/c-c++-common/analyzer/write-to-function-1.c
> @@ -1,3 +1,5 @@
> +/* { dg-skip-if "c++ does not allow for conversion from function pointer to 
> 'void *'" { c++ } } */
> +

Likewise here.

[...snip...]

> diff --git a/gcc/testsuite/gcc.dg/analyzer/pr104369-1.c 
> b/gcc/testsuite/gcc.dg/analyzer/pr104369-1.c
> index c05137bb219..788a2059013 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/pr104369-1.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/pr104369-1.c
> @@ -1,5 +1,8 @@
>  /* { dg-additional-options "-Wno-analyzer-too-complex -Wno-analyzer-fd-leak" 
> } */
>  // TODO: remove need for these options
> +/* This test needs not be moved to c-c++-common/analyzer as C++
> +   does not support transparent_union. */
> +

Perhaps update this comment to:

/* C only: C++ does not support transparent_union.*/

so that we can grep for (the absen