On Tue, Sep 10, 2013 at 2:50 PM, Aaron Ballman <[email protected]>wrote:
> On Tue, Sep 10, 2013 at 5:40 PM, Eli Friedman <[email protected]> > wrote: > > On Tue, Sep 10, 2013 at 2:33 PM, Aaron Ballman <[email protected]> > > wrote: > >> > >> On Tue, Sep 10, 2013 at 5:21 PM, Eli Friedman <[email protected]> > >> wrote: > >> > On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman < > [email protected]> > >> > wrote: > >> >> > >> >> Currently, if you have a value assignment involving structures > >> >> containing a flexible array, no diagnostic is emitted. However, > since > >> >> the array members won't be copied as part of the assignment, a > >> >> diagnostic will help programmers avoid bugs. This patch emits said > >> >> diagnostic. > >> >> > >> > > >> > +def warn_flexible_array_assignment : ExtWarn< > >> > + "assignment of flexible arrays does not copy the array members">, > >> > + InGroup<FlexibleArrayExtensions>; > >> > > >> > I assume this is supposed to be a Warning, not an ExtWarn? > >> > >> Yes, thanks for pointing that out. > >> > >> > Also, I'm not sure putting this into the FlexibleArrayExtensions > >> > diagnostic > >> > group is the best idea. > >> > >> I wasn't certain either, but it was the closest existing group. Would > >> it make more sense to add a new group for it? > > > > > > I think so. > > Any complaints with naming the group flexible-arrays? > That's sort of vague; maybe flexible-array-slicing? > > > > >> > >> > What sort of mistakes do you expect this to catch in practice? > >> > >> Simple programmer mistakes, mostly. It also nicely covers a CERT > >> secure C coding rule: > >> > >> > >> > https://www.securecoding.cert.org/confluence/display/seccode/MEM33-C.++Allocate+and+copy+structures+containing+flexible+array+members+dynamically > > > > > > Okay. > > > > Do you have any idea what the false positive rate is? > > I would expect it to be quite low, but have nothing concrete to back > that up. If the warning turns out to be chatty for some reason, it > could be tweaked easily enough (esp with a separate diagnostic group). > Okay. > > > >> > >> > >> > >> > Have you considered warning about the variable declaration rather than > >> > the > >> > assignment? In your testcase, it's impossible to call foo() without > >> > triggering the warning, so it seems better to warn about foo() itself > >> > rather > >> > than the call. > >> > >> Hmmm, would there ever be a case where it would make sense to declare > >> a structure with a flexible array member as a value type?The only > >> > >> situation I could think of would be overlaying the value type with > >> stack-allocated memory in some sort of bizarre union type punning > >> scenario. So I'm thinking that may be a better approach than checking > >> on assignment, unless there are intermediary ways you could get this > >> assignment to happen in C. > >> > > > > Well, you can write something like "*a = *b;", which doesn't involve > > declaring a variable... > > True. Assignment would be the catch-all way to do it, since that's > what causes the slicing behavior. Perhaps both declaration and > assignment diagnostics would make sense? Declarations like that are > definitively bizarre, but assignment still misbehaves. > > The CERT page classifies the mistakes into three categories; covering all of them seems reasonable. -Eli
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
