[ItaniumDemangle] Add Named flag to "pm" operator

[Michael137]

Compilers can generate mangled names such as _ZN1CpmEi which we currently fail to demangle. The OperatorInfo table only marked the pt operator as being "named", which prevented the others from demangling properly. Removing this logic for the other kinds of member operators isn't causing any tests to fail.

[llvmbot]

@llvm/pr-subscribers-libcxxabi

Author: Michael Buch (Michael137)

Changes

Compilers can generate mangled names such as _ZN1CpmEi which we currently fail to demangle. The OperatorInfo table only marked the pt operator as being "named", which prevented the others from demangling properly. Removing this logic for the other kinds of member operators isn't causing any tests to fail.

---

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

3 Files Affected:

• (modified) libcxxabi/src/demangle/ItaniumDemangle.h (+4-7)
• (modified) libcxxabi/test/test_demangle.pass.cpp (+4)
• (modified) llvm/include/llvm/Demangle/ItaniumDemangle.h (+4-7)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h
index 69932e63669bf..67cc4560f061d 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -3389,9 +3389,9 @@ const typename AbstractManglingParser<
     {"de", OperatorInfo::Prefix, false, Node::Prec::Unary, "operator*"},
     {"dl", OperatorInfo::Del, /*Ary*/ false, Node::Prec::Unary,
      "operator delete"},
-    {"ds", OperatorInfo::Member, /*Named*/ false, Node::Prec::PtrMem,
+    {"ds", OperatorInfo::Member, false, Node::Prec::PtrMem,
      "operator.*"},
-    {"dt", OperatorInfo::Member, /*Named*/ false, Node::Prec::Postfix,
+    {"dt", OperatorInfo::Member, false, Node::Prec::Postfix,
      "operator."},
     {"dv", OperatorInfo::Binary, false, Node::Prec::Assign, "operator/"},
     {"eO", OperatorInfo::Binary, false, Node::Prec::Assign, "operator^="},
@@ -3421,11 +3421,11 @@ const typename AbstractManglingParser<
     {"or", OperatorInfo::Binary, false, Node::Prec::Ior, "operator|"},
     {"pL", OperatorInfo::Binary, false, Node::Prec::Assign, "operator+="},
     {"pl", OperatorInfo::Binary, false, Node::Prec::Additive, "operator+"},
-    {"pm", OperatorInfo::Member, /*Named*/ false, Node::Prec::PtrMem,
+    {"pm", OperatorInfo::Member, false, Node::Prec::PtrMem,
      "operator->*"},
     {"pp", OperatorInfo::Postfix, false, Node::Prec::Postfix, "operator++"},
     {"ps", OperatorInfo::Prefix, false, Node::Prec::Unary, "operator+"},
-    {"pt", OperatorInfo::Member, /*Named*/ true, Node::Prec::Postfix,
+    {"pt", OperatorInfo::Member, false, Node::Prec::Postfix,
      "operator->"},
     {"qu", OperatorInfo::Conditional, false, Node::Prec::Conditional,
      "operator?"},
@@ -3499,9 +3499,6 @@ AbstractManglingParser<Derived, Alloc>::parseOperatorName(NameState *State) {
     if (Op->getKind() >= OperatorInfo::Unnameable)
       /* Not a nameable operator.  */
       return nullptr;
-    if (Op->getKind() == OperatorInfo::Member && !Op->getFlag())
-      /* Not a nameable MemberExpr */
-      return nullptr;
 
     return make<NameType>(Op->getName());
   }
diff --git a/libcxxabi/test/test_demangle.pass.cpp b/libcxxabi/test/test_demangle.pass.cpp
index 53da1bf6765e7..22b39fb06458d 100644
--- a/libcxxabi/test/test_demangle.pass.cpp
+++ b/libcxxabi/test/test_demangle.pass.cpp
@@ -30246,6 +30246,10 @@ const char* cases[][2] = {
 
     {"_Z3fooPU9__ptrauthILj3ELb1ELj234EEPi", "foo(int* __ptrauth<3u, true, 234u>*)"},
     {"_Z3fooIPU9__ptrauthILj1ELb0ELj64EEPiEvT_", "void foo<int* __ptrauth<1u, false, 64u>*>(int* __ptrauth<1u, false, 64u>*)"},
+
+    {"_ZN1CpmEi", "C::operator->*(int)"},
+    {"_ZN1CdtEi", "C::operator.(int)"},
+    {"_ZN1CdsEi", "C::operator.*(int)"},
     // clang-format on
 };
 
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index b4a4c72021fd1..f75e309b06e0a 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -3389,9 +3389,9 @@ const typename AbstractManglingParser<
     {"de", OperatorInfo::Prefix, false, Node::Prec::Unary, "operator*"},
     {"dl", OperatorInfo::Del, /*Ary*/ false, Node::Prec::Unary,
      "operator delete"},
-    {"ds", OperatorInfo::Member, /*Named*/ false, Node::Prec::PtrMem,
+    {"ds", OperatorInfo::Member, false, Node::Prec::PtrMem,
      "operator.*"},
-    {"dt", OperatorInfo::Member, /*Named*/ false, Node::Prec::Postfix,
+    {"dt", OperatorInfo::Member, false, Node::Prec::Postfix,
      "operator."},
     {"dv", OperatorInfo::Binary, false, Node::Prec::Assign, "operator/"},
     {"eO", OperatorInfo::Binary, false, Node::Prec::Assign, "operator^="},
@@ -3421,11 +3421,11 @@ const typename AbstractManglingParser<
     {"or", OperatorInfo::Binary, false, Node::Prec::Ior, "operator|"},
     {"pL", OperatorInfo::Binary, false, Node::Prec::Assign, "operator+="},
     {"pl", OperatorInfo::Binary, false, Node::Prec::Additive, "operator+"},
-    {"pm", OperatorInfo::Member, /*Named*/ false, Node::Prec::PtrMem,
+    {"pm", OperatorInfo::Member, false, Node::Prec::PtrMem,
      "operator->*"},
     {"pp", OperatorInfo::Postfix, false, Node::Prec::Postfix, "operator++"},
     {"ps", OperatorInfo::Prefix, false, Node::Prec::Unary, "operator+"},
-    {"pt", OperatorInfo::Member, /*Named*/ true, Node::Prec::Postfix,
+    {"pt", OperatorInfo::Member, false, Node::Prec::Postfix,
      "operator->"},
     {"qu", OperatorInfo::Conditional, false, Node::Prec::Conditional,
      "operator?"},
@@ -3499,9 +3499,6 @@ AbstractManglingParser<Derived, Alloc>::parseOperatorName(NameState *State) {
     if (Op->getKind() >= OperatorInfo::Unnameable)
       /* Not a nameable operator.  */
       return nullptr;
-    if (Op->getKind() == OperatorInfo::Member && !Op->getFlag())
-      /* Not a nameable MemberExpr */
-      return nullptr;
 
     return make<NameType>(Op->getName());
   }

[Michael137]

@urnathan I couldn't really find an explanation for what "Not a nameable MemberExpr" are. Any idea if this is still needed? Test-suite runs cleanly without this

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[urnathan]

On vacation. But iirc this logic predates my rewrite. Ill try and remember to look more later

[Michael137]

On vacation. But iirc this logic predates my rewrite. Ill try and remember to look more later

Thanks!

Seems to have been introduced by:

commit 7f89fa32e8e9251b0b780009c340bc0d8f4d2e88                                            
Author: Nathan Sidwell <[email protected]>                                                    
Date:   Thu Jan 27 13:23:16 2022 -0800                                                     
                                                                                           
    [demangler][NFC] Tabularize operator name parsing                                      
                                                                                           
    We need to parse operator names in 3 places -- expressions, names &                    
    fold expressions.  Currently we have 3 separate pieces to do this, and a FIXME.        
                                                                                           
    The operator name and expression parsing are implemented as                            
    handwritten two-character nested switches, the fold expression is a                    
    sequence of string comparisons.                                                        
                                                                                           
    This adds a new OperatorInfo class to encode the operator info                         
    (encoding, kind, name), and has a table that it can binary search.                     
    From that each of the above 3 uses are altered to use the new scheme.                  
                                                                                           
    Existing tests cover parsing operator encodings.                                       
                                                                                           
    Reviewed By: ChuanqiXu                                                                 
                                                                                           
    Differential Revision: https://reviews.llvm.org/D119467                                

But couldn't quite tell from the diff why this condition was necessary.

[Michael137]

Ah what was probably meant by this is that the operators are non-overloadable, and thus "not nameable". If we want to keep that check we should at least mark ->* as "Named", since it can be overloaded and something that Clang will generate. Though FWIW, the GCC demangler does demangle . and .* accesses just fine

[Michael137]

Decided to put condition back and just make pm named. Where "Named" basically means "is it overloadable"

[ldionne]

LGTM. Please give a few days for @urnathan to chime in, but I'd say you can merge this at EOW if there's no further activity.