Skip to content

[lldb][ClangASTImporter][NFC] Emit a log message when we break MapImported invariant #9454

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

Conversation

Michael137
Copy link

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
. 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
(cherry picked from commit aa32060)

…orted 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
(cherry picked from commit aa32060)
@Michael137
Copy link
Author

@swift-ci test

@Michael137
Copy link
Author

@swift-ci test Linux

@Michael137 Michael137 merged commit 9c753b7 into stable/20240723 Oct 22, 2024
3 checks passed
@Michael137 Michael137 deleted the lldb/warn-on-mapimported-conflict-to-20240723 branch October 22, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants