-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d836983
[lldb][test] Add test for ASTImporter's name conflict resolution
Michael137 2079127
fixup! clang-format
Michael137 f0adb9e
fixup! turn test into API test; no need for dlopen/dlsym
Michael137 110b0d6
fixup! skip for now
Michael137 18d5b5e
fixup! python format
Michael137 ba1a258
fixup! fix header guard
Michael137 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 |
29 changes: 29 additions & 0 deletions
29
lldb/test/API/lang/cpp/odr-handling-with-dylib/TestOdrHandlingWithDylib.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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() {} | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 // PLUGIN_H_IN |
15 changes: 15 additions & 0 deletions
15
lldb/test/API/lang/cpp/odr-handling-with-dylib/service.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | ||
| int __resv1; | ||
| #endif // !HIDE_FROM_PLUGIN | ||
|
|
||
| Service *__owner; | ||
| ServiceAux *aux; | ||
| }; | ||
|
|
||
| void exported(); | ||
|
|
||
| #endif | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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)?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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