[PATCH 2/5] C front end support to detect out-of-bounds accesses to array parameters

Martin Sebor msebor@gmail.com
Wed Aug 19 22:56:47 GMT 2020


On 8/17/20 4:09 PM, Joseph Myers wrote:
> On Thu, 13 Aug 2020, Martin Sebor via Gcc-patches wrote:
> 
>>> * Maybe cdk_pointer is followed by cdk_attrs before cdk_id.  In this case
>>> the code won't return.
>>
>> I think I see the problem you're pointing out (I just don't see how
>> to trigger it or test that it doesn't happen).  If the tweak in
>> the attached update doesn't fix it a test case would be helpful.
> 
> I think you need a while loop there, not just an if, to account for the
> case of multiple consecutive cdk_attrs.  At least the GNU attribute syntax
> 
>     direct-declarator:
> [...]
>       ( gnu-attributes[opt] declarator )
> 
> should produce multiple consecutive cdk_attrs for each level of
> parentheses with attributes inside.

I had considered a loop but couldn't find a way to trigger what you
describe (or a test in the testsuite that would do it) so I didn't
use one.  I saw loops like that in other places but I couldn't get
even those to uncover such a test case.  Here's what I tried:

   #define A(N) __attribute__ ((aligned (N), may_alias))
   int n;
   void f (int (* A (2) A (4) (* A (2) A (4) (* A (2) A (4) [n])[n])));

Sequences of consecutive attributes are all chained together.

I've added the loop here but I have no test for it.  It would be
good to add one if it really is needed.

> 
>>> * Maybe the code is correct to continue because we're in the case of an
>>> array of pointers (cdk_array follows).  But as I understand it, the intent
>>> is to set up an "arg spec" that describes only the (multidimensional)
>>> array that is the parameter itself - not any array pointed to.  And it
>>> looks to me like, in the case of an array of pointers to arrays, both sets
>>> of array bounds would end up in the spec constructed.
>>
>> Ideally, I'd like to check even pointers to arrays and so they should
>> be recorded somewhere.  The middle end code doesn't do any checking
>> of those yet for out-of-bounds accesses.  It wasn't a goal for
>> the first iteration so I've tweaked the code to avoid recording them.
> 
> Could you expand the comment on get_parm_array_spec to specify exactly
> what you think the function should be putting in the returned attribute,
> in what order, in cases where there are array declarators (constant,
> empty, [*] and VLA) intermixed with other kinds of declarators and the
> type from the type specifiers may or may not be an array type itself?
> That will provide a basis for subsequent rounds of review of whether the
> function is actually behaving as expected.

Done.

> 
> As far as I can see, the logic
> 
> +      if (TREE_CODE (nelts) == INTEGER_CST)
> +       {
> +         /* Skip all constant bounds except the most significant one.
> +            The interior ones are included in the array type.  */
> +         if (next && (next->kind == cdk_array || next->kind == cdk_pointer))
> +           continue;
> 
> will skip constant bounds in an array that's the target of a pointer
> declarator, but not any other kind of bounds.  Is that what you intend -
> that all the other kind of bounds in pointed-to arrays will be recorded in
> this string?

The immediate goal (for the front end) is to detect differences
in the form ([] vs [N]) or value of the most significant bound
in parameters of array types (before they decay to pointers),
and differences in the form ([*] vs [n]) or (the presumed) value
(e.g., [n] vs [n + 1]) of VLA bounds.

So I need to encode every variable bound (specified or unspecified).
For ordinary arrays I want to encode just the form of the most
significant bound.

> 
>>> Then, the code
>>>
>>> +      if (pd->kind == cdk_id)
>>> +       {
>>> +         /* Extract the upper bound from a parameter of an array type.  */
>>>
>>> also seems misplaced.  If the type specifiers for the parameter are a
>>> typedef for an array type, that array type should be processed *before*
>>> the declarator to get the correct semantics (as if the bounds from those
>>> type specifiers were given in the declarator), not at the end which gets
>>> that type out of order with respect to array declarators.  (Processing
>>> before the declarator also means clearing the results of that processing
>>> if a pointer declarator is encountered at any point, because in that case
>>> the array type in the type specifiers is irrelevant.)
>>
>> I'm not sure I follow you here.  Can you show me what you mean on
>> a piece of code?  This test case (which IIUC does what you described)
>> works as expected:
>>
>> $ cat q.c && gcc -O2 -S -Wall q.c
>> typedef int A[7][9];
>>
>> void f (A[3][5]);
> 
> So this is equivalent to A[3][5][7][9].  The c_declarator structures have
> the one for the [3] (the top-level bound) inside the one for the [5].
> The [5] bound is skipped by the "Skip all constant bounds except the most
> significant one." logic.  When the [3] bound is reached, the "break;" at
> the end of that processing means the "Extract the upper bound from a
> parameter of an array type." never gets executed.  Try replacing the [3]
> bound by a VLA bound.  As I read the code, it will end up generating a
> spec string that records first the VLA, then the [7], when it should be
> first the 9 (skipped), then the 7 (skipped), then the 5 (skipped), then
> the VLA.  Or if it's "void f (A *[variable][5]);", it will do the same
> thing (VLA, then 7, although both the 7 and the 9 are part of the
> pointed-to type).

Replacing the constant bound with a variable one helped, thanks!
Attached is another update.  While working on it and making sure
the function avoids pointers to arrays I decided to add another
one that at least checks them for mismatches (they're ignored
otherwise).  It helped me get a better handle on these obscure
things.  I can't imagine anyone actually declaring a function
that takes a pointer to an array of pointers to arrays (I can't
even read it) but it was instructive nonetheless and helped me
find some of the problems you were pointing out that I couldn't
see.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-50584-2.diff
Type: text/x-patch
Size: 77621 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200819/74a4f40c/attachment-0001.bin>


More information about the Gcc-patches mailing list