Re: [PATCH] Fix ICE in modified_type_die (PR debug/80461)

[Jason Merrill <jason at redhat dot com>]

On Wed, Apr 19, 2017 at 12:29 PM, Jeff Law <law@redhat.com> wrote:
> On 04/19/2017 10:13 AM, Jakub Jelinek wrote:
>>
>> On Wed, Apr 19, 2017 at 09:55:19AM -0600, Jeff Law wrote:
>>>
>>> On 04/19/2017 09:10 AM, Jakub Jelinek wrote:
>>>>
>>>> On Wed, Apr 19, 2017 at 08:57:52AM -0600, Jeff Law wrote:
>>>>>>
>>>>>> 2017-04-19  Jakub Jelinek  <jakub@redhat.com>
>>>>>>
>>>>>>         PR debug/80461
>>>>>>         * dwarf2out.c (modified_type_die, gen_type_die_with_usage):
>>>>>>         Check for t with zero TYPE_QUALS_NO_ADDR_SPACE.
>>>>>>
>>>>>>         * g++.dg/debug/pr80461.C: New test.
>>>>>
>>>>> I'm going to assume your use of TYPE_QUALS_NO_ADDR_SPACE vs TYPE_QUALS
>>>>> or
>>>>> TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC is correct.
>>>>
>>>>
>>>> I don't really know.  For address space quals I think one would need to
>>>> have
>>>> pointer-to-members in different code address spaces, not sure if we
>>>> support
>>>> anything like that.  And atomic is C only which doesn't have
>>>> pointer-to-members.
>>>
>>> To put it another way, in your message you indicated that
>>> modified_type_die
>>> expects either an unqualified type or the type itself and that your patch
>>> makes sure we only pick unqualified types.
>>>
>>> Using TYPE_QUALS_NO_ADDR_SPACE like you have seems to conflict with those
>>> statements.  So I'm curious why you allow address space qualifiers to
>>> pass
>>> through, but no others.   It just seems odd.
>>
>>
>> I used TYPE_QUALS_NO_ADDR_SPACE because that seems to be used in
>> modified_type_die elsewhere (dwarf2out.c has only one use of TYPE_QUALS
>> and
>> even that one ignores addr space bits, as it is masked with qual_mask;
>> the rest just TYPE_QUALS_NO_ADDR_SPACE).  I think with FUNCTION/METHOD
>> types
>> with addr space quals we end up in grossly untested territory that most
>> likely will just not work at all.  I think we don't do anything with the
>> address space quals right now, in the future DWARF has segment identifiers
>> and something like that could be emitted, but there needs to be ABI
>> agreement on what it means.
>
> OK.  So let's go with your patch -- my reading of modified_type_die was that
> it would ignore address-space qualifiers.  So I think your patch is safe, it
> was the inconsistency in the message and the patch itself I was most
> concerned about.

BTW, I think it would be a bit more correct to compare the quals with
those of "main" rather than compare them to 0, though I suspect there
aren't currently any types for which that produces a different result.

Jason