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
