llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/83295.diff 5 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp (+16-7) - (added) lldb/test/API/lang/cpp/gmodules/alignment/Makefile (+4) - (added) lldb/test/API/lang/cpp/gmodules/alignment/TestPchAlignment.py (+60) - (added) lldb/test/API/lang/cpp/gmodules/alignment/main.cpp (+10) - (added) lldb/test/API/lang/cpp/gmodules/alignment/pch.h (+21) ``````````diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 62a30c14912bc9..754191ad83fe8a 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -527,7 +527,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()) { @@ -537,13 +536,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 different + // 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 `````````` </details> https://github.com/llvm/llvm-project/pull/83295 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
