Hi, Jason,
> On Feb 11, 2022, at 11:27 AM, Jason Merrill <ja...@redhat.com> wrote:
>>>>
>>>> Sure, we might as well make this code more robust. But we can do better
>>>> than <unnamed type> if we check TYPE_PTRMEMFUNC_P.
>>> Okay, so what should we print to the user if it's “TYPE_PTRMEMFUNC_P”?
>>> Print nothing or some special string?
>>>>
>>>>> 2. The second level issue is what you suggested in the above, shall we
>>>>> print the “compiler generated internal type” to the user? And I agree
>>>>> with you that it might not be a good idea to print such compiler internal
>>>>> names to the user. Are we do this right now in general? (i.e, check
>>>>> whether the current TYPE is a source level TYPE or a compiler internal
>>>>> TYPE, and then only print out the name of TYPE for the source level
>>>>> TYPE?) and is there a bit in the TYPE to distinguish whether a TYPE is
>>>>> user -level type or a compiler generated internal type?
>>>>
>>>>>> I think the real problem comes sooner, when c_fold_indirect_ref_for_warn
>>>>>> turns a MEM_REF with RECORD_TYPE into a COMPONENT_REF with POINTER_TYPE.
>>>>
>>>>> What’s the major issue for this transformation? (I will study this in
>>>>> more details).
>>>>
>>>> We told c_fold_indirect_ref that we want a RECORD_TYPE (the PMF as a
>>>> whole) and it gave us back a POINTER_TYPE instead (the __pmf member).
>>>> Folding shouldn't change the type of an expression like that.
>>>
>>> Yes, this is not correct transformation, will study in more detail and try
>>> to fix it.
>> After a deeper study of commit r11-6729-gadb520606ce3e1e1 (which triggered
>> the ICE and introduced the new routine “c_fold_indirect_ref_for_warn”), from
>> my understanding, the above transformation from a RECORD_TYPE (the PMF as a
>> whole) to POINTER_TYPE (the __pmf member) is what the function intended to
>> do as following:
>> 1823 static tree
>> 1824 c_fold_indirect_ref_for_warn (location_t loc, tree type, tree op,
>> 1825 offset_int &off)
>> 1826 {
>> …
>> 1870 */* ((foo *)&struct_with_foo_field)[x] => COMPONENT_REF */*
>> 1871 else if (TREE_CODE (optype) == RECORD_TYPE)
>> 1872 {
>> 1873 for (tree field = TYPE_FIELDS (optype);
>> 1874 field; field = DECL_CHAIN (field))
>> 1875 if (TREE_CODE (field) == FIELD_DECL
>> …
>> 1886 if(upos <= off && off < upos + el_sz)
>> 1887 {
>> 1888 tree cop = build3_loc (loc, COMPONENT_REF, TREE_TYPE
>> (field),
>> 1889 op, field, NULL_TREE);
>> 1890 off = off - upos;
>> The above code was used to transform a MEM_REF to a RECORD_TYPE to a
>> COMPONENT_REF to the corresponding FIELD.
>
> Yes, that's what the above code would correctly do if TYPE were the
> pointer-to-method type. It's wrong for this case because TYPE is unrelated
> to TREE_TYPE (field).
>
> I think the problem is just this line:
>
>> if (tree ret = c_fold_indirect_ref_for_warn (loc, type, cop,
>> off))
>> return ret;
>> return cop;
> ^^^^^^^^^^
>
> The recursive call does the proper type checking, but then the "return cop"
> line returns the COMPONENT_REF even though the type check failed. The
> parallel code in cxx_fold_indirect_ref_1 doesn't have this line,
Just compared the routine “cxx_fold_indirect_ref_1” and
“c_fold_indirect_ref_for_warn”, looks like there are more places that have such
difference, for example,
In “cxx_fold_indirect_ref_1”:
/* ((foo *)&fooarray)[x] => fooarray[x] */
else if (TREE_CODE (optype) == ARRAY_TYPE
&& tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (optype)))
&& !integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (optype))))
…
if (tree_fits_uhwi_p (min_val))
{
tree index = size_int (idx + tree_to_uhwi (min_val));
op = build4_loc (loc, ARRAY_REF, TREE_TYPE (optype), op, index,
NULL_TREE, NULL_TREE);
return cxx_fold_indirect_ref_1 (ctx, loc, type, op, rem,
empty_base);
}
However, in “c_fold_indirect_ref_for_warn”, the corresponding part is:
/* ((foo *)&fooarray)[x] => fooarray[x] */
if (TREE_CODE (optype) == ARRAY_TYPE
&& TYPE_SIZE_UNIT (TREE_TYPE (optype))
&& TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (optype))) == INTEGER_CST
&& !integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (optype))))
…
if (TREE_CODE (min_val) == INTEGER_CST)
{
tree index
= wide_int_to_tree (sizetype, idx + wi::to_offset (min_val));
op = build4_loc (loc, ARRAY_REF, TREE_TYPE (optype), op, index,
NULL_TREE, NULL_TREE);
off = rem;
if (tree ret = c_fold_indirect_ref_for_warn (loc, type, op, off))
return ret;
return op;
}
The exactly same difference as for “RECORD_TYPE”. So, I suspect that it’s a
typo for “RECORD_TYPE” in “c_fold_indirect_ref_for_warn”.
> and removing it fixes the testcase, so I see
>
> warning: ‘*(ptrmemfunc*)&x.ptrmemfunc::ptr’ is used uninitialized
The question is:
For the following IR:
struct sp x;
void (*<T389>) (void) _1;
...
<bb 2> [local count: 1073741824]:
_1 = MEM[(struct ptrmemfunc_U *)&x].ptr;
_7 = _1 != 8B;
Which message is better:
1. warning: ‘*(ptrmemfunc*)&x.ptrmemfunc::ptr’ is used uninitialized
Or
2. warning: ‘*(ptrmemfunc*)((char*)&x + offsetof(void
(S::*)(),__PTRMEMFUNC)).ptrmemfunc::ptr’ is used uninitialized
From the source code:
====
struct S
{
int j;
};
struct T : public S
{
virtual void h () {}
};
struct ptrmemfunc
{
void (*ptr) ();
};
typedef void (S::*sp)();
int main ()
{
T t;
sp x;
ptrmemfunc *xp = (ptrmemfunc *) &x;
if (xp->ptr != ((void (*)())(sizeof(void *))))
return 1;
}
====
The reference “xp->ptr” went through from “x” to “xp”, and there is a clear
type casting from S::__PTRMEMFUNC to ptrmemfunc::ptr.
Shall we emit such type casting to the user?
Qing
> Jason
>
