Skip to content

Commit a444c28

Browse files
committed
[lldb][ClangASTImporter] Import record layouts from origin if available (llvm#83295)
Layout information for a record gets stored in the `ClangASTImporter` associated with the `DWARFASTParserClang` that originally parsed the record. LLDB sometimes moves clang types from one AST to another (in the reproducer the origin AST was a precompiled-header and the destination was the AST backing the executable). When clang then asks LLDB to `layoutRecordType`, it will do so with the help of the `ClangASTImporter` the type is associated with. If the type's origin is actually in a different LLDB module (and thus a different `DWARFASTParserClang` was used to set its layout info), we won't find the layout info in our local `ClangASTImporter`. In the reproducer this meant we would drop the alignment info of the origin type and misread a variable's contents with `frame var` and `expr`. There is logic in `ClangASTSource::layoutRecordType` to import an origin's layout info. This patch re-uses that infrastructure to import an origin's layout from one `ClangASTImporter` instance to another. rdar://123274144 (cherry picked from commit 07ffb7e)
1 parent 55bdd99 commit a444c28

File tree

5 files changed

+110
-7
lines changed

5 files changed

+110
-7
lines changed

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,6 @@ bool ClangASTImporter::LayoutRecordType(
777777

778778
RecordDeclToLayoutMap::iterator pos =
779779
m_record_decl_to_layout_map.find(record_decl);
780-
bool success = false;
781780
base_offsets.clear();
782781
vbase_offsets.clear();
783782
if (pos != m_record_decl_to_layout_map.end()) {
@@ -786,13 +785,22 @@ bool ClangASTImporter::LayoutRecordType(
786785
field_offsets.swap(pos->second.field_offsets);
787786
base_offsets.swap(pos->second.base_offsets);
788787
vbase_offsets.swap(pos->second.vbase_offsets);
789-
success = true;
790-
} else {
791-
bit_size = 0;
792-
alignment = 0;
793-
field_offsets.clear();
794788
}
795-
return success;
789+
790+
// It's possible that we calculated the layout in a different
791+
// ClangASTImporter instance. Try to import such layout if
792+
// our decl has an origin.
793+
if (auto origin = GetDeclOrigin(record_decl); origin.Valid())
794+
if (importRecordLayoutFromOrigin(record_decl, bit_size, alignment,
795+
field_offsets, base_offsets,
796+
vbase_offsets))
797+
return true;
798+
799+
bit_size = 0;
800+
alignment = 0;
801+
field_offsets.clear();
802+
803+
return false;
796804
}
797805

798806
void ClangASTImporter::SetRecordLayout(clang::RecordDecl *decl,
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
PCH_CXX_SOURCE = pch.h
2+
CXX_SOURCES = main.cpp
3+
4+
include Makefile.rules
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
"""
2+
Tests that we correctly track AST layout info
3+
(specifically alignment) when moving AST nodes
4+
between ClangASTImporter instances (in this case,
5+
from pch to executable to expression AST).
6+
"""
7+
8+
import lldb
9+
import os
10+
from lldbsuite.test.decorators import *
11+
from lldbsuite.test.lldbtest import *
12+
from lldbsuite.test import lldbutil
13+
14+
15+
class TestPchAlignment(TestBase):
16+
@add_test_categories(["gmodules"])
17+
def test_expr(self):
18+
self.build()
19+
lldbutil.run_to_source_breakpoint(
20+
self, "return data", lldb.SBFileSpec("main.cpp")
21+
)
22+
23+
self.expect(
24+
"frame variable data",
25+
substrs=["row = 1", "col = 2", "row = 3", "col = 4", "stride = 5"],
26+
)
27+
28+
@add_test_categories(["gmodules"])
29+
def test_frame_var(self):
30+
self.build()
31+
lldbutil.run_to_source_breakpoint(
32+
self, "return data", lldb.SBFileSpec("main.cpp")
33+
)
34+
35+
self.expect_expr(
36+
"data",
37+
result_type="MatrixData",
38+
result_children=[
39+
ValueCheck(
40+
name="section",
41+
children=[
42+
ValueCheck(
43+
name="origin",
44+
children=[
45+
ValueCheck(name="row", value="1"),
46+
ValueCheck(name="col", value="2"),
47+
],
48+
),
49+
ValueCheck(
50+
name="size",
51+
children=[
52+
ValueCheck(name="row", value="3"),
53+
ValueCheck(name="col", value="4"),
54+
],
55+
),
56+
],
57+
),
58+
ValueCheck(name="stride", value="5"),
59+
],
60+
)
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
int main(int argc, const char *argv[]) {
2+
struct MatrixData data = {0};
3+
data.section.origin.row = 1;
4+
data.section.origin.col = 2;
5+
data.section.size.row = 3;
6+
data.section.size.col = 4;
7+
data.stride = 5;
8+
9+
return data.section.size.row;
10+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#ifndef PCH_H_IN
2+
#define PCH_H_IN
3+
4+
static const int kAlignment = 64;
5+
6+
struct [[gnu::aligned(kAlignment)]] RowCol {
7+
unsigned row;
8+
unsigned col;
9+
};
10+
11+
struct [[gnu::aligned(kAlignment)]] Submatrix {
12+
struct RowCol origin;
13+
struct RowCol size;
14+
};
15+
16+
struct [[gnu::aligned(kAlignment)]] MatrixData {
17+
struct Submatrix section;
18+
unsigned stride;
19+
};
20+
21+
#endif // _H_IN

0 commit comments

Comments
 (0)