Re: [PATCH] Fix PR 101515 (ICE in pp_cxx_unqualified_id, at cp/cxx-pretty-print.c:128)

[Qing Zhao <qing.zhao@oracle.com>]

> On Feb 8, 2022, at 4:20 PM, Jason Merrill <jason@redhat.com> wrote:
> 
> On 2/8/22 15:11, Qing Zhao wrote:
>> Hi,
>> This is the patch to fix PR101515 (ICE in pp_cxx_unqualified_id, at  cp/cxx-pretty-print.c:128)
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101515
>> It's possible that the TYPE_NAME of a record_type is NULL, therefore when
>> printing the TYPE_NAME, we should check and handle this special case.
>> Please see the comment of pr101515 for more details.
>> The fix is very simple, just check and special handle cases when TYPE_NAME is NULL.
>> Bootstrapped and regression tested on both x86 and aarch64, no issues.
>> Okay for commit?
>> Thanks.
>> Qing
>> =====================================
>> From f37ee8d21b80cb77d8108cb97a487c84c530545b Mon Sep 17 00:00:00 2001
>> From: Qing Zhao <qing.zhao@oracle.com>
>> Date: Tue, 8 Feb 2022 16:10:37 +0000
>> Subject: [PATCH] Fix PR 101515 ICE in pp_cxx_unqualified_id, at
>>  cp/cxx-pretty-print.c:128.
>> It's possible that the TYPE_NAME of a record_type is NULL, therefore when
>> printing the TYPE_NAME, we should check and handle this special case.
>> gcc/cp/ChangeLog:
>> 	* cxx-pretty-print.cc (pp_cxx_unqualified_id): Check and handle
>> 	the case when TYPE_NAME is NULL.
>> gcc/testsuite/ChangeLog:
>> 	* g++.dg/pr101515.C: New test.
>> ---
>>  gcc/cp/cxx-pretty-print.cc      |  5 ++++-
>>  gcc/testsuite/g++.dg/pr101515.C | 25 +++++++++++++++++++++++++
>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/g++.dg/pr101515.C
>> diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
>> index 4f9a090e520d..744ed0add5ba 100644
>> --- a/gcc/cp/cxx-pretty-print.cc
>> +++ b/gcc/cp/cxx-pretty-print.cc
>> @@ -171,7 +171,10 @@ pp_cxx_unqualified_id (cxx_pretty_printer *pp, tree t)
>>      case ENUMERAL_TYPE:
>>      case TYPENAME_TYPE:
>>      case UNBOUND_CLASS_TEMPLATE:
>> -      pp_cxx_unqualified_id (pp, TYPE_NAME (t));
>> +      if (TYPE_NAME (t))
>> +	pp_cxx_unqualified_id (pp, TYPE_NAME (t));
>> +      else
>> +	pp_string (pp, "<unnamed type>");
> 
> Hmm, but it's not an unnamed class, it's a pointer to member function type, and it would be better to avoid dumping compiler internal representations like the __pfn field name.
Yes, It’s not an unnamed class, but the ICE happened when try to print the compiler generated member function type “__ptrmemfunc_type”, whose TYPE_NAME is NULLed during building this type in c++ FE and the c++ FE does not handle the case when TYPE_NAME is NULL correctly. 

So, there are two levels of issues:

1. The first level issue is that the current C++ FE does not handle the case TYPE_NAME being NULL correctly, this is indeed a bug in the current code and should be fixed as in the current patch. 

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).

thanks.

Qing


> 
>>        if (tree ti = TYPE_TEMPLATE_INFO_MAYBE_ALIAS (t))
>>  	if (PRIMARY_TEMPLATE_P (TI_TEMPLATE (ti)))
>>  	  {
>> diff --git a/gcc/testsuite/g++.dg/pr101515.C b/gcc/testsuite/g++.dg/pr101515.C
>> new file mode 100644
>> index 000000000000..898c7e003c22
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr101515.C
>> @@ -0,0 +1,25 @@
>> +/* PR101515 -  ICE in pp_cxx_unqualified_id, at cp/cxx-pretty-print.c:128
>> +   { dg-do compile }
>> +   { dg-options "-Wuninitialized -O1" } */
>> +
>> +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 *))))  /* { dg-warning "is used uninitialized" } */
>> +    return 1;
>> +}
>