diff mbox series

Fix handling of stores in modref_summary::useful_p

Message ID 20200927214600.GC52458@kam.mff.cuni.cz
State New
Headers show
Series Fix handling of stores in modref_summary::useful_p | expand

Commit Message

Jan Hubicka Sept. 27, 2020, 9:46 p.m. UTC
Hi,
this patch fixes a pasto in modref_summary::useful_p that made
ipa-modref to give up on tracking stores when all load info got lost.

Bootstrapped/regtested x86_64-linux, comitted.

gcc/ChangeLog:

2020-09-27  Jan Hubicka  <hubicka@ucw.cz>

	* ipa-modref.c (modref_summary::useful_p): Fix testing of stores.

Comments

Vaseeharan Vinayagamoorthy Oct. 5, 2020, 11:52 a.m. UTC | #1
Hi,

After this patch, I am noticing that some glibc crypto tests get stuck in scanf which goes into busy loop.

My build/host/target setup is:
Build: aarch64-none-linux-gnu
Host: aarch64-none-linux-gnu
Target: aarch64-none-linux-gnu



Kind regards
Vasee


On 27/09/2020, 22:46, "Gcc-patches on behalf of Jan Hubicka" <gcc-patches-bounces@gcc.gnu.org on behalf of hubicka@ucw.cz> wrote:

    Hi,
    this patch fixes a pasto in modref_summary::useful_p that made
    ipa-modref to give up on tracking stores when all load info got lost.

    Bootstrapped/regtested x86_64-linux, comitted.

    gcc/ChangeLog:

    2020-09-27  Jan Hubicka  <hubicka@ucw.cz>

    	* ipa-modref.c (modref_summary::useful_p): Fix testing of stores.

    diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
    index 728c6c1523d..6225552e41a 100644
    --- a/gcc/ipa-modref.c
    +++ b/gcc/ipa-modref.c
    @@ -135,7 +135,7 @@ modref_summary::useful_p (int ecf_flags)
         return true;
       if (ecf_flags & ECF_PURE)
         return false;
    -  return stores && !loads->every_base;
    +  return stores && !stores->every_base;
     }

     /* Dump A to OUT.  */
Szabolcs Nagy Oct. 5, 2020, 4:28 p.m. UTC | #2
The 10/05/2020 12:52, Vaseeharan Vinayagamoorthy wrote:
> Hi,
> 
> After this patch, I am noticing that some glibc crypto tests get stuck in scanf which goes into busy loop.
> 
> My build/host/target setup is:
> Build: aarch64-none-linux-gnu
> Host: aarch64-none-linux-gnu
> Target: aarch64-none-linux-gnu

i can reproduce this on aarch64, i'm looking at it:

if i compile glibc with gcc trunk after this commit i see

$ ./testrun.sh crypt/cert < $glibcsrc/crypt/cert.input
 K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
 K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
 K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
 K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
 K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
 K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
...

it just keeps printing this.

same test binary with glibc code compiled with an
older gcc works, so something in glibc gets miscompiled.

i will have to do more digging to figure out what.




> 
> 
> 
> Kind regards
> Vasee
> 
> 
> On 27/09/2020, 22:46, "Gcc-patches on behalf of Jan Hubicka" <gcc-patches-bounces@gcc.gnu.org on behalf of hubicka@ucw.cz> wrote:
> 
>     Hi,
>     this patch fixes a pasto in modref_summary::useful_p that made
>     ipa-modref to give up on tracking stores when all load info got lost.
> 
>     Bootstrapped/regtested x86_64-linux, comitted.
> 
>     gcc/ChangeLog:
> 
>     2020-09-27  Jan Hubicka  <hubicka@ucw.cz>
> 
>     * ipa-modref.c (modref_summary::useful_p): Fix testing of stores.
> 
>     diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
>     index 728c6c1523d..6225552e41a 100644
>     --- a/gcc/ipa-modref.c
>     +++ b/gcc/ipa-modref.c
>     @@ -135,7 +135,7 @@ modref_summary::useful_p (int ecf_flags)
>          return true;
>        if (ecf_flags & ECF_PURE)
>          return false;
>     -  return stores && !loads->every_base;
>     +  return stores && !stores->every_base;
>      }
> 
>      /* Dump A to OUT.  */
> 

--
Szabolcs Nagy Oct. 5, 2020, 4:54 p.m. UTC | #3
The 10/05/2020 17:28, Szabolcs Nagy via Gcc-patches wrote:
> The 10/05/2020 12:52, Vaseeharan Vinayagamoorthy wrote:
> > Hi,
> > 
> > After this patch, I am noticing that some glibc crypto tests get stuck in scanf which goes into busy loop.
> > 
> > My build/host/target setup is:
> > Build: aarch64-none-linux-gnu
> > Host: aarch64-none-linux-gnu
> > Target: aarch64-none-linux-gnu
> 
> i can reproduce this on aarch64, i'm looking at it:
> 
> if i compile glibc with gcc trunk after this commit i see
> 
> $ ./testrun.sh crypt/cert < $glibcsrc/crypt/cert.input
>  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
>  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
>  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
>  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
>  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
>  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> ...
> 
> it just keeps printing this.
> 
> same test binary with glibc code compiled with an
> older gcc works, so something in glibc gets miscompiled.
> 
> i will have to do more digging to figure out what.

minimal reproducer:

#include <stdio.h>
int main()
{
        int r,t;
        r = sscanf("01", "%2x", &t);
        printf("scanf: %d  %02x\n", r, t);
        return 0;
}

should print

scanf: 1  01

but when glibc is compiled with gcc trunk on aarch64 it prints

scanf: 0  00

i will continute the debugging from here tomorrow.


> > On 27/09/2020, 22:46, "Gcc-patches on behalf of Jan Hubicka" <gcc-patches-bounces@gcc.gnu.org on behalf of hubicka@ucw.cz> wrote:
> > 
> >     Hi,
> >     this patch fixes a pasto in modref_summary::useful_p that made
> >     ipa-modref to give up on tracking stores when all load info got lost.
> > 
> >     Bootstrapped/regtested x86_64-linux, comitted.
> > 
> >     gcc/ChangeLog:
> > 
> >     2020-09-27  Jan Hubicka  <hubicka@ucw.cz>
> > 
> >     * ipa-modref.c (modref_summary::useful_p): Fix testing of stores.
> > 
> >     diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> >     index 728c6c1523d..6225552e41a 100644
> >     --- a/gcc/ipa-modref.c
> >     +++ b/gcc/ipa-modref.c
> >     @@ -135,7 +135,7 @@ modref_summary::useful_p (int ecf_flags)
> >          return true;
> >        if (ecf_flags & ECF_PURE)
> >          return false;
> >     -  return stores && !loads->every_base;
> >     +  return stores && !stores->every_base;
> >      }
> > 
> >      /* Dump A to OUT.  */
> >
Jan Hubicka Oct. 5, 2020, 9:45 p.m. UTC | #4
> The 10/05/2020 17:28, Szabolcs Nagy via Gcc-patches wrote:
> > The 10/05/2020 12:52, Vaseeharan Vinayagamoorthy wrote:
> > > Hi,
> > > 
> > > After this patch, I am noticing that some glibc crypto tests get stuck in scanf which goes into busy loop.
> > > 
> > > My build/host/target setup is:
> > > Build: aarch64-none-linux-gnu
> > > Host: aarch64-none-linux-gnu
> > > Target: aarch64-none-linux-gnu
> > 
> > i can reproduce this on aarch64, i'm looking at it:
> > 
> > if i compile glibc with gcc trunk after this commit i see
> > 
> > $ ./testrun.sh crypt/cert < $glibcsrc/crypt/cert.input
> >  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> >  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> >  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> >  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> >  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> >  K: 0000000000000000 P: 0000000000000000 C: 0000000000000000 Encrypt FAIL
> > ...
> > 
> > it just keeps printing this.
> > 
> > same test binary with glibc code compiled with an
> > older gcc works, so something in glibc gets miscompiled.
> > 
> > i will have to do more digging to figure out what.
> 
> minimal reproducer:
> 
> #include <stdio.h>
> int main()
> {
>         int r,t;
>         r = sscanf("01", "%2x", &t);
>         printf("scanf: %d  %02x\n", r, t);
>         return 0;
> }
> 
> should print
> 
> scanf: 1  01
> 
> but when glibc is compiled with gcc trunk on aarch64 it prints
> 
> scanf: 0  00
> 
> i will continute the debugging from here tomorrow.

There is a report on glibc issue here 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97264
it turned out to be a latent glibc bug type punning const char * and
const unsigned char *.

I wonder if it is same as problem you are seeing?
Honza
> 
> 
> > > On 27/09/2020, 22:46, "Gcc-patches on behalf of Jan Hubicka" <gcc-patches-bounces@gcc.gnu.org on behalf of hubicka@ucw.cz> wrote:
> > > 
> > >     Hi,
> > >     this patch fixes a pasto in modref_summary::useful_p that made
> > >     ipa-modref to give up on tracking stores when all load info got lost.
> > > 
> > >     Bootstrapped/regtested x86_64-linux, comitted.
> > > 
> > >     gcc/ChangeLog:
> > > 
> > >     2020-09-27  Jan Hubicka  <hubicka@ucw.cz>
> > > 
> > >     * ipa-modref.c (modref_summary::useful_p): Fix testing of stores.
> > > 
> > >     diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> > >     index 728c6c1523d..6225552e41a 100644
> > >     --- a/gcc/ipa-modref.c
> > >     +++ b/gcc/ipa-modref.c
> > >     @@ -135,7 +135,7 @@ modref_summary::useful_p (int ecf_flags)
> > >          return true;
> > >        if (ecf_flags & ECF_PURE)
> > >          return false;
> > >     -  return stores && !loads->every_base;
> > >     +  return stores && !stores->every_base;
> > >      }
> > > 
> > >      /* Dump A to OUT.  */
> > >
Szabolcs Nagy Oct. 6, 2020, 8:52 a.m. UTC | #5
The 10/05/2020 23:45, Jan Hubicka wrote:
> > The 10/05/2020 17:28, Szabolcs Nagy via Gcc-patches wrote:
> > minimal reproducer:
> > 
> > #include <stdio.h>
> > int main()
> > {
> >         int r,t;
> >         r = sscanf("01", "%2x", &t);
> >         printf("scanf: %d  %02x\n", r, t);
> >         return 0;
> > }
> > 
> > should print
> > 
> > scanf: 1  01
> > 
> > but when glibc is compiled with gcc trunk on aarch64 it prints
> > 
> > scanf: 0  00
> > 
> > i will continute the debugging from here tomorrow.
> 
> There is a report on glibc issue here 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97264
> it turned out to be a latent glibc bug type punning const char * and
> const unsigned char *.
> 
> I wonder if it is same as problem you are seeing?

thanks, that indeed looks very similar, i'll comment on the glibc bug.
diff mbox series

Patch

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 728c6c1523d..6225552e41a 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -135,7 +135,7 @@  modref_summary::useful_p (int ecf_flags)
     return true;
   if (ecf_flags & ECF_PURE)
     return false;
-  return stores && !loads->every_base;
+  return stores && !stores->every_base;
 }
 
 /* Dump A to OUT.  */