On Mon, Jun 12, 2017 at 4:04 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Fri, Jun 09, 2017 at 12:30:10PM -0700, Jason Merrill wrote:
>> On Thu, Jun 8, 2017 at 12:30 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>> > cp_genericize_r now instruments INTEGER_CSTs that have REFERENCE_TYPE,
>> > so that we can diagnose binding references to NULL in some cases,
>> > see PR79572. As the following testcase shows, there is one exception
>> > when we do not want to do that - in MEM_EXPR, the second operand
>> > is an INTEGER_CST whose value is an offset, but type is something
>> > unrelated - what should be used for aliasing purposes. So, that
>> > is something we do not want to diagnose, and it is also invalid IL,
>> > as the second argument has to be an INTEGER_CST, not some expression
>> > with side-effects.
>> >
>> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
>> > ok for trunk/7.x?
>> >
>> > PR c++/80973
>> > * cp-gimplify.c (cp_genericize_r): Don't instrument MEM_REF second
>> > argument even if it has REFERENCE_TYPE.
>>
>> I wonder if we want to handle this in walk_tree_1, so all tree walks
>> by default avoid the second operand.
>
> MEM_REF has the second argument and there could be callbacks that really
> want to walk even that argument for whatever reason (gather all types
> mentioned in some expression, whatever). Implying from one place where
> we do not want that what other code might want to do doesn't look right.
Well, such callbacks could walk it specifically, but...
> But, if you want, the patch could be simplified, handle the MEM_REF
> in there this way unconditionally, so:
> else if (TREE_CODE (stmt) == MEM_REF)
> {
> cp_walk_tree (&TREE_OPERAND (stmt, 0), cp_genericize_r, data, NULL);
> *walk_subtrees = 0;
> }
> before the sanitizer block, perhaps with some comments explaining it,
> because we know cp_genericize_r doesn't need to have the callback
> on the second MEM_REF argument.
...this approach is fine, too.
Jason
