Hi Aaron, Euvgeni, I'll try to whitelist these functions.
Kind regards, Roger -----Original Message----- From: Aaron Ballman [mailto:aaron.ball...@gmail.com] Sent: 14 June 2016 00:30 To: Evgenii Stepanov Cc: reviews+d20561+public+af1fea8a731d8...@reviews.llvm.org; Roger Ferrer Ibanez; Richard Smith; cfe-commits Subject: Re: [PATCH] D20561: Warn when taking address of packed member On Mon, Jun 13, 2016 at 7:17 PM, Evgenii Stepanov <euge...@google.com> wrote: > On Mon, Jun 13, 2016 at 4:12 PM, Aaron Ballman <aaron.ball...@gmail.com> > wrote: >> On Mon, Jun 13, 2016 at 6:30 PM, Evgeniy Stepanov <euge...@google.com> wrote: >>> eugenis added a subscriber: eugenis. >>> eugenis added a comment. >>> >>> In http://reviews.llvm.org/D20561#446031, @aaron.ballman wrote: >>> >>>> In http://reviews.llvm.org/D20561#445824, @rogfer01 wrote: >>>> >>>> > I think I wasn't clear with the purpose of the fix-it: there are a few >>>> > cases where getting the address of an unaligned pointer is safe (i.e. >>>> > false positives). >>>> > >>>> > For instance, when I checked Firefox and Chromium there are cases where >>>> > getting the address of an unaligned pointer is fine. For the particular >>>> > case of these two browsers, they both use a library (usrsctp) that >>>> > represents protocol data as packed structs. That library passes >>>> > addresses of packed fields to `memcpy` and `memmove` which is OK. >>>> >>>> >>>> I think this is a false-positive that should be fixed. >>> >>> >>> This patch was committed without fixing the false positive case, why? >> >> Can you give some more code context, like perhaps a test case we can >> add to the suite? I believe the current behavior should be >> essentially false-positive-free because it only triggers the >> diagnostic when the alignments actually differ, so if there are other >> false-positives, I agree that we should determine a disposition for them. > > This is actually the same library the comment above is talking about: > https://bugs.chromium.org/p/chromium/issues/detail?id=619640 The first example (gettimeofday()) looks to be a true-positive for sure. The remainder are tricky. They're not going to trigger UB in practice because the void * that memcpy() takes will get promptly typecast into a character pointer to avoid UB with the copy, and that's 1-byte aligned. There's a part of me that thinks it may be reasonable to assume that casting to void * always means "trust me, it'll be fine" because the pointer needs to be cast back out of a void * to be useful, and so the diagnostic should be skipped in that case. However, it's also just as likely to see code like (esp in C): void func(void *ptr) { int *blah = ptr; *blah = 12; } void foo(void) { struct some_packed_struct s; func(&s.something); } which has drifted into UB. Roger, is there a way to whitelist memcpy (memmove, memset, et al) for this check? >>> Could this warning be excluded from -Wall? >> >> Would you like the warning to be excluded from -Wall even if the >> false-positive is fixed? > > No, the warning seems useful if the false positive is fixed. Ok, good to know. ~Aaron _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
