Re: [PATCH] strace: Fix crash caused over-optimization

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Apr 17 03:39, Daniel Santos wrote:
> On 04/16/2017 05:21 AM, Jon Turney wrote:
> > On 15/04/2017 23:27, Daniel Santos wrote:
> > > Recent versions of gcc are optimizing away the TLS buffer allocated in
> > > main, so we need to tell gcc that it's really used.
> > > ---
> > >  winsup/utils/strace.cc | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/winsup/utils/strace.cc b/winsup/utils/strace.cc
> > > index beab67b90..1e581b4a4 100644
> > > --- a/winsup/utils/strace.cc
> > > +++ b/winsup/utils/strace.cc
> > > @@ -1192,6 +1192,8 @@ main (int argc, char **argv)
> > >    char buf[CYGTLS_PADSIZE];
> > > 
> > >    memset (buf, 0, sizeof (buf));
> > > +  /* Prevent buf from being optimized away.  */
> > > +  __asm__ __volatile__("" :: "m" (buf));
> > 
> > wouldn't adding volatile to the definition of buf be a better way to
> > write this?
> 
> I actually did try that, although I had guessed it wouldn't (and shouldn't)
> work.  I believe that the reason is that rather the accesses are volatile or
> not, gcc can see nothing else using it and memset can be a treated as a
> compiler built-in (per the C spec, maybe C89?), so it can presume the
> outcome.  If there's a cleaner way to do this, I would really love to learn
> that.  __attribute__ ((used)) only works on variables with static storage.
> 
> Now I suspect that I may have found a little bug in gcc, because if I call
> memset by casting it directly as a volatile function pointer, it is still
> optimized away, and it should not:
> 
>   ((void *(*volatile)(void *, int, size_t))memset) (buf, 0, sizeof (buf));
> 
> And most interestingly, if I first assign a local volatile function pointer
> to the address, then gcc properly does *not* optimize it away:
> 
>   void *(*volatile vol_memset)(void *, int, size_t) = memset;
>   vol_memset (buf, 0, sizeof (buf));
> 
> I'm actually really glad for your response and that I played with this
> because I need to make sure that this problem doesn't exist in gcc7.  I have
> changes going into gcc8 shortly and this could mask problems from my test
> program where I cast functions as volatile w/o assigning using a local.
> 
> Daniel

What about using RtlSecureZeroMemory instead?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature