Re: libiberty D tuple demangling

[Jan Beulich <jbeulich@suse.com>]

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