[lldb][ClangASTImporter][NFC] Emit a log message when we break MapImported invariant #112748
Conversation
This file contains 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
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis patch emits a warning into the expression log when we call The underlying issue is discussed in more detail in: #112566. In a non-asserts build, the last few expression log entries would look as follows: Note how we start "completing" rdar://135551810 Full diff: https://github.com/llvm/llvm-project/pull/112748.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 630ad7e20ab7e0..2fbd8e6ca688fc 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -1136,6 +1136,25 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) {
void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
clang::Decl *to, clang::Decl *from) {
+ Log *log = GetLog(LLDBLog::Expressions);
+
+ auto getDeclName = [](Decl const *decl) {
+ std::string name_string;
+ if (auto const *from_named_decl = dyn_cast<clang::NamedDecl>(decl)) {
+ llvm::raw_string_ostream name_stream(name_string);
+ from_named_decl->printName(name_stream);
+ }
+
+ return name_string;
+ };
+
+ if (auto *D = GetAlreadyImportedOrNull(from); D && D != to)
+ LLDB_LOG(log,
+ "[ClangASTImporter] WARNING: overwriting an already imported decl "
+ "'{0:x}' ('{1}') from '{2:x}' with '{3:x}'. Likely due to a name "
+ "conflict when importing '{1}'.",
+ D, getDeclName(from), from, to);
+
// We might have a forward declaration from a shared library that we
// gave external lexical storage so that Clang asks us about the full
// definition when it needs it. In this case the ASTImporter isn't aware
@@ -1145,8 +1164,6 @@ void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
// tell the ASTImporter that 'to' was imported from 'from'.
MapImported(from, to);
- Log *log = GetLog(LLDBLog::Expressions);
-
if (llvm::Error err = ImportDefinition(from)) {
LLDB_LOG_ERROR(log, std::move(err),
"[ClangASTImporter] Error during importing definition: {0}");
@@ -1158,11 +1175,7 @@ void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
if (Log *log_ast = GetLog(LLDBLog::AST)) {
- std::string name_string;
- if (NamedDecl *from_named_decl = dyn_cast<clang::NamedDecl>(from)) {
- llvm::raw_string_ostream name_stream(name_string);
- from_named_decl->printName(name_stream);
- }
+ std::string name_string = getDeclName(from);
LLDB_LOG(log_ast,
"==== [ClangASTImporter][TUDecl: {0:x}] Imported "
"({1}Decl*){2:x}, named {3} (from "
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This patch emits a warning into the expression log when we call
MapImportedon a decl which has already been imported, but with a newtodestination decl. In asserts builds this would lead to triggering this ASTImporter::MapImported assertion. In no-asserts builds we will likely crash, in potentially non-obvious ways. The hope is that the log message will help in diagnosing this type of issue in the field.The underlying issue is discussed in more detail in: #112566.
In a non-asserts build, the last few expression log entries would look as follows:
Note how we start "completing"
Foo. Then emit our newWARNING. Shortly after, we crash, and the log abruptly ends.rdar://135551810