[PATCH] unshare expressions in attribute arguments

Martin Sebor msebor@gmail.com
Fri Nov 20 21:30:43 GMT 2020


On 11/20/20 1:37 PM, Jakub Jelinek wrote:
> On Fri, Nov 20, 2020 at 01:28:03PM -0700, Martin Sebor via Gcc-patches wrote:
>> On 11/20/20 12:29 PM, Marek Polacek wrote:
>>> On Fri, Nov 20, 2020 at 12:00:58PM -0700, Martin Sebor via Gcc-patches wrote:
>>>> To detect a subset of VLA misuses, the C front associates the bounds
>>>> of VLAs in function argument lists with the corresponding variables
>>>> by implicitly adding an instance of attribute access to each function
>>>> declared to take VLAs with the bound expressions chained on the list
>>>> of attribute arguments.
>>>>
>>>> Some of these expressions end up modified by the middle end, which
>>>> results in references to nonlocal variables (and perhaps other nodes)
>>>> used in these expression getting garbage collected.  A simple example
>>>> of this is described in pr97172.
>>>>
>>>> By unsharing the bound expressions the patch below prevents this from
>>>> happening (it's not a fix for pr97172).
>>>>
>>>> My understanding of the details of node sharing and garbage collection
>>>> in GCC is very limited (I didn't expect a tree to be garbage-collected
>>>> if it's still referenced by something).  Is this the right approach
>>>> to solving this problem?
>>>
>>> ISTM that a more natural thing would be to use build_distinct_type_copy
>>> to copy the type you're about to modify.
>>
>> The get_parm_array_spec function doesn't modify a type.  It's called
>> from push_parm_decl() to build an "arg spec" attribute with the VLA
>> bounds as arguments.  push_parm_decl() then adds the attribute to
>> the function's PARM_DECL by calling decl_attributes().  When all of
>> the function's parameters have been processed the "arg specs" are
>> then extracted and added as an attribute access specification with
>> the VLA bounds added to the function declaration.
> 
> Guess it isn't that the trees would be GC collected, that can't happen if
> they are referenced from reachable trees, but the thing is that the
> gimplifier is destructive, it overwrites various trees as it is gimplifying
> function bodies.  That is why the function bodies are normally unshared, but
> that unsharing doesn't really walk function attributes.
> On the other side, for VLAs unsharing is quite harmful, e.g. if there is
>    int vla[foo ()];
> then if it is unshared (except when it is a SAVE_EXPR that wouldn't be
> unshared), then it could call the foo () function multiple times.
> For VLA bounds in PARM_DECLs we are hopefully more restricted than that,
> if it involves only other PARM_DECLs and constants and expressions composed
> of them, the unsharing could be fine.

VLA parameter bounds can involve any other expressions, including
function calls.  It's those rather than other parameters that also
trigger the problem (at least in the test cases I've seen).

When/how would the unsharing cause the expression to be evaluated
multiple times?  And if/when it did, would simply wrapping the whole
expression in a SAVE_EXPR be the right way to avoid it or would it
need to be more involved than that?

Martin



More information about the Gcc-patches mailing list