[Lldb-commits] [lldb] 07ffb7e - [lldb][ClangASTImporter] Import record layouts from origin if available (#83295)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Feb 29 13:40:06 PST 2024
Author: Michael Buch
Date: 2024-02-29T21:40:02Z
New Revision: 07ffb7e294767b74e43f90e9ab3d713da929b907
URL: https://github.com/llvm/llvm-project/commit/07ffb7e294767b74e43f90e9ab3d713da929b907
DIFF: https://github.com/llvm/llvm-project/commit/07ffb7e294767b74e43f90e9ab3d713da929b907.diff
LOG: [lldb][ClangASTImporter] Import record layouts from origin if available (#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
Added:
lldb/test/API/lang/cpp/gmodules/alignment/Makefile
lldb/test/API/lang/cpp/gmodules/alignment/TestPchAlignment.py
lldb/test/API/lang/cpp/gmodules/alignment/main.cpp
lldb/test/API/lang/cpp/gmodules/alignment/pch.h
Modified:
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 99d210c347a21d..30b50df79da90f 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -760,7 +760,6 @@ bool ClangASTImporter::LayoutRecordType(
&vbase_offsets) {
RecordDeclToLayoutMap::iterator pos =
m_record_decl_to_layout_map.find(record_decl);
- bool success = false;
base_offsets.clear();
vbase_offsets.clear();
if (pos != m_record_decl_to_layout_map.end()) {
@@ -770,13 +769,23 @@ bool ClangASTImporter::LayoutRecordType(
base_offsets.swap(pos->second.base_offsets);
vbase_offsets.swap(pos->second.vbase_offsets);
m_record_decl_to_layout_map.erase(pos);
- success = true;
- } else {
- bit_size = 0;
- alignment = 0;
- field_offsets.clear();
+ return true;
}
- return success;
+
+ // It's possible that we calculated the layout in a
diff erent
+ // ClangASTImporter instance. Try to import such layout if
+ // our decl has an origin.
+ if (auto origin = GetDeclOrigin(record_decl); origin.Valid())
+ if (importRecordLayoutFromOrigin(record_decl, bit_size, alignment,
+ field_offsets, base_offsets,
+ vbase_offsets))
+ return true;
+
+ bit_size = 0;
+ alignment = 0;
+ field_offsets.clear();
+
+ return false;
}
void ClangASTImporter::SetRecordLayout(clang::RecordDecl *decl,
diff --git a/lldb/test/API/lang/cpp/gmodules/alignment/Makefile b/lldb/test/API/lang/cpp/gmodules/alignment/Makefile
new file mode 100644
index 00000000000000..a6c3e8ca84a3e4
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/alignment/Makefile
@@ -0,0 +1,4 @@
+PCH_CXX_SOURCE = pch.h
+CXX_SOURCES = main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/gmodules/alignment/TestPchAlignment.py b/lldb/test/API/lang/cpp/gmodules/alignment/TestPchAlignment.py
new file mode 100644
index 00000000000000..535dd13d0ada48
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/alignment/TestPchAlignment.py
@@ -0,0 +1,60 @@
+"""
+Tests that we correctly track AST layout info
+(specifically alignment) when moving AST nodes
+between ClangASTImporter instances (in this case,
+from pch to executable to expression AST).
+"""
+
+import lldb
+import os
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPchAlignment(TestBase):
+ @add_test_categories(["gmodules"])
+ def test_expr(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "return data", lldb.SBFileSpec("main.cpp")
+ )
+
+ self.expect(
+ "frame variable data",
+ substrs=["row = 1", "col = 2", "row = 3", "col = 4", "stride = 5"],
+ )
+
+ @add_test_categories(["gmodules"])
+ def test_frame_var(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "return data", lldb.SBFileSpec("main.cpp")
+ )
+
+ self.expect_expr(
+ "data",
+ result_type="MatrixData",
+ result_children=[
+ ValueCheck(
+ name="section",
+ children=[
+ ValueCheck(
+ name="origin",
+ children=[
+ ValueCheck(name="row", value="1"),
+ ValueCheck(name="col", value="2"),
+ ],
+ ),
+ ValueCheck(
+ name="size",
+ children=[
+ ValueCheck(name="row", value="3"),
+ ValueCheck(name="col", value="4"),
+ ],
+ ),
+ ],
+ ),
+ ValueCheck(name="stride", value="5"),
+ ],
+ )
diff --git a/lldb/test/API/lang/cpp/gmodules/alignment/main.cpp b/lldb/test/API/lang/cpp/gmodules/alignment/main.cpp
new file mode 100644
index 00000000000000..5481f3fad1ff76
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/alignment/main.cpp
@@ -0,0 +1,10 @@
+int main(int argc, const char *argv[]) {
+ struct MatrixData data = {0};
+ data.section.origin.row = 1;
+ data.section.origin.col = 2;
+ data.section.size.row = 3;
+ data.section.size.col = 4;
+ data.stride = 5;
+
+ return data.section.size.row;
+}
diff --git a/lldb/test/API/lang/cpp/gmodules/alignment/pch.h b/lldb/test/API/lang/cpp/gmodules/alignment/pch.h
new file mode 100644
index 00000000000000..f0be272aa601aa
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/alignment/pch.h
@@ -0,0 +1,21 @@
+#ifndef PCH_H_IN
+#define PCH_H_IN
+
+static const int kAlignment = 64;
+
+struct [[gnu::aligned(kAlignment)]] RowCol {
+ unsigned row;
+ unsigned col;
+};
+
+struct [[gnu::aligned(kAlignment)]] Submatrix {
+ struct RowCol origin;
+ struct RowCol size;
+};
+
+struct [[gnu::aligned(kAlignment)]] MatrixData {
+ struct Submatrix section;
+ unsigned stride;
+};
+
+#endif // _H_IN
More information about the lldb-commits
mailing list