[lldb] Improve identification of Dlang mangled names #93881
Conversation
|
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesFull diff: https://github.com/llvm/llvm-project/pull/93881.diff 4 Files Affected:
diff --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 8efc4c639cca5..3142c81d12ed9 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -19,6 +19,7 @@
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Demangle/Demangle.h"
#include "llvm/Support/Compiler.h"
@@ -48,8 +49,14 @@ Mangled::ManglingScheme Mangled::GetManglingScheme(llvm::StringRef const name) {
if (name.starts_with("_R"))
return Mangled::eManglingSchemeRustV0;
- if (name.starts_with("_D"))
- return Mangled::eManglingSchemeD;
+ if (name.starts_with("_D")) {
+ // A dlang mangled name begins with `_D`, followed by a numeric length.
+ // See `SymbolName` and `LName` in
+ // https://dlang.org/spec/abi.html#name_mangling
+ llvm::StringRef buf = name.drop_front(2);
+ if (!buf.empty() && llvm::isDigit(buf.front()))
+ return Mangled::eManglingSchemeD;
+ }
if (name.starts_with("_Z"))
return Mangled::eManglingSchemeItanium;
diff --git a/lldb/test/API/lang/c/non-mangled/Makefile b/lldb/test/API/lang/c/non-mangled/Makefile
new file mode 100644
index 0000000000000..695335e068c0c
--- /dev/null
+++ b/lldb/test/API/lang/c/non-mangled/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/c/non-mangled/TestCNonMangled.py b/lldb/test/API/lang/c/non-mangled/TestCNonMangled.py
new file mode 100644
index 0000000000000..32bd778fa6eb6
--- /dev/null
+++ b/lldb/test/API/lang/c/non-mangled/TestCNonMangled.py
@@ -0,0 +1,16 @@
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestCase(TestBase):
+
+ def test_functions_having_dlang_mangling_prefix(self):
+ """
+ Ensure C functions with a '_D' prefix alone are not mistakenly treated
+ as a Dlang mangled name. A proper Dlang mangling will have digits
+ immediately following the '_D' prefix.
+ """
+ self.build()
+ _, _, thread, _ = lldbutil.run_to_name_breakpoint(self, "_Dfunction")
+ symbol = thread.frame[0].symbol
+ self.assertEqual(symbol.GetDisplayName(), "_Dfunction")
diff --git a/lldb/test/API/lang/c/non-mangled/main.c b/lldb/test/API/lang/c/non-mangled/main.c
new file mode 100644
index 0000000000000..ad9d86e5c25a8
--- /dev/null
+++ b/lldb/test/API/lang/c/non-mangled/main.c
@@ -0,0 +1,8 @@
+#include <stdio.h>
+
+void _Dfunction() {}
+
+int main() {
+ _Dfunction();
+ return 0;
+}
|
|
✅ With the latest revision this PR passed the Python code formatter. |
There was a problem hiding this comment.
This a little bit raises the question why we don't do the same checks for _R and _Z.
It would be even better if the various demanglers in llvm had a quick "could this be mine" check so we didn't have to encode this in lldb.
But you don't have to fix everything to be allowed to fix one thing.
I asked myself the same. Turns out Rust doesn't have a number following the
I agree. That can be a next step. |
For |
|
Change itself looks fine to me.
Agreed. For example, the mangled name |
|
Also, something to keep in mind: If Dlang ever decides to change their mangled name scheme in the future, this change may become wrong then. I'm not sure what commitments the D language project has for their ABI stability though. |
|
Thanks Alex, Michael. I didn't even look closely at C++ mangling because I knew I didn't want to bundle any C++ changes with this smaller scoped patch. |
That's why this shouldn't be in lldb in the long run, we should ask the D demangler in llvm (if we don't have a D demangler then we should just be ignoring D mangled symbols). We can't guarantee we'll grok anything newer than the D in your lldb, but that seems reasonable. But that way we wouldn't have to track this in lldb. |
kastiglione
deleted the
lldb-Improve-identification-of-Dlang-mangled-names
branch
|
We started seeing test failures in |
|
@gulfemsavrun apologies, fix is here: #94046 |
Follow up to #93881. Updates missed tests and handles `_Dmain`.
Thanks, this fixed the issue. |
|
The test is still failing on Windows on Arm, I will figure it out. |
|
@DavidSpickett thanks, is there a link to a log? I'm curious. |
|
Fix is #94196. There isn't anything to log really, the function just didn't have a symbol on Windows. |
Reduce false positive identification of C names as Dlang mangled names. This happens when a C function uses the prefix `_D`. The [Dlang ABI](https://dlang.org/spec/abi.html#name_mangling) shows that mangled names have a length immediately following the `_D` prefix. This change checks for a digit after the `_D` prefix, when identifying the mangling scheme of a symbol. This doesn't prevent false positives entirely, but does make it less likely. (cherry picked from commit 5a02a9a)
Follow up to llvm#93881. Updates missed tests and handles `_Dmain`. (cherry picked from commit 68fdc1c)
AIUI, symtabs just aren't a thing on windows. You either have debug info, or you have the exported symbols (aka .dynsym). There's no inbetween state. |
Reduce false positive identification of C names as Dlang mangled names. This happens when a C function uses the prefix
_D.The Dlang ABI shows that mangled names have a length immediately following the
_Dprefix. This change checks for a digit after the_Dprefix, when identifying the mangling scheme of a symbol. This doesn't prevent false positives entirely, but does make it less likely.