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