diff mbox series

tustsuite: pr102892: Avoid undefined behavior

Message ID 20220504004650.5199-1-palmer@rivosinc.com
State New
Headers
Series tustsuite: pr102892: Avoid undefined behavior |

Commit Message

Palmer Dabbelt May 4, 2022, 12:46 a.m. UTC 
  This test was relying on undefined behavior, with -Wundef I get

    gcc/testsuite/gcc.dg/pr102892-1.c: In function 'main':
    cc/testsuite/gcc.dg/pr102892-1.c:14:18: warning: 'a' is used uninitialized [-Wuninitialized]
       14 |   for (long a; a < 1; ++a)
          |                ~~^~~
    gcc/testsuite/gcc.dg/pr102892-1.c:14:13: note: 'a' was declared here
       14 |   for (long a; a < 1; ++a)
          |             ^

gcc/testsuite/ChangeLog:

	* gcc.dg/pr102892-1.c (main): Avoid undefined behavior.
---
The discussion on bug 102892
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102892> suggests this may
always have been a result of undefined behavior, but with these simple
changes the test passes for me.  No idea why we're hitting this in
RISC-V (and SPARC) and not elsewhere, and I also haven't gone back and
checked the original fix to see if it was always just UB causing the
issue.  Happy to look at this further, but one suggestion on bugzilla
was to just drop the test as invalid so I didn't want to chase something
that didn't matter.
---
 gcc/testsuite/gcc.dg/pr102892-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Biener May 4, 2022, 6:37 a.m. UTC | #1 
On Wed, May 4, 2022 at 2:47 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> This test was relying on undefined behavior, with -Wundef I get
>
>     gcc/testsuite/gcc.dg/pr102892-1.c: In function 'main':
>     cc/testsuite/gcc.dg/pr102892-1.c:14:18: warning: 'a' is used uninitialized [-Wuninitialized]
>        14 |   for (long a; a < 1; ++a)
>           |                ~~^~~
>     gcc/testsuite/gcc.dg/pr102892-1.c:14:13: note: 'a' was declared here
>        14 |   for (long a; a < 1; ++a)
>           |             ^
>
> gcc/testsuite/ChangeLog:

Please verify if the testcase then still reproduces the missed DCE before
the fix.  An alternative solution might be to make a initialized from a global
variable.

>         * gcc.dg/pr102892-1.c (main): Avoid undefined behavior.
> ---
> The discussion on bug 102892
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102892> suggests this may
> always have been a result of undefined behavior, but with these simple
> changes the test passes for me.  No idea why we're hitting this in
> RISC-V (and SPARC) and not elsewhere, and I also haven't gone back and
> checked the original fix to see if it was always just UB causing the
> issue.  Happy to look at this further, but one suggestion on bugzilla
> was to just drop the test as invalid so I didn't want to chase something
> that didn't matter.
> ---
>  gcc/testsuite/gcc.dg/pr102892-1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.dg/pr102892-1.c b/gcc/testsuite/gcc.dg/pr102892-1.c
> index a9302b536df..b13f94d2a87 100644
> --- a/gcc/testsuite/gcc.dg/pr102892-1.c
> +++ b/gcc/testsuite/gcc.dg/pr102892-1.c
> @@ -11,7 +11,7 @@ int
>  main ()
>  {
>    long c = 0;
> -  for (long a; a < 1; ++a)
> +  for (long a = 0; a < 2; ++a)
>      for (; c <= 1; c++) {
>        bar();
>        if (1 == b[c][0])
> --
> 2.34.1
>
Jakub Jelinek May 4, 2022, 6:42 a.m. UTC | #2 
On Tue, May 03, 2022 at 05:46:50PM -0700, Palmer Dabbelt wrote:
> This test was relying on undefined behavior, with -Wundef I get
> 
>     gcc/testsuite/gcc.dg/pr102892-1.c: In function 'main':
>     cc/testsuite/gcc.dg/pr102892-1.c:14:18: warning: 'a' is used uninitialized [-Wuninitialized]
>        14 |   for (long a; a < 1; ++a)
>           |                ~~^~~
>     gcc/testsuite/gcc.dg/pr102892-1.c:14:13: note: 'a' was declared here
>        14 |   for (long a; a < 1; ++a)
>           |             ^
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/pr102892-1.c (main): Avoid undefined behavior.
> ---
> The discussion on bug 102892
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102892> suggests this may
> always have been a result of undefined behavior, but with these simple
> changes the test passes for me.  No idea why we're hitting this in
> RISC-V (and SPARC) and not elsewhere, and I also haven't gone back and
> checked the original fix to see if it was always just UB causing the
> issue.  Happy to look at this further, but one suggestion on bugzilla
> was to just drop the test as invalid so I didn't want to chase something
> that didn't matter.

It is a dg-do link testcase, so it certainly can contain UB in it
(similarly, dg-do run testcase could contain UB as long as it isn't
encountered during runtime).
The important question is if whatever this has been reduced from had that UB
in it or if it has been introduced during the reduction (the reporter didn't
answer this) and whether the testcase with the " = 0" added behaved the same
(11.2.0 can optimize foo away even in that case, r12-4790-g4b3a325f07acebf4^
can't and r12-4790-g4b3a325f07acebf4 can again).
I'd say if the latter is true we probably should change it even if the
reporter doesn't respond.

>  gcc/testsuite/gcc.dg/pr102892-1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/pr102892-1.c b/gcc/testsuite/gcc.dg/pr102892-1.c
> index a9302b536df..b13f94d2a87 100644
> --- a/gcc/testsuite/gcc.dg/pr102892-1.c
> +++ b/gcc/testsuite/gcc.dg/pr102892-1.c
> @@ -11,7 +11,7 @@ int
>  main ()
>  {
>    long c = 0;
> -  for (long a; a < 1; ++a)
> +  for (long a = 0; a < 2; ++a)
>      for (; c <= 1; c++) {
>        bar();
>        if (1 == b[c][0])
> -- 
> 2.34.1

	Jakub
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/pr102892-1.c b/gcc/testsuite/gcc.dg/pr102892-1.c
index a9302b536df..b13f94d2a87 100644
--- a/gcc/testsuite/gcc.dg/pr102892-1.c
+++ b/gcc/testsuite/gcc.dg/pr102892-1.c
@@ -11,7 +11,7 @@  int
 main ()
 {
   long c = 0;
-  for (long a; a < 1; ++a)
+  for (long a = 0; a < 2; ++a)
     for (; c <= 1; c++) {
       bar();
       if (1 == b[c][0])