-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[lldb][test] Add test for ASTImporter's name conflict resolution #112566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
d836983
2079127
f0adb9e
110b0d6
18d5b5e
ba1a258
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| CXX_SOURCES := main.cpp service.cpp | ||
|
|
||
| DYLIB_CXX_SOURCES := plugin.cpp | ||
| DYLIB_NAME := plugin | ||
|
|
||
| include Makefile.rules |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import lldb | ||
| from lldbsuite.test.decorators import * | ||
| from lldbsuite.test.lldbtest import * | ||
| from lldbsuite.test import lldbutil | ||
|
|
||
|
|
||
| class OdrHandlingWithDylibTestCase(TestBase): | ||
| @skipIf( | ||
| bugnumber="https://github.com/llvm/llvm-project/issues/50375, rdar://135551810" | ||
| ) | ||
| def test(self): | ||
| """ | ||
| Tests that the expression evaluator is able to deal with types | ||
| whose definitions conflict across multiple LLDB modules (in this | ||
| case the definition for 'class Service' in the main executable | ||
| has an additional field compared to the definition found in the | ||
| dylib). This causes the ASTImporter to detect a name conflict | ||
| while importing 'Service'. With LLDB's liberal ODRHandlingType | ||
| the ASTImporter happily creates a conflicting AST node for | ||
| 'Service' in the scratch ASTContext, leading to a crash down | ||
| the line. | ||
| """ | ||
| self.build() | ||
|
|
||
| lldbutil.run_to_source_breakpoint( | ||
| self, "plugin_entry", lldb.SBFileSpec("plugin.cpp") | ||
| ) | ||
|
|
||
| self.expect_expr("*gProxyThis") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| #include "plugin.h" | ||
|
|
||
| #define HIDE_FROM_PLUGIN 1 | ||
| #include "service.h" | ||
|
|
||
| int main() { | ||
| exported(); | ||
| plugin_init(); | ||
| plugin_entry(); | ||
| return 0; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| #include "plugin.h" | ||
| #include "service.h" | ||
|
|
||
| struct Proxy : public Service { | ||
| State *proxyState; | ||
| }; | ||
|
|
||
| Proxy *gProxyThis = 0; | ||
|
|
||
| extern "C" { | ||
| void plugin_init() { gProxyThis = new Proxy; } | ||
|
|
||
| void plugin_entry() {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| #ifndef PLUGIN_H_IN | ||
| #define PLUGIN_H_IN | ||
|
|
||
| extern "C" { | ||
| void plugin_entry(void); | ||
| void plugin_init(void); | ||
| } | ||
|
|
||
| #endif // _H_IN | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| #define HIDE_FROM_PLUGIN 1 | ||
| #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); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #ifndef SERVICE_H_IN | ||
| #define SERVICE_H_IN | ||
|
|
||
| struct ServiceAux; | ||
|
|
||
| struct Service { | ||
| struct State; | ||
| bool start(State *) { return true; } | ||
|
|
||
| #ifdef HIDE_FROM_PLUGIN | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if this is really how the original code looked like. This is a particularly nasty form of ODR violation as it messes with the offsets of every field that comes after it (including of the non-plugin Proxy class which inherits from it). It doesn't really matter, as I still think it lldb should be able to handle this kind of a situation, but I'm surprised that code like this could ever work. Could it be that the original code used a milder form of this, like declaring a field with a different name/type (but the same size&alignment)?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea unfortunately this is what the internal users are doing. I reached out to them to understand how this is not causing issues for them. They compile with non-standards conforming extensions, so that might be part of the story |
||
| int __resv1; | ||
| #endif // !HIDE_FROM_PLUGIN | ||
|
|
||
| Service *__owner; | ||
| ServiceAux *aux; | ||
| }; | ||
|
|
||
| void exported(); | ||
|
|
||
| #endif | ||
Uh oh!
There was an error while loading. Please reload this page.