libiberty D tuple demangling

libiberty D tuple demangling

[Jan Beulich]

Hello,

while commit 3f30a274913b ("libiberty: Update D symbol demangling
for latest ABI spec") mentions in its description that tuple encoding
has changed, there's no real adjustment to dlang_parse_tuple() there,
nor are there any new (or replaced) test cases for that. Was this
simply overlooked?

Furthermore the current ABI specifies "B Parameters Z". As I don't
know what the old ABI said, I can only wonder whether the present
code decoding (in a loop) merely a Type (and not a Parameter) was
actually correct.

Thanks for any insight, Jan

Re: libiberty D tuple demangling

[ibuclaw]

> On 25/07/2022 08:45 CEST Jan Beulich <jbeulich@suse.com> wrote:
> 
>  
> Hello,
> 
> while commit 3f30a274913b ("libiberty: Update D symbol demangling
> for latest ABI spec") mentions in its description that tuple encoding
> has changed, there's no real adjustment to dlang_parse_tuple() there,
> nor are there any new (or replaced) test cases for that. Was this
> simply overlooked?
> 

Hi Jan,

Is there any specific example that fails to demangle, or are you just skimming?

From what I recall, there is a couple places in the dlang_demangle parser that handle ambiguities in a mangled symbol.  The ABI change only added a terminating 'Z', which makes said code that handles ambiguity redundant - but of course kept around so we handle both old and new symbols.

> Furthermore the current ABI specifies "B Parameters Z". As I don't
> know what the old ABI said, I can only wonder whether the present
> code decoding (in a loop) merely a Type (and not a Parameter) was
> actually correct.
> 

Do you think we should instead be calling dlang_function_args instead?

(Having a quick look at both, that does seem to be the case).

Iain.

Re: libiberty D tuple demangling

[Jan Beulich]

On 25.07.2022 14:05, ibuclaw@gdcproject.org wrote:
>> On 25/07/2022 08:45 CEST Jan Beulich <jbeulich@suse.com> wrote:
>> while commit 3f30a274913b ("libiberty: Update D symbol demangling
>> for latest ABI spec") mentions in its description that tuple encoding
>> has changed, there's no real adjustment to dlang_parse_tuple() there,
>> nor are there any new (or replaced) test cases for that. Was this
>> simply overlooked?
> 
> Is there any specific example that fails to demangle, or are you just skimming?

I'm merely looking at the code alongside the ABI spec.

> From what I recall, there is a couple places in the dlang_demangle parser that handle ambiguities in a mangled symbol.  The ABI change only added a terminating 'Z', which makes said code that handles ambiguity redundant - but of course kept around so we handle both old and new symbols.

It's not just the addition of Z at the end but also the dropping of the
number of elements at the beginning, aiui. It's actually that aspect
which caught my attention, since the ABI doesn't talk about any number
there, but the code fetches one.

>> Furthermore the current ABI specifies "B Parameters Z". As I don't
>> know what the old ABI said, I can only wonder whether the present
>> code decoding (in a loop) merely a Type (and not a Parameter) was
>> actually correct.
>>
> 
> Do you think we should instead be calling dlang_function_args instead?
> 
> (Having a quick look at both, that does seem to be the case).

Well - with a number of elements specified, it might have needed to
be a function processing a single argument only. For the new ABI -
yes, that's the function I would have expected to be called.

Jan

Re: libiberty D tuple demangling

[ibuclaw]

> On 25/07/2022 14:13 CEST Jan Beulich <jbeulich@suse.com> wrote:
> 
>  
> On 25.07.2022 14:05, ibuclaw@gdcproject.org wrote:
> >> On 25/07/2022 08:45 CEST Jan Beulich <jbeulich@suse.com> wrote:
> >> while commit 3f30a274913b ("libiberty: Update D symbol demangling
> >> for latest ABI spec") mentions in its description that tuple encoding
> >> has changed, there's no real adjustment to dlang_parse_tuple() there,
> >> nor are there any new (or replaced) test cases for that. Was this
> >> simply overlooked?
> > 
> > Is there any specific example that fails to demangle, or are you just skimming?
> 
> I'm merely looking at the code alongside the ABI spec.
> 
> > From what I recall, there is a couple places in the dlang_demangle parser that handle ambiguities in a mangled symbol.  The ABI change only added a terminating 'Z', which makes said code that handles ambiguity redundant - but of course kept around so we handle both old and new symbols.
> 
> It's not just the addition of Z at the end but also the dropping of the
> number of elements at the beginning, aiui. It's actually that aspect
> which caught my attention, since the ABI doesn't talk about any number
> there, but the code fetches one.
> 

Went to have a look at docarchives, but it appears to be down (that's on me, I have been meaning to migrate the service to new servers).

Yes, your right, the number was indeed dropped too from the ABI.

https://web.archive.org/web/20170812061158/https://dlang.org/spec/abi.html#TypeTuple

    TypeTuple:
        B Number Parameters

https://dlang.org/spec/abi.html#TypeTuple

    TypeTuple:
        B Parameters Z

However, it gets worse the more I stare at it. Looks like it was not understood what 'Number' meant in the old ABI. I assumed it was the encoded number of tuple elements - same as static arrays - however what I see in the front-end is instead an encoded buffer length.

https://github.com/gcc-mirror/gcc/blob/releases/gcc-10/gcc/d/dmd/dmangle.c#L312-L313

So the loop should instead be more like:
---
  unsigned long len;

  mangled = dlang_number (mangled, &len);
  if (mangled == NULL)
    return NULL;

  string_append (decl, "Tuple!(");

  const char *endp = mangled + len;
  int elements = 0;
  while (mangled != endp)
    {
      if (elements++)
        string_append (decl, ", ");

      mangled = dlang_type (decl, mangled, info);
      if (mangled == NULL || mangled > endp)
	return NULL;
    }

  string_append (decl, ")");
  return mangled;
---

On top of that, TypeTuple is a compile-time-only type - it never leaks to the code generator - so the grammar entry in the ABI is frivolous (although internally, that it gets a mangle at all would save some memory as duplicated types are merged).

Iain.

Re: libiberty D tuple demangling

[Jan Beulich]

On 25.07.2022 17:45, ibuclaw@gdcproject.org wrote:
>> On 25/07/2022 14:13 CEST Jan Beulich <jbeulich@suse.com> wrote:
>>
>>  
>> On 25.07.2022 14:05, ibuclaw@gdcproject.org wrote:
>>>> On 25/07/2022 08:45 CEST Jan Beulich <jbeulich@suse.com> wrote:
>>>> while commit 3f30a274913b ("libiberty: Update D symbol demangling
>>>> for latest ABI spec") mentions in its description that tuple encoding
>>>> has changed, there's no real adjustment to dlang_parse_tuple() there,
>>>> nor are there any new (or replaced) test cases for that. Was this
>>>> simply overlooked?
>>>
>>> Is there any specific example that fails to demangle, or are you just skimming?
>>
>> I'm merely looking at the code alongside the ABI spec.
>>
>>> From what I recall, there is a couple places in the dlang_demangle parser that handle ambiguities in a mangled symbol.  The ABI change only added a terminating 'Z', which makes said code that handles ambiguity redundant - but of course kept around so we handle both old and new symbols.
>>
>> It's not just the addition of Z at the end but also the dropping of the
>> number of elements at the beginning, aiui. It's actually that aspect
>> which caught my attention, since the ABI doesn't talk about any number
>> there, but the code fetches one.
>>
> 
> Went to have a look at docarchives, but it appears to be down (that's on me, I have been meaning to migrate the service to new servers).
> 
> Yes, your right, the number was indeed dropped too from the ABI.
> 
> https://web.archive.org/web/20170812061158/https://dlang.org/spec/abi.html#TypeTuple
> 
>     TypeTuple:
>         B Number Parameters
> 
> https://dlang.org/spec/abi.html#TypeTuple
> 
>     TypeTuple:
>         B Parameters Z
> 
> However, it gets worse the more I stare at it. Looks like it was not understood what 'Number' meant in the old ABI. I assumed it was the encoded number of tuple elements - same as static arrays - however what I see in the front-end is instead an encoded buffer length.
> 
> https://github.com/gcc-mirror/gcc/blob/releases/gcc-10/gcc/d/dmd/dmangle.c#L312-L313
> 
> So the loop should instead be more like:
> ---
>   unsigned long len;
> 
>   mangled = dlang_number (mangled, &len);
>   if (mangled == NULL)
>     return NULL;
> 
>   string_append (decl, "Tuple!(");
> 
>   const char *endp = mangled + len;
>   int elements = 0;
>   while (mangled != endp)
>     {
>       if (elements++)
>         string_append (decl, ", ");
> 
>       mangled = dlang_type (decl, mangled, info);
>       if (mangled == NULL || mangled > endp)
> 	return NULL;
>     }
> 
>   string_append (decl, ")");
>   return mangled;
> ---

Oh. Then two of the testcases are actually wrong as well:

_D8demangle4testFB2OaaZv
_D8demangle4testFB3aDFZaaZv

I would have assumed they had been taken from observable output of a
compiler, ...

> On top of that, TypeTuple is a compile-time-only type - it never leaks to the code generator - so the grammar entry in the ABI is frivolous (although internally, that it gets a mangle at all would save some memory as duplicated types are merged).

... but one way of reading this would make me infer that can't have
been the case.

Jan