[PATCH] diagnosing flexible array assignments

[Aaron Ballman]

On Tue, Sep 10, 2013 at 5:40 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Tue, Sep 10, 2013 at 2:33 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Tue, Sep 10, 2013 at 5:21 PM, Eli Friedman <eli.friedman at gmail.com>
>> wrote:
>> > On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman <aaron at aaronballman.com>
>> > 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?

>
>>
>> > 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).
>
>>
>>
>>
>> > 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.

Thanks!

~Aaron