Re: [PATCH] strace: Fix crash caused over-optimization
On 04/18/2017 05:04 AM, Corinna Vinschen wrote: On Apr 17 03:39, Daniel Santos wrote: 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 Well that's surprising. Yes, it does solve the problem and I presume would be more portable. :) It even inlines the "memset", but uses a single byte rep stos. Technically, I think that a double-word stos could be used in this case, but I doubt that matters much. Daniel
[PATCH v2] strace: Fix "over-optimization" flaw in strace.
Recent versions of gcc are optimizing away the TLS buffer allocated in main, so we need to tell gcc that it's really used. RtlSecureZeroMemory accomplishes this while also inlining the memset. Signed-off-by: Daniel Santos --- winsup/utils/strace.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/utils/strace.cc b/winsup/utils/strace.cc index beab67b90..ae62cdc5f 100644 --- a/winsup/utils/strace.cc +++ b/winsup/utils/strace.cc @@ -1191,7 +1191,7 @@ main (int argc, char **argv) registry setting to 0x10 (TOP_DOWN). */ char buf[CYGTLS_PADSIZE]; - memset (buf, 0, sizeof (buf)); + RtlSecureZeroMemory (buf, sizeof (buf)); exit (main2 (argc, argv)); } -- 2.11.0
resend: Re: [PATCH] strace: Fix crash caused over-optimization
sourceware.org decided that I was a spammer for some weird reason... Maybe this one will go through... On 04/19/2017 10:52 AM, mailer-dae...@sourceware.org wrote: Hi. This is the qmail-send program at sourceware.org. I'm afraid I wasn't able to deliver your message to the following addresses. This is a permanent error; I've given up. Sorry it didn't work out. : Mail rejected: List address must be in To: or Cc: headers. See http://sourceware.org/lists.html#sourceware-list-info for more information. If you are not a "spammer", we apologize for the inconvenience. You can add yourself to the cygwin.com "global allow list" by sending email *from*the*blocked*email*address* to: Forwarded Message Subject:Re: [PATCH] strace: Fix crash caused over-optimization Date: Wed, 19 Apr 2017 10:57:02 -0500 From: Daniel Santos To: cygwin-patches@cygwin.com, Corinna Vinschen On 04/18/2017 05:04 AM, Corinna Vinschen wrote: On Apr 17 03:39, Daniel Santos wrote: 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 Well that's surprising. Yes, it does solve the problem and I presume would be more portable. :) It even inlines the "memset", but uses a single byte rep stos. Technically, I think that a double-word stos could be used in this case, but I doubt that matters much. Daniel
Re: [PATCH v2] strace: Fix "over-optimization" flaw in strace.
On Apr 19 11:06, 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. RtlSecureZeroMemory > accomplishes this while also inlining the memset. > > Signed-off-by: Daniel Santos > --- > winsup/utils/strace.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/winsup/utils/strace.cc b/winsup/utils/strace.cc > index beab67b90..ae62cdc5f 100644 > --- a/winsup/utils/strace.cc > +++ b/winsup/utils/strace.cc > @@ -1191,7 +1191,7 @@ main (int argc, char **argv) > registry setting to 0x10 (TOP_DOWN). */ >char buf[CYGTLS_PADSIZE]; > > - memset (buf, 0, sizeof (buf)); > + RtlSecureZeroMemory (buf, sizeof (buf)); >exit (main2 (argc, argv)); > } > > -- > 2.11.0 Pushed. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat signature.asc Description: PGP signature
Re: [PATCH v2] strace: Fix "over-optimization" flaw in strace.
On 04/19/2017 01:48 PM, Corinna Vinschen wrote: Pushed. Thanks, Corinna Thanks. I hope this message goes through. Earlier when I tried to respond with both you and cygwin-patches in the To: header it bounced. I emailed cygwin-ow...@cygwin.com about this, is that the right address for mailing list problems? Thanks, Daniel
test message
Will this bounce again?
Re: [PATCH v2] strace: Fix "over-optimization" flaw in strace.
On 04/19/2017 05:10 PM, Daniel Santos wrote: Thanks. I hope this message goes through. Earlier when I tried to respond with both you and cygwin-patches in the To: header it bounced. I emailed cygwin-ow...@cygwin.com about this, is that the right address for mailing list problems? Actually, any email I send with corinna-cyg...@cygwin.com in the To header is bounced. : Mail rejected: List address must be in To: or Cc: headers. See http://sourceware.org/lists.html#sourceware-list-info for more information. Daniel
