On 6/22/20 12:55 PM, Jason Merrill wrote:
> On 6/22/20 1:25 PM, Martin Sebor wrote:
>> The attached fix parallels the one for the equivalent C bug 95580
>> where the pretty printers don't correctly handle MEM_REF arguments
>> with type void* or other pointers to an incomplete type.
>>
>> The incorrect handling was exposed by the recent change to
>> -Wuninitialized which includes such expressions in diagnostics.
> 
>> +        if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
>> +          if (!integer_onep (size))
>> +            {
>> +              pp_cxx_left_paren (pp);
>> +              dump_type (pp, ptr_type_node, flags);
>> +              pp_cxx_right_paren (pp);
>> +            }
> 
> Don't we want to print the cast if the pointer target type is incomplete?

I suppose, yes, although after some more testing I think what should
be output is the type of the access.  The target pointer type isn't
meaningful (at least not in this case).

Here's what the warning looks like in C for the test case in
gcc.dg/pr95580.c:

   warning: ‘*((void *)(p)+1)’ may be used uninitialized

and like this in C++:

   warning: ‘*(p +1)’ may be used uninitialized

The +1 is a byte offset, which is correct given that incrementing
a void* in GCC is the same as adding 1 to the byte address, but
dereferencing a void* doesn't correspond to what's going on in
the source.

Even for a complete type (with size greater than 1), printing
the type of the argument plus a byte offset is wrong.  It ends
up with this for the C++ test case from 95768:

   warning: ‘*((int*)<unknown> +4)’ is used uninitialized

when the access is actually ‘*((int*)<unknown> +1)’

So it seems to me for MEM_REF, to make the output meaningful,
it's the type of the access (i.e., the MEM_REF type) that should
be printed here, and the offset should either be in elements of
the accessed type, i.e.,

   warning: ‘*((int*)<unknown> +1)’ is used uninitialized

or, if the access is misaligned, the argument should first be
cast to char*, the offset added, and the result then cast to
the access type, like this:

   warning: ‘*(T*)((char*)<unknown> +1)’ is used uninitialized

The attached revised and less than fully tested patch implements
this for C++ only for now.  If we agree on this approach I'll see
about making the corresponding change in C.

Martin