Skip to content

Commit aa32060

Browse files
authored
[lldb][ClangASTImporter][NFC] Emit a log message when we break MapImported invariant (llvm#112748)
This patch emits a warning into the expression log when we call `MapImported` on a decl which has already been imported, but with a new `to` destination decl. In asserts builds this would lead to triggering this [ASTImporter::MapImported assertion](https://github.com/llvm/llvm-project/blob/6d7712a70c163d2ae9e1dc928db31fcb45d9e404/clang/lib/AST/ASTImporter.cpp#L10493-L10494). 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: llvm#112566. In a non-asserts build, the last few expression log entries would look as follows: ``` CompleteTagDecl on (ASTContext*)scratch ASTContext Completing (TagDecl*)0x00000001132d31d0 named Foo CTD Before: CXXRecordDecl 0x1132d31d0 <<invalid sloc>> <invalid sloc> <undeserialized declarations> struct Foo [ClangASTImporter] WARNING: overwriting an already imported decl '0x000000014378fd80' ('Foo') from '0x0000000143790c00' with 0x00000001132d31d0. Likely due to a name conflict when importing 'Foo'. [ClangASTImporter] Imported (FieldDecl*)0x0000000143790220, named service (from (Decl*)0x0000000143791270), metadata 271 [ClangASTImporter] Decl has no origin information in (ASTContext*)0x00000001132c8c00 FindExternalLexicalDecls on (ASTContext*)0x0000000143c1f600 'scratch ASTContext' in 'Foo' (CXXRecordDecl*)0x000000014378FD80 FELD Original decl (ASTContext*)0x00000001132c8c00 (Decl*)0x0000000143790c00: CXXRecordDecl 0x143790c00 <<invalid sloc>> <invalid sloc> struct Foo definition |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal | |-DefaultConstructor exists trivial needs_implicit | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param | |-MoveConstructor exists simple trivial needs_implicit | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param | |-MoveAssignment exists simple trivial needs_implicit | `-Destructor simple irrelevant trivial needs_implicit |-FieldDecl 0x143791270 <<invalid sloc>> <invalid sloc> service 'Service *' `-FieldDecl 0x1437912c8 <<invalid sloc>> <invalid sloc> mach_endpoint 'int' FELD Adding [to CXXRecordDecl Foo] lexical FieldDecl FieldDecl 0x143791270 <<invalid sloc>> <invalid sloc> service 'Service *' FELD Adding [to CXXRecordDecl Foo] lexical FieldDecl FieldDecl 0x1437912c8 <<invalid sloc>> <invalid sloc> mach_endpoint 'int' [ClangASTImporter] Imported (FieldDecl*)0x0000000143790278, named mach_endpoint (from (Decl*)0x00000001437912c8), metadata 280 [ClangASTImporter] Decl has no origin information in (ASTContext*)0x00000001132c8c00 ``` Note how we start "completing" `Foo`. Then emit our new `WARNING`. Shortly after, we crash, and the log abruptly ends. rdar://135551810
1 parent faed85b commit aa32060

File tree

1 file changed

+25
-9
lines changed

1 file changed

+25
-9
lines changed

lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,6 +1136,29 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) {
11361136

11371137
void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
11381138
clang::Decl *to, clang::Decl *from) {
1139+
Log *log = GetLog(LLDBLog::Expressions);
1140+
1141+
auto getDeclName = [](Decl const *decl) {
1142+
std::string name_string;
1143+
if (auto const *from_named_decl = dyn_cast<clang::NamedDecl>(decl)) {
1144+
llvm::raw_string_ostream name_stream(name_string);
1145+
from_named_decl->printName(name_stream);
1146+
}
1147+
1148+
return name_string;
1149+
};
1150+
1151+
if (log) {
1152+
if (auto *D = GetAlreadyImportedOrNull(from); D && D != to) {
1153+
LLDB_LOG(
1154+
log,
1155+
"[ClangASTImporter] ERROR: overwriting an already imported decl "
1156+
"'{0:x}' ('{1}') from '{2:x}' with '{3:x}'. Likely due to a name "
1157+
"conflict when importing '{1}'.",
1158+
D, getDeclName(from), from, to);
1159+
}
1160+
}
1161+
11391162
// We might have a forward declaration from a shared library that we
11401163
// gave external lexical storage so that Clang asks us about the full
11411164
// definition when it needs it. In this case the ASTImporter isn't aware
@@ -1145,8 +1168,6 @@ void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
11451168
// tell the ASTImporter that 'to' was imported from 'from'.
11461169
MapImported(from, to);
11471170

1148-
Log *log = GetLog(LLDBLog::Expressions);
1149-
11501171
if (llvm::Error err = ImportDefinition(from)) {
11511172
LLDB_LOG_ERROR(log, std::move(err),
11521173
"[ClangASTImporter] Error during importing definition: {0}");
@@ -1158,18 +1179,13 @@ void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
11581179
to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
11591180

11601181
if (Log *log_ast = GetLog(LLDBLog::AST)) {
1161-
std::string name_string;
1162-
if (NamedDecl *from_named_decl = dyn_cast<clang::NamedDecl>(from)) {
1163-
llvm::raw_string_ostream name_stream(name_string);
1164-
from_named_decl->printName(name_stream);
1165-
}
11661182
LLDB_LOG(log_ast,
11671183
"==== [ClangASTImporter][TUDecl: {0:x}] Imported "
11681184
"({1}Decl*){2:x}, named {3} (from "
11691185
"(Decl*){4:x})",
11701186
static_cast<void *>(to->getTranslationUnitDecl()),
1171-
from->getDeclKindName(), static_cast<void *>(to), name_string,
1172-
static_cast<void *>(from));
1187+
from->getDeclKindName(), static_cast<void *>(to),
1188+
getDeclName(from), static_cast<void *>(from));
11731189

11741190
// Log the AST of the TU.
11751191
std::string ast_string;

0 commit comments

Comments
 (0)