[Lldb-commits] [lldb] 633b002 - [lldb] Fix PR52702 by fixing bool conversion of Mangled

[PoYao Chang via lldb-commits]

Author: PoYao Chang
Date: 2021-12-29T17:17:52+08:00
New Revision: 633b002944b966ddb64c85f4a8c017a858afb4fc

URL: https://github.com/llvm/llvm-project/commit/633b002944b966ddb64c85f4a8c017a858afb4fc
DIFF: https://github.com/llvm/llvm-project/commit/633b002944b966ddb64c85f4a8c017a858afb4fc.diff

LOG: [lldb] Fix PR52702 by fixing bool conversion of Mangled

Remove the Mangled::operator! and Mangled::operator void* where the
comments in header and implementation files disagree and replace them
with operator bool.

This fix PR52702 as https://reviews.llvm.org/D106837 used the buggy
Mangled::operator! in Symbol::SynthesizeNameIfNeeded. For example,
consider the symbol "puts" in a hello world C program:

// Inside Symbol::SynthesizeNameIfNeeded
(lldb) p m_mangled
(lldb_private::Mangled) $0 = (m_mangled = None, m_demangled = "puts")
(lldb) p !m_mangled
(bool) $1 = true          # should be false!!
This leads to Symbol::SynthesizeNameIfNeeded overwriting m_demangled
part of Mangled (in this case "puts").

In conclusion, this patch turns
callq  0x401030                  ; symbol stub for: ___lldb_unnamed_symbol36
back into
callq  0x401030                  ; symbol stub for: puts .

Differential Revision: https://reviews.llvm.org/D116217

Added: 
    

Modified: 
    lldb/include/lldb/Core/Mangled.h
    lldb/source/Core/Mangled.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/unittests/Core/MangledTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/Mangled.h b/lldb/include/lldb/Core/Mangled.h
index 6c92591a0881..35705b0319ab 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@ class Mangled {
     return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,25 +84,9 @@ class Mangled {
   /// \endcode
   ///
   /// \return
-  ///     A pointer to this object if either the mangled or unmangled
-  ///     name is set, NULL otherwise.
-  operator void *() const;
-
-  /// Logical NOT operator.
-  ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
-  ///
-  /// \code
-  /// Mangled mangled(...);
-  /// if (!mangled)
-  /// { ...
-  /// \endcode
-  ///
-  /// \return
-  ///     Returns \b true if the object has an empty mangled and
-  ///     unmangled name, \b false otherwise.
-  bool operator!() const;
+  ///     Returns \b true if either the mangled or unmangled name is set,
+  ///     \b false if the object has an empty mangled and unmangled name.
+  explicit operator bool() const;
 
   /// Clear the mangled and demangled values.
   void Clear();

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index c8aacdefefa2..4e10324401dc 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -70,23 +70,13 @@ Mangled::Mangled(llvm::StringRef name) {
     SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast<Mangled *>(this) : nullptr;
-}
-
-// Logical NOT operator. This allows code to check any Mangled objects to see
-// if they are invalid using code such as:
-//
-//  Mangled mangled(...);
-//  if (!file_spec)
-//  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 8c995ef2eb2a..ca701c6f2fcc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@ void SymbolFileDWARF::FindGlobalVariables(
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+                         Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
                                                       context, basename))

diff  --git a/lldb/unittests/Core/MangledTest.cpp b/lldb/unittests/Core/MangledTest.cpp
index 4c1bb0cc45c2..284c2f21aadd 100644
--- a/lldb/unittests/Core/MangledTest.cpp
+++ b/lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,25 @@ TEST(MangledTest, EmptyForInvalidDLangName) {
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+    ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+    Mangled TheMangled(MangledName);
+    EXPECT_EQ(true, bool(TheMangled));
+    EXPECT_EQ(false, !TheMangled);
+  }
+  {
+    ConstString UnmangledName("puts");
+    Mangled TheMangled(UnmangledName);
+    EXPECT_EQ(true, bool(TheMangled));
+    EXPECT_EQ(false, !TheMangled);
+  }
+  {
+    Mangled TheMangled{};
+    EXPECT_EQ(false, bool(TheMangled));
+    EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII<FileSystem, HostInfo, ObjectFileELF, SymbolFileSymtab>