Fix C++ demangling for _BitInt type #143466

lfmeadow · 2025-06-10T02:07:38Z

FE expects _BitInt to be available for substitution; ensure DB is added to Subs in ItaniumDemangle.h
Add a test case to libc++abi
Copy changed files to llvm/include/llvm/

* FE expects _BitInt to be available for substitution; ensure DB<n> is added to Subs in ItaniumDemangle.h * Add a test case to libcxxabi * Copy changed files to llvm/include/llvm/

github-actions · 2025-06-10T02:07:57Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-06-10T02:08:27Z

@llvm/pr-subscribers-libcxxabi

Author: Larry Meadows (lfmeadow)

Changes

* FE expects _BitInt to be available for substitution; ensure DB&lt;n&gt; is added to Subs in ItaniumDemangle.h
* Add a test case to libcxxabi
* Copy changed files to llvm/include/llvm/

Full diff: https://github.com/llvm/llvm-project/pull/143466.diff

4 Files Affected:

(modified) libcxxabi/src/demangle/ItaniumDemangle.h (+3-1)
(modified) libcxxabi/test/DemangleTestCases.inc (+1)
(modified) llvm/include/llvm/Demangle/ItaniumDemangle.h (+3-1)
(modified) llvm/include/llvm/Testing/Demangle/DemangleTestCases.inc (+1)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h
index 4e7f92dd1991a..b15cf03aa1f9c 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -4468,7 +4468,9 @@ Node *AbstractManglingParser<Derived, Alloc>::parseType() {
         return nullptr;
       if (!consumeIf('_'))
         return nullptr;
-      return make<BitIntType>(Size, Signed);
+      // The FE expects this to be available for Substitution
+      Result = make<BitIntType>(Size, Signed);
+      break;
     }
     //                ::= Di   # char32_t
     case 'i':
diff --git a/libcxxabi/test/DemangleTestCases.inc b/libcxxabi/test/DemangleTestCases.inc
index 1e3f7459deaa2..2721d2aa5504e 100644
--- a/libcxxabi/test/DemangleTestCases.inc
+++ b/libcxxabi/test/DemangleTestCases.inc
@@ -6,6 +6,7 @@
 {"_Z1fDU10_", "f(unsigned _BitInt(10))"},
 {"_Z1fIfEvDUstPT__", "void f<float>(unsigned _BitInt(sizeof (float*)))"},
 {"_Z1fIiEvDBstPT__", "void f<int>(_BitInt(sizeof (int*)))"},
+{"_Z6myfuncRDB8_S0_", "myfunc(_BitInt(8)&, _BitInt(8)&)"},
 {"_Z4testI1A1BE1Cv", "C test<A, B>()"},
 {"_Z4testI1A1BET0_T_S3_", "B test<A, B>(A, A)"},
 {"_ZN1SgtEi", "S::operator>(int)"},
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index f4569850b093c..38ac8473e3e97 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -4468,7 +4468,9 @@ Node *AbstractManglingParser<Derived, Alloc>::parseType() {
         return nullptr;
       if (!consumeIf('_'))
         return nullptr;
-      return make<BitIntType>(Size, Signed);
+      // The FE expects this to be available for Substitution
+      Result = make<BitIntType>(Size, Signed);
+      break;
     }
     //                ::= Di   # char32_t
     case 'i':
diff --git a/llvm/include/llvm/Testing/Demangle/DemangleTestCases.inc b/llvm/include/llvm/Testing/Demangle/DemangleTestCases.inc
index 1e3f7459deaa2..2721d2aa5504e 100644
--- a/llvm/include/llvm/Testing/Demangle/DemangleTestCases.inc
+++ b/llvm/include/llvm/Testing/Demangle/DemangleTestCases.inc
@@ -6,6 +6,7 @@
 {"_Z1fDU10_", "f(unsigned _BitInt(10))"},
 {"_Z1fIfEvDUstPT__", "void f<float>(unsigned _BitInt(sizeof (float*)))"},
 {"_Z1fIiEvDBstPT__", "void f<int>(_BitInt(sizeof (int*)))"},
+{"_Z6myfuncRDB8_S0_", "myfunc(_BitInt(8)&, _BitInt(8)&)"},
 {"_Z4testI1A1BE1Cv", "C test<A, B>()"},
 {"_Z4testI1A1BET0_T_S3_", "B test<A, B>(A, A)"},
 {"_ZN1SgtEi", "S::operator>(int)"},

lfmeadow · 2025-06-10T02:14:51Z

This tiny fix resolved a number of problems with demangling names that use _BitInt; in particular that is how some of the 8-bit (and smaller) types are represented in various MI operations.
I would like to request @VitaNuo to review this; I made good use of your #111345 PR.

VitaNuo

Looks reasonable, thank you. But I am not a maintainer for the demangler, so I would recommend to also request a review from someone in the owners list.

ldionne

This makes sense to me with a rewording of the comment, potentially.

For history, @zsrkmyn originally added this support in a23652f.

libcxxabi/src/demangle/ItaniumDemangle.h

zsrkmyn · 2025-06-13T03:41:21Z

Thanks for mentioning me here.

But AFAIK, _BitInt and _FloatN were not candidates for substitution by c++abi. Are there any updates now? I once made a proposal to fix the ABI but wasn't merged.

See also: https://reviews.llvm.org/D140359

rjmccall · 2025-06-13T21:20:33Z

Is Clang treating these as substitution candidates, and for how long?

rjmccall · 2025-06-17T01:27:11Z

@zsrkmyn, I got a notification about a response you made, but I don't see it here anymore. I think you're saying that this is not so much a fix as a change intended to unblock a future clang change. But in either case, we seem to have a serious problem if we're retroactively changing the mangling rules for _BitInt. It may have made sense to do this several years, but if we didn't, we may just have to live with the rules as they are.

zsrkmyn · 2025-06-18T02:25:09Z

@rjmccall Sorry, I posted a comment but just found that I was wrong, so I deleted it. I believe you were questioning about how long clang mangler treated these as substitute candidates, but I was answering about demangler.

I haven't worked with _BitInt for years, so I'm not sure what's the current status of it -- neither mangler nor demangler. I posted comments just for historical reference. @lfmeadow may know these things much better than me now :-D

improve comment

Improve comment

lfmeadow · 2025-06-18T21:31:30Z

I did not look into how long the FE has been making _BitInt substitutable. I will look into it.

lfmeadow · 2025-06-18T21:41:51Z

@zsrkmyn @rjmccall @ldionne It seems one of you needs to approve the workflows before this change can be merged...

ldionne

I assumed that Clang already produced these manglings. I'd like to have clarity on whether we already do produce these manglings, otherwise this change may indeed never be required. @lfmeadow Can you chime in?

lfmeadow · 2025-06-25T01:12:08Z

I assumed that Clang already produced these manglings. I'd like to have clarity on whether we already do produce these manglings, otherwise this change may indeed never be required. @lfmeadow Can you chime in?

Yes, clang does produce the mangling for _BitInt, and it does not demangle properly without this change. It's been there for quite some time, with a broken demangler.

lfmeadow · 2025-06-25T18:50:48Z

@ldionne Are you satisfied with my response, or do you need me to try to figure out when/where clang started producing these manglings?

ldionne

I think this is fine, then.

github-actions · 2025-06-25T19:06:32Z

@lfmeadow Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

rjmccall · 2025-06-25T20:44:24Z

Thanks, yeah, that was what I was asking. If clang is producing manglings that assume that this is substitutable, seems like this should be accepted.

zsrkmyn · 2025-06-26T01:56:45Z

@rjmccall I would suggest updating itanium abi as well :-D

The front-end expects _BitInt to be available for substitution; ensure DB<n> is added to Subs in ItaniumDemangle.h. Also add a test case to libc++abi and sync the files to llvm/include/llvm.

Fix C++ demangling for _BitInt type

13feed5

* FE expects _BitInt to be available for substitution; ensure DB<n> is added to Subs in ItaniumDemangle.h * Add a test case to libcxxabi * Copy changed files to llvm/include/llvm/

lfmeadow requested a review from a team as a code owner June 10, 2025 02:07

llvmbot added the libc++abi libc++abi C++ Runtime Library. Not libc++. label Jun 10, 2025

frederick-vs-ja requested a review from VitaNuo June 10, 2025 03:51

VitaNuo approved these changes Jun 10, 2025

View reviewed changes

ldionne approved these changes Jun 12, 2025

View reviewed changes

libcxxabi/src/demangle/ItaniumDemangle.h Outdated Show resolved Hide resolved

zsrkmyn requested a review from rjmccall June 13, 2025 05:09

lfmeadow added 3 commits June 18, 2025 14:20

Update ItaniumDemangle.h

9f4ac96

improve comment

Update ItaniumDemangle.h

defd09d

Improve comment

Merge branch 'main' into bitint-demangle-fix

280e728

ldionne requested changes Jun 20, 2025

View reviewed changes

ldionne approved these changes Jun 25, 2025

View reviewed changes

ldionne merged commit 20f56d1 into llvm:main Jun 25, 2025
110 of 116 checks passed

lfmeadow deleted the bitint-demangle-fix branch June 25, 2025 19:36

Fix C++ demangling for _BitInt type #143466

Fix C++ demangling for _BitInt type #143466

Uh oh!

Conversation

lfmeadow commented Jun 10, 2025 • edited by ldionne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 10, 2025

Uh oh!

llvmbot commented Jun 10, 2025

Uh oh!

lfmeadow commented Jun 10, 2025

Uh oh!

VitaNuo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zsrkmyn commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rjmccall commented Jun 13, 2025

Uh oh!

rjmccall commented Jun 17, 2025

Uh oh!

zsrkmyn commented Jun 18, 2025

Uh oh!

lfmeadow commented Jun 18, 2025

Uh oh!

lfmeadow commented Jun 18, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

lfmeadow commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lfmeadow commented Jun 25, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 25, 2025

Uh oh!

rjmccall commented Jun 25, 2025

Uh oh!

zsrkmyn commented Jun 26, 2025

Uh oh!

Uh oh!

lfmeadow commented Jun 10, 2025 •

edited by ldionne

Loading

VitaNuo left a comment •

edited

Loading

zsrkmyn commented Jun 13, 2025 •

edited

Loading

lfmeadow commented Jun 25, 2025 •

edited

Loading