[Lldb-commits] [lldb] [lldb][test] Add test for ASTImporter's name conflict resolution (PR #112566)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 16 08:19:48 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
<details>
<summary>Changes</summary>
This is a reduced test case from a crash we've observed in the past. The assertion that this test triggers is:
```
Assertion failed: ((Pos == ImportedDecls.end() || Pos->second == To) && "Try to import an already imported Decl"), function MapImported, file ASTImporter.cpp, line 10494.
```
In a non-asserts build we crash later on in the ASTImporter. The root cause is, as the assertion above points out, that we erroneously replace an existing `From->To` decl mapping with a `To` decl that isn't complete. Then we try to complete it but it has no definition and we dereference a nullptr.
The reason this happens is basically what's been described in https://reviews.llvm.org/D67803?id=220956#<!-- -->1676588
The dylib contains a definition of `Service` which is different to the one in the main executable. When we start dumping the children of the variable we're printing, we start completing it's members, `ASTImport`ing fields in the process. When the ASTImporter realizes there's been a name conflict (i.e., a structural mismatch on the `Service` type) it would usually report back an error. However, LLDB uses `ODRHandlingType::Liberal`, which means we create a new decl for the ODR'd type instead of re-using the previously mapped decl. Eventually this leads us to crash.
Ideally we'd be using `ODRHandlingType::Conservative` and warn/error, though LLDB relies on this in some cases (particularly for distinguishing template specializations, though maybe there's better a way to deal with those).
We should really warn the user when this happens and not crash. To avoid the crash we'd need to know to not create a decl for the ODR violation, and instead re-use the definition we've previously seen. Though I'm not yet sure that's viable for all of LLDB's use-cases (where ODR violations might legimiately occur in a program, e.g., with opaque definitions, etc.).
---
Full diff: https://github.com/llvm/llvm-project/pull/112566.diff
6 Files Affected:
- (added) lldb/test/Shell/Expr/Inputs/name-conflict-test/main.cpp (+21)
- (added) lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.cpp (+15)
- (added) lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.h (+10)
- (added) lldb/test/Shell/Expr/Inputs/name-conflict-test/service.cpp (+15)
- (added) lldb/test/Shell/Expr/Inputs/name-conflict-test/service.h (+20)
- (added) lldb/test/Shell/Expr/TestODRHandlingWithDylib.test (+17)
``````````diff
diff --git a/lldb/test/Shell/Expr/Inputs/name-conflict-test/main.cpp b/lldb/test/Shell/Expr/Inputs/name-conflict-test/main.cpp
new file mode 100644
index 00000000000000..74d40d04ed4e20
--- /dev/null
+++ b/lldb/test/Shell/Expr/Inputs/name-conflict-test/main.cpp
@@ -0,0 +1,21 @@
+#include "service.h"
+#include <cassert>
+#include <dlfcn.h>
+
+#ifndef PLUGIN_PATH
+#error "Expected PLUGIN_PATH to be defined"
+#endif // !PLUGIN_PATH
+
+int main() {
+ void *handle = dlopen(PLUGIN_PATH, RTLD_NOW);
+ assert(handle != nullptr);
+ void (*plugin_init)(void) = (void (*)(void))dlsym(handle, "plugin_init");
+ assert(plugin_init != nullptr);
+ void (*plugin_entry)(void) = (void (*)(void))dlsym(handle, "plugin_entry");
+ assert(plugin_entry != nullptr);
+
+ exported();
+ plugin_init();
+ plugin_entry();
+ return 0;
+}
diff --git a/lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.cpp b/lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.cpp
new file mode 100644
index 00000000000000..8f8e98ed5c4b01
--- /dev/null
+++ b/lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.cpp
@@ -0,0 +1,15 @@
+#include "service.h"
+#include "plugin.h"
+
+struct Proxy : public Service {
+ State *proxyState;
+};
+
+Proxy *gProxyThis = 0;
+
+extern "C" {
+void plugin_init() { gProxyThis = new Proxy; }
+
+void plugin_entry() {}
+}
+
diff --git a/lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.h b/lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.h
new file mode 100644
index 00000000000000..a04b0974165e71
--- /dev/null
+++ b/lldb/test/Shell/Expr/Inputs/name-conflict-test/plugin.h
@@ -0,0 +1,10 @@
+#ifndef PLUGIN_H_IN
+#define PLUGIN_H_IN
+
+extern "C" {
+void plugin_entry(void);
+void plugin_init(void);
+}
+
+#endif // _H_IN
+
diff --git a/lldb/test/Shell/Expr/Inputs/name-conflict-test/service.cpp b/lldb/test/Shell/Expr/Inputs/name-conflict-test/service.cpp
new file mode 100644
index 00000000000000..5fd008c2aa8b80
--- /dev/null
+++ b/lldb/test/Shell/Expr/Inputs/name-conflict-test/service.cpp
@@ -0,0 +1,15 @@
+#include "service.h"
+
+struct ServiceAux {
+ Service *Owner;
+};
+
+struct Service::State {};
+
+void exported() {
+ // Make sure debug-info for definition of Service is
+ // emitted in this CU.
+ Service service;
+ service.start(0);
+}
+
diff --git a/lldb/test/Shell/Expr/Inputs/name-conflict-test/service.h b/lldb/test/Shell/Expr/Inputs/name-conflict-test/service.h
new file mode 100644
index 00000000000000..53762a6e00357a
--- /dev/null
+++ b/lldb/test/Shell/Expr/Inputs/name-conflict-test/service.h
@@ -0,0 +1,20 @@
+#ifndef SERVICE_H_IN
+#define SERVICE_H_IN
+
+struct ServiceAux;
+
+struct Service {
+ struct State;
+ bool start(State *) { return true; }
+
+#if HIDE_FROM_PLUGIN
+ int __resv1;
+#endif // !HIDE_FROM_PLUGIN
+
+ Service *__owner;
+ ServiceAux *aux;
+};
+
+void exported();
+
+#endif
diff --git a/lldb/test/Shell/Expr/TestODRHandlingWithDylib.test b/lldb/test/Shell/Expr/TestODRHandlingWithDylib.test
new file mode 100644
index 00000000000000..7ee34ff4431d8b
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestODRHandlingWithDylib.test
@@ -0,0 +1,17 @@
+# REQUIRES: system-darwin
+
+# RUN: %clangxx_host %p/Inputs/name-conflict-test/plugin.cpp -shared -gdwarf -O0 -o %t.plugin.dylib
+#
+# RUN: %clangxx_host %p/Inputs/name-conflict-test/main.cpp \
+# RUN: %p/Inputs/name-conflict-test/service.cpp \
+# RUN: -DPLUGIN_PATH=\"%t.plugin.dylib\" \
+# RUN: -DHIDE_FROM_PLUGIN \
+# RUN: -gdwarf -O0 -o %t.a.out
+#
+# RUN: %lldb %t.a.out \
+# RUN: -o "b plugin_entry" \
+# RUN: -o run \
+# RUN: -o "expr *gProxyThis" \
+# RUN: -o exit | FileCheck %s
+
+# CHECK: (lldb) expr *gProxyThis
``````````
</details>
https://github.com/llvm/llvm-project/pull/112566
More information about the lldb-commits
mailing list