public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] tustsuite: pr102892: Avoid undefined behavior
@ 2022-05-04  0:46 Palmer Dabbelt
  2022-05-04  6:37 ` Richard Biener
  2022-05-04  6:42 ` Jakub Jelinek
  0 siblings, 2 replies; 3+ messages in thread
From: Palmer Dabbelt @ 2022-05-04  0:46 UTC (permalink / raw)
  To: gcc-patches

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(-)

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tustsuite: pr102892: Avoid undefined behavior
  2022-05-04  0:46 [PATCH] tustsuite: pr102892: Avoid undefined behavior Palmer Dabbelt
@ 2022-05-04  6:37 ` Richard Biener
  2022-05-04  6:42 ` Jakub Jelinek
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2022-05-04  6:37 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: GCC Patches

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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tustsuite: pr102892: Avoid undefined behavior
  2022-05-04  0:46 [PATCH] tustsuite: pr102892: Avoid undefined behavior Palmer Dabbelt
  2022-05-04  6:37 ` Richard Biener
@ 2022-05-04  6:42 ` Jakub Jelinek
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2022-05-04  6:42 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: gcc-patches

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-05-04  6:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04  0:46 [PATCH] tustsuite: pr102892: Avoid undefined behavior Palmer Dabbelt
2022-05-04  6:37 ` Richard Biener
2022-05-04  6:42 ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).