Re: [PATCH] unshare expressions in attribute arguments

[Jeff Law <law@redhat.com>]

On 11/20/20 12:00 PM, 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?
So if the tree node is reachable from a GC root, then it won't be
removed by the GC system.     It's a simple mark/sweep with a set of
registered roots.  The only real complexity is the auto-generated code
to walk the data structures (ie, all the gengtype insanity).

From the BZ:


 <tree_list 0x7fffea924f28
    value <tree_list 0x7fffea924d20
        value <plus_expr 0x7fffea924c80 type <integer_type
0x7fffea8105e8 int>
            arg:0 <var_decl 0x7ffff7ffbb40 n>
            arg:1 <integer_cst 0x7fffea815090 constant 1>
            /build/tmp/z.c:2:48 start: /build/tmp/z.c:2:46 finish:
/build/tmp/z.c:2:50>>>

Then later indicate it looks like this (presumably at LTO stream-out time):


 <tree_list 0x7fffea924ed8
    value <tree_list 0x7fffea924cf8
        value <plus_expr 0x7fffea924c80 type <integer_type
0x7fffea8105e8 int>
          
            arg:0 <ssa_name 0x7fffea801cf0 type <error_mark 0x7fffea7f7cc0>
                nothrow
                def_stmt
                version:1 in-free-list>
            arg:1 <integer_cst 0x7fffea815090 constant 1>
            /build/tmp/z.c:2:55 start: /build/tmp/z.c:2:45 finish:
/build/tmp/z.c:2:57>>>


Note the structure of the value in the tree list, in particular note the
PLUS_EXPR node.  It's at address 0x7fffea924c80 in both.  But in the
first it's a VAR_DECL.  In the second it's a released SSA_NAME.


That to me doesn't look like a GC issue.  To me it looks like you have
violated the structure sharing assumptions by inadvertently sharing the
PLUS_EXPR node.  Naturally when the gimplifier and SSA renaming does its
thing, the first operand of the PLUS_EXPR gets changed to an SSA_NAME. 
I strongly suspect that SSA_NAME ultimately ends up dead and gets
released back to the SSA_NAME manager for re-use (hence the
error_mark_node for the type and in-free-list tag for arg0 of the
PLUS_EXPR in the second instance).

So the first question is presumably you want the original form with the
_DECL node?   That argues that you need the unshare_expr so that your
copy is independent of the actions of gimplification and SSA renaming. 
However, as Jakub noted, there may be a SAVE_EXPR issue that needs to be
addressed here.



jeff