On 3/18/22 14:20, Jakub Jelinek wrote:
On Fri, Mar 18, 2022 at 01:35:53PM -0400, Jason Merrill wrote:
On 3/15/22 12:06, Jakub Jelinek wrote:
On Tue, Mar 15, 2022 at 11:57:22AM -0400, Jason Merrill wrote:
The intent of r11-6729 is that it prints something that helps user to figure
out what exactly is being accessed.
When we find a unique non-static data member that is being accessed, even
when we can't fold it nicely, IMNSHO it is better to print
((sometype *)&var)->field
or
(*(sometype *)&var).field
instead of
*(fieldtype *)((char *)&var + 56)
because the user doesn't know what is at offset 56, we shouldn't ask user
to decipher structure layout etc.
The problem is that the reference is *not* to any non-static data member,
it's to the PMF as a whole. But c_fold_indirect_ref_for_warn wrongly turns
it into a reference to the first non-static data member.
We asked c_fold_indirect_ref_warn to fold a MEM_REF with RECORD_TYPE, and it
gave us back a COMPONENT_REF with POINTER_TYPE. That seems clearly wrong.
That is not what I see on the testcase.
I see the outer c_fold_indirect_ref_for_warn call with type ptrmemfunc
which is a 64-bit RECORD_TYPE containing a single ptr member which has
pointer to function type, and op which is the x VAR_DECL with sp type which
is 128-bit RECORD_TYPE containing 64-bit __pfn member and 64-bit __delta
member.
Exactly: TYPE is RECORD_TYPE, TREE_TYPE (field) is POINTER_TYPE.
As all the bits of the ptrmemfunc RECORD_TYPE fit within the __pfn member
(they are equal size), it wants to print (cast)(something.__pfn).
I disagree that this is what we want; we asked to fold an expression with
class type, it seems unlikely that we want to get back an expression with
pointer type.
That doesn't matter. What c_fold_indirect_ref_warn returns is something
that can help the user understand what is actually being accessed.
Consider slightly modified testcase (which doesn't use the PMFs so that
we don't ICE on those too):
// PR c++/101515
// { dg-do compile }
// { dg-options "-O1 -Wuninitialized" }
struct S { int j; };
struct T : public S { virtual void h () {} };
struct U { char buf[32]; void (*ptr) (); };
struct sp { char a[14]; char b[50]; void (*pfn) (); long delta; };
int
main ()
{
T t;
sp x;
U *xp = (U *) &x.b[18];
if (xp->ptr != ((void (*) ()) (sizeof (void *))))
return 1;
}
Trunk emits:
pr101515-2.C: In function ‘int main()’:
pr101515-2.C:16:11: warning: ‘*(U*)((char*)&x + offsetof(sp,
sp::b[18])).U::ptr’ is used uninitialized [-Wuninitialized]
16 | if (xp->ptr != ((void (*) ()) (sizeof (void *))))
| ~~~~^~~
pr101515-2.C:14:6: note: ‘x’ declared here
14 | sp x;
| ^
here, which is indeed quite long expression, but valid C++ and helps the
user to narrow down what exactly is being accessed.
If I comment out the c_fold_indirect_ref_warn RECORD_TYPE handling so that
it punts on it, it prints:
pr101515-2.C: In function ‘int main()’:
pr101515-2.C:16:11: warning: ‘*(U*)((char*)&x + 32).U::ptr’ is used
uninitialized [-Wuninitialized]
16 | if (xp->ptr != ((void (*) ()) (sizeof (void *))))
| ~~~~^~~
pr101515-2.C:14:6: note: ‘x’ declared here
14 | sp x;
| ^
That is also correct C++ expression, but user probably has no idea what is
present at offset 32 into the variable.
Of course if there is a type match and not any kind of type punning,
it will try to print much shorter and more readable expressions.
One important point about the OP's testcase is that we're dealing with
offset 0, which corresponds to both the PMF as a whole and its first
element. Since we're looking for a RECORD_TYPE, choosing to interpret
the desired object as the (RECORD_TYPE) PMF as a whole seems like the
better choice; being more flexible probably does make sense at non-0
offsets.
Jason
