* libiberty D tuple demangling [not found] <f35a2359-e102-7682-65b4-e56e602f9adf@aol.com> @ 2022-07-25 6:45 ` Jan Beulich 2022-07-25 12:05 ` ibuclaw 0 siblings, 1 reply; 5+ messages in thread From: Jan Beulich @ 2022-07-25 6:45 UTC (permalink / raw) To: gcc; +Cc: Ian Lance Taylor, Iain Buclaw 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: libiberty D tuple demangling 2022-07-25 6:45 ` libiberty D tuple demangling Jan Beulich @ 2022-07-25 12:05 ` ibuclaw 2022-07-25 12:13 ` Jan Beulich 0 siblings, 1 reply; 5+ messages in thread From: ibuclaw @ 2022-07-25 12:05 UTC (permalink / raw) To: Jan Beulich, gcc; +Cc: Ian Lance Taylor > 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: libiberty D tuple demangling 2022-07-25 12:05 ` ibuclaw @ 2022-07-25 12:13 ` Jan Beulich 2022-07-25 15:45 ` ibuclaw 0 siblings, 1 reply; 5+ messages in thread From: Jan Beulich @ 2022-07-25 12:13 UTC (permalink / raw) To: ibuclaw; +Cc: Ian Lance Taylor, gcc 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: libiberty D tuple demangling 2022-07-25 12:13 ` Jan Beulich @ 2022-07-25 15:45 ` ibuclaw 2022-07-25 15:57 ` Jan Beulich 0 siblings, 1 reply; 5+ messages in thread From: ibuclaw @ 2022-07-25 15:45 UTC (permalink / raw) To: Jan Beulich; +Cc: Ian Lance Taylor, gcc > 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: libiberty D tuple demangling 2022-07-25 15:45 ` ibuclaw @ 2022-07-25 15:57 ` Jan Beulich 0 siblings, 0 replies; 5+ messages in thread From: Jan Beulich @ 2022-07-25 15:57 UTC (permalink / raw) To: ibuclaw; +Cc: Ian Lance Taylor, gcc 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-25 15:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <f35a2359-e102-7682-65b4-e56e602f9adf@aol.com> 2022-07-25 6:45 ` libiberty D tuple demangling Jan Beulich 2022-07-25 12:05 ` ibuclaw 2022-07-25 12:13 ` Jan Beulich 2022-07-25 15:45 ` ibuclaw 2022-07-25 15:57 ` Jan Beulich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).