[PATCH] D20561: Warn when taking address of packed member

2019-10-07 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf7b9f3149b76: Add missing tests (authored by rogfer01). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D20561?vs=67807=223564#toc Repository: rG LLVM Github Monorepo

[PATCH] D20561: Warn when taking address of packed member

2019-10-03 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. I think I found a false negative with this where if the member is accessed from a packed struct type returned from a function, the warning does not appear: typedef struct { uint8_t a;

[PATCH] D20561: Warn when taking address of packed member

2017-02-02 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment. Yes, the project is interested on reducing the number of false positives. The example you gave is *not* a FP, but exactly the kind of situation the warning is supposed to trigger on. Repository: rL LLVM https://reviews.llvm.org/D20561

[PATCH] D20561: Warn when taking address of packed member

2017-02-02 Thread Roger via Phabricator via cfe-commits
royger added a comment. Ping? It's not clear to me whether upstream is going to do something about this or not. I would like to know in case I need to start passing "-Waddress-of-packed-member" around. Repository: rL LLVM https://reviews.llvm.org/D20561

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment. The last review left out the case of naturally aligned packed members, IIRC. I have to go over the warning list in NetBSD again, but I'm moderately sure it is not fixed. The better example is: struct __attribute__((__packed__)) bar { uint16_t x1; uint16_t

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger via Phabricator via cfe-commits
royger added a comment. In https://reviews.llvm.org/D20561#663069, @joerg wrote: > @royger: Your example is missing explicit alignment. packed has two side > effects: remove internal padding and set the alignment to 1. That means that > the offset of base doesn't matter so much because reg

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment. In https://reviews.llvm.org/D20561#663069, @joerg wrote: > It is not true that all false positives have been fixed, some of the more > complex cases involving nested data structures are still open. I could not find any in bugzilla after a quick search. So please feel

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment. It is not true that all false positives have been fixed, some of the more complex cases involving nested data structures are still open. @royger: Your example is missing explicit alignment. packed has two side effects: remove internal padding and set the alignment to 1.

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger via Phabricator via cfe-commits
royger added a comment. In https://reviews.llvm.org/D20561#663006, @kimgr wrote: > ... and both revisions should be in the 4.0 branch (taken from r291814) > > I was looking forward to this warning to help iron out alignment issues at > compile-time instead of runtime (our ARM CPUs don't like

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr added a comment. ... and both revisions should be in the 4.0 branch (taken from r291814) I was looking forward to this warning to help iron out alignment issues at compile-time instead of runtime (our ARM CPUs don't like unaligned access). @royger, can you expand a little on what you

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment. In https://reviews.llvm.org/D20561#662963, @royger wrote: > In https://reviews.llvm.org/D20561#662959, @rogfer01 wrote: > > > We fixed all identified false positives in later patches to this one. So > > maybe you want to check against trunk clang. If trunk still

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger via Phabricator via cfe-commits
royger added a comment. In https://reviews.llvm.org/D20561#662959, @rogfer01 wrote: > We fixed all identified false positives in later patches to this one. So > maybe you want to check against trunk clang. If trunk still diagnoses false > positives, please report them to me and I will be more

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment. In https://reviews.llvm.org/D20561#662922, @royger wrote: > Hi, some targets do not support unaligned accesses so this warning is valuable in terms of portability. In platforms where unaligned accesses are allowed, it may happen that such accesses incur in a

[PATCH] D20561: Warn when taking address of packed member

2017-02-01 Thread Roger via Phabricator via cfe-commits
royger reopened this revision. royger added a comment. This revision is now accepted and ready to land. Hello, This addition is silly, and too noisy for real world usage. For once, it doesn't really check whether the access is actually unaligned or not, and then on x86 for example unaligned

Re: [PATCH] D20561: Warn when taking address of packed member

2016-08-15 Thread Aaron Ballman via cfe-commits
On Mon, Aug 15, 2016 at 6:20 PM, Matthias Braun wrote: > MatzeB added a subscriber: MatzeB. > MatzeB added a comment. > > The sanitizer code triggers this warning for code that looks conceptually > like this: > > typedef struct Bla { char bar; int foo; }

Re: [PATCH] D20561: Warn when taking address of packed member

2016-08-15 Thread Matthias Braun via cfe-commits
MatzeB added a subscriber: MatzeB. MatzeB added a comment. The sanitizer code triggers this warning for code that looks conceptually like this: typedef struct Bla { char bar; int foo; } __attribute__((packed)); uintptr_t getu(struct Bla *b) { return (uintptr_t)>foo; } Resulting in:

Re: [PATCH] D20561: Warn when taking address of packed member

2016-08-12 Thread Roger Ferrer Ibanez via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL278483: This patch implements PR#22821. (authored by rogfer01). Changed prior to commit: https://reviews.llvm.org/D20561?vs=64114=67807#toc Repository: rL LLVM https://reviews.llvm.org/D20561

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-28 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Hi, friendly ping. @rsmith any further comment on this? Thank you very much. https://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-18 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. > Please wait until someone has had the chance to review before committing (the > fix does not appear trivial and the original code broke code). I'm on > vacation today (in theory), but perhaps @rsmith will have a chance to review. Sure. Thanks.

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-15 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated the summary for this revision. rogfer01 removed rL LLVM as the repository for this revision. rogfer01 updated this revision to Diff 64114. rogfer01 added a comment. Remove the diagnostic for the binding of references. https://reviews.llvm.org/D20561 Files:

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-15 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. I've opened https://llvm.org/bugs/show_bug.cgi?id=28571 to track the reference binding issue. Repository: rL LLVM https://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-15 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. In https://reviews.llvm.org/D20561#484421, @jyknight wrote: > This seems to trigger even for the implicitly generated copier of a packed > struct. E.g. > > #include > > void copyit(epoll_event, const epoll_event ) { > out = in; > } > > > > Is that as

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-15 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 reopened this revision. rogfer01 added a comment. This revision is now accepted and ready to land. I suggest that we handle the reference binding diagnostic in another change and leave this one only for the address taking diagnostic. Repository: rL LLVM

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-14 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Reverted. Repository: rL LLVM https://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-14 Thread Roger Ferrer Ibanez via cfe-commits
; cfe-commits Subject: Re: [PATCH] D20561: Warn when taking address of packed member Roger, can you please revert? This seems to have caused some pain for our users, and it would be good to unblock them while deciding what to do about the issues. ~Aaron On Thu, Jul 14, 2016 at 2:19 PM, James Y

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-14 Thread Aaron Ballman via cfe-commits
Roger, can you please revert? This seems to have caused some pain for our users, and it would be good to unblock them while deciding what to do about the issues. ~Aaron On Thu, Jul 14, 2016 at 2:19 PM, James Y Knight wrote: > jyknight added a comment. > > Regardless, I

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-14 Thread James Y Knight via cfe-commits
jyknight added a comment. Regardless, I think this should not have added a new un-disableable error, but instead only a default-on warning. The ""binding reference to packed member" error is firing on some of our code, and even if it's not a false-positive, it should be possible to disable it

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-14 Thread James Y Knight via cfe-commits
jyknight added a subscriber: jyknight. jyknight added a comment. This seems to trigger even for the implicitly generated copier of a packed struct. E.g. #include void copyit(epoll_event, const epoll_event ) { out = in; } Is that as intended? Repository: rL LLVM

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-14 Thread Roger Ferrer Ibanez via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL275417: Diagnose taking address and reference binding of packed members (authored by rogfer01). Changed prior to commit: https://reviews.llvm.org/D20561?vs=61089=63970#toc Repository: rL LLVM

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-13 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. In http://reviews.llvm.org/D20561#482963, @rogfer01 wrote: > If there are not any objections I will commit this tomorrow. If @rsmith doesn't comment by tomorrow, then I think any feedback can be handled post-commit instead. http://reviews.llvm.org/D20561

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-13 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. If there are not any objections I will commit this tomorrow. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-08 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Ping? Looking forward any further comments before committing. Thank you very much. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20561: Warn when taking address of packed member

2016-07-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Ping? Any comment on this? Thank you very much. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-24 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. In http://reviews.llvm.org/D20561#466545, @rogfer01 wrote: > Ping? Any further comments, thoughts? > > Thank you very much. The usual practice is to ping after a week has gone by, btw. There were standards meetings this week, so that may be delaying some of the

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-24 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Ping? Any further comments, thoughts? Thank you very much. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-21 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Ping? Thank you very much. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-17 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated this revision to Diff 61089. rogfer01 added a comment. Following @rsmith suggestion, the diagnostic is silenced if the address is converted to a pointer with lower or equal alignment requirements. http://reviews.llvm.org/D20561 Files:

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-17 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked an inline comment as done. rogfer01 added a comment. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-16 Thread Richard Smith via cfe-commits
rsmith added a comment. I agree, that looks like correct behavior for the warning. Thanks! Comment at: lib/Sema/SemaCast.cpp:259-260 @@ -258,2 +258,4 @@ return ExprError(); + if (DestType->isVoidPointerType()) +DiscardMisalignedMemberAddress(E); }

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-16 Thread Evgeniy Stepanov via cfe-commits
eugenis added a comment. This timeval thing looks like a legitimate warning to me. I don't think the analysis should go beyond the function boundaries. If a callee expects timeval * as part of its signature it should get a properly aligned timeval *. http://reviews.llvm.org/D20561

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-16 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Firefox build shows a couple of warnings in the sctp library that already gave warnings with the earlier patches. That code has the following structure. #define SCTP_PACKED __attribute__((packed)) #define SCTP_IDENTIFICATION_SIZE 16 // ... /* state cookie

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-16 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated the summary for this revision. rogfer01 updated this revision to Diff 60951. rogfer01 added a comment. Thanks @rsmith for the suggestions. I removed the whitelisting totally and changed the strategy. This patch implements your second suggestion which seemed more obvious to me.

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-14 Thread Richard Smith via cfe-commits
rsmith added a comment. This still looks like it will have has lots of false positives for cases like: struct __attribute__((packed)) A { char c; int n; } a; void *p = It also looks like it will now have false negatives for cases like: memcpy(x, y, *); I think whitelisting

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-14 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. A build of Firefox does not show any false positive now. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-14 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 removed rL LLVM as the repository for this revision. rogfer01 updated this revision to Diff 60664. rogfer01 added a comment. This new patch adds a whitelist for memcpy, memmove, memset and memcmp. I'm not entirely happy with the strategy that I'm using, where Parser communicates to

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-14 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 reopened this revision. rogfer01 added a comment. This revision is now accepted and ready to land. I've reverted the commit. During implementation of the white list I felt the changes were too much for a follow up commit, so I prefer some feedback first. Repository: rL LLVM

RE: [PATCH] D20561: Warn when taking address of packed member

2016-06-14 Thread Roger Ferrer Ibanez via cfe-commits
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,

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Aaron Ballman via cfe-commits
On Mon, Jun 13, 2016 at 7:17 PM, Evgenii Stepanov wrote: > On Mon, Jun 13, 2016 at 4:12 PM, Aaron Ballman > wrote: >> On Mon, Jun 13, 2016 at 6:30 PM, Evgeniy Stepanov wrote: >>> eugenis added a subscriber: eugenis. >>> eugenis

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Evgenii Stepanov via cfe-commits
On Mon, Jun 13, 2016 at 4:12 PM, Aaron Ballman wrote: > On Mon, Jun 13, 2016 at 6:30 PM, Evgeniy Stepanov wrote: >> eugenis added a subscriber: eugenis. >> eugenis added a comment. >> >> In http://reviews.llvm.org/D20561#446031, @aaron.ballman wrote:

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Aaron Ballman via cfe-commits
On Mon, Jun 13, 2016 at 6:30 PM, Evgeniy Stepanov 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

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Evgeniy Stepanov via cfe-commits
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

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Roger Ferrer Ibanez via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL272552: Warn when taking address of a packed member (authored by rogfer01). Changed prior to commit: http://reviews.llvm.org/D20561?vs=59342=60537#toc Repository: rL LLVM

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Thank you Aaron. I will commit ASAP. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-13 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. I think Richard has had enough time to comment. You can go ahead and commit, and if there are any issues, they can be handled post-commit. Thanks! http://reviews.llvm.org/D20561 ___ cfe-commits mailing list

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-08 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Ping? Thanks a lot. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-02 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked an inline comment as done. rogfer01 added a comment. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-02 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think the patch LGTM (with a minor formatting nit). @rsmith, what are your thoughts? Comment at: test/SemaCXX/address-packed.cpp:92 @@ +91,3 @@ +void g1() +{

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-02 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated this revision to Diff 59342. rogfer01 marked 3 inline comments as done. rogfer01 added a comment. Remove fix-it as it is not suitable for this diagnostic. http://reviews.llvm.org/D20561 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-02 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked 3 inline comments as done. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5392 @@ -5388,1 +5391,3 @@ +def note_address_of_packed_member_silence : Note< + "place parentheses around the '%0' expression to silence this warning">;

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Aaron Ballman via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. 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

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated this revision to Diff 59241. rogfer01 added a comment. This change adds a fix-it suggesting parentheses to silence the warning. http://reviews.llvm.org/D20561 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/address-packed.c

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked 5 inline comments as done. rogfer01 added a comment. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. 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

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked 2 inline comments as done. rogfer01 added a comment. Well, the assignment-vs-comparison warning does emit a fix-it in `Sema::DiagnoseAssignmentAsCondition(Expr *E)` Diag(Loc, diag::note_condition_assign_silence) << FixItHint::CreateInsertion(Open, "(") <<

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. In http://reviews.llvm.org/D20561#445730, @rogfer01 wrote: > It came to my mind that might be good idea adding one of those "fix-it" > suggestions so the user knows it can silence the warning by using > parentheses. What do you think? I don't think that would

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. It came to my mind that might be good idea adding one of those "fix-it" suggestions so the user knows it can silence the warning by using parentheses. What do you think? http://reviews.llvm.org/D20561 ___ cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. This is getting really close, I mostly only have nits left. Comment at: lib/Sema/SemaExpr.cpp:10529 @@ +10528,3 @@ +bool ByteAligned = Context.getTypeAlignInChars(MD->getType()).isOne(); +if (ByteAligned) // packed has had not any effect

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked an inline comment as done. rogfer01 added a comment. http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-06-01 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated this revision to Diff 59178. rogfer01 marked an inline comment as done. rogfer01 added a comment. Do not warn if the alignment of the type of the field is already 1 as packed does not have any effect on those fields. http://reviews.llvm.org/D20561 Files:

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-31 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10527 @@ +10526,3 @@ +if (RD->hasAttr() || +ME->getMemberDecl()->hasAttr()) { + Diag(OpLoc, diag::warn_taking_address_of_packed_member) rogfer01 wrote: > aaron.ballman wrote:

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-31 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked 3 inline comments as done. Comment at: lib/Sema/SemaExpr.cpp:10527 @@ +10526,3 @@ +if (RD->hasAttr() || +ME->getMemberDecl()->hasAttr()) { + Diag(OpLoc, diag::warn_taking_address_of_packed_member) aaron.ballman wrote: > Ah, I

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-31 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Ping? Thank you very much http://reviews.llvm.org/D20561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-27 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 updated this revision to Diff 58760. rogfer01 added a comment. Only warn if the expression is of the form this way &(a.x) can be used to silence the warning. http://reviews.llvm.org/D20561 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-27 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Forget my wrong comment about packed fields that might actually be aligned. It does not make sense. Regarding the way to selectively disable this, there is little syntax available at this point that we can leverage, so I propose to use parentheses as a way to disable

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-26 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 added a comment. Firefox build has ended and exposes 62 diagnostics, 44 unique ocurrences and 10 different diagnostics (shown below) in networking code. taking address of packed member 'address' of class or structure 'sctp_state_cookie' may result in an unaligned pointer value

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-26 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 marked 4 inline comments as done. Comment at: lib/Sema/SemaExpr.cpp:10527 @@ +10526,3 @@ +ME->getMemberDecl()->hasAttr()) { + Diag(OpLoc, diag::warn_taking_address_of_packed_member) + << ME->getMemberDecl() << RD; aaron.ballman

Re: [PATCH] D20561: Warn when taking address of packed member

2016-05-26 Thread Aaron Ballman via cfe-commits
aaron.ballman added a subscriber: aaron.ballman. aaron.ballman added a comment. Thank you for working on this diagnostic! A few questions and comments below. Comment at: lib/Sema/SemaExpr.cpp:10518 @@ +10517,3 @@ + // Taking the address of a data member/field of a packed + //

[PATCH] D20561: Warn when taking address of packed member

2016-05-24 Thread Roger Ferrer Ibanez via cfe-commits
rogfer01 created this revision. rogfer01 added a reviewer: rsmith. rogfer01 added a subscriber: cfe-commits. This patch implements PR#22821. Taking the address of a packed member is dangerous since the reduced alignment of the pointee is lost. This can lead to memory alignment faults in some