Author: Raphael Isemann Date: 2021-07-22T13:30:48+02:00 New Revision: 67c588c481bb2fb9f72459fbc205c3e1ee8c4b3b
URL: https://github.com/llvm/llvm-project/commit/67c588c481bb2fb9f72459fbc205c3e1ee8c4b3b DIFF: https://github.com/llvm/llvm-project/commit/67c588c481bb2fb9f72459fbc205c3e1ee8c4b3b.diff LOG: [lldb] Generalize empty record size computation to avoid giving empty C++ structs a size of 0 C doesn't allow empty structs but Clang/GCC support them and give them a size of 0. LLDB implements this by checking the tag kind and if it's `DW_TAG_structure_type` then we give it a size of 0 via an empty external RecordLayout. This is done because our internal TypeSystem is always in C++ mode (which means we would give them a size of 1). The current check for when we have this special case is currently too lax as types with `DW_TAG_structure_type` can also occur in C++ with types defined using the `struct` keyword. This means that in a C++ program with `struct Empty{};`, LLDB would return `0` for `sizeof(Empty)` even though the correct size is 1. This patch removes this special case and replaces it with a generic approach that just assigns empty structs the byte_size as specified in DWARF. The GCC/Clang special case is handles as they both emit an explicit `DW_AT_byte_size` of 0. And if another compiler decides to use a different byte size for this case then this should also be handled by the same code as long as that information is provided via `DW_AT_byte_size`. Reviewed By: werat, shafik Differential Revision: https://reviews.llvm.org/D105471 Added: lldb/test/API/lang/c/sizeof/Makefile lldb/test/API/lang/c/sizeof/TestCSizeof.py lldb/test/API/lang/c/sizeof/main.c lldb/test/API/lang/cpp/sizeof/Makefile lldb/test/API/lang/cpp/sizeof/TestCPPSizeof.py lldb/test/API/lang/cpp/sizeof/main.cpp Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 6bbcf46ef34d6..f10fbceaea540 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1685,14 +1685,18 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, die.GetOffset(), attrs.name.GetCString()); } - if (tag == DW_TAG_structure_type) // this only applies in C - { + // If the byte size of the record is specified then overwrite the size + // that would be computed by Clang. This is only needed as LLDB's + // TypeSystemClang is always in C++ mode, but some compilers such as + // GCC and Clang give empty structs a size of 0 in C mode (in contrast to + // the size of 1 for empty structs that would be computed in C++ mode). + if (attrs.byte_size) { clang::RecordDecl *record_decl = TypeSystemClang::GetAsRecordDecl(clang_type); - if (record_decl) { - GetClangASTImporter().SetRecordLayout( - record_decl, ClangASTImporter::LayoutInfo()); + ClangASTImporter::LayoutInfo layout; + layout.bit_size = *attrs.byte_size * 8; + GetClangASTImporter().SetRecordLayout(record_decl, layout); } } } else if (clang_type_was_created) { diff --git a/lldb/test/API/lang/c/sizeof/Makefile b/lldb/test/API/lang/c/sizeof/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/lang/c/sizeof/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/lang/c/sizeof/TestCSizeof.py b/lldb/test/API/lang/c/sizeof/TestCSizeof.py new file mode 100644 index 0000000000000..5bcbc42e3dfcf --- /dev/null +++ b/lldb/test/API/lang/c/sizeof/TestCSizeof.py @@ -0,0 +1,18 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def test(self): + self.build() + self.createTestTarget() + + # Empty structs are not allowed in C, but Clang/GCC allow them and + # give them a size of 0. + self.expect_expr("sizeof(Empty) == sizeof_empty", result_value="true") + self.expect_expr("sizeof(SingleMember) == sizeof_single", result_value="true") + self.expect_expr("sizeof(PaddingMember) == sizeof_padding", result_value="true") diff --git a/lldb/test/API/lang/c/sizeof/main.c b/lldb/test/API/lang/c/sizeof/main.c new file mode 100644 index 0000000000000..08bf906edb4af --- /dev/null +++ b/lldb/test/API/lang/c/sizeof/main.c @@ -0,0 +1,21 @@ +struct Empty {}; +struct SingleMember { + int i; +}; + +struct PaddingMember { + int i; + char c; +}; + +const unsigned sizeof_empty = sizeof(struct Empty); +const unsigned sizeof_single = sizeof(struct SingleMember); +const unsigned sizeof_padding = sizeof(struct PaddingMember); + +int main() { + struct Empty empty; + struct SingleMember single; + struct PaddingMember padding; + // Make sure globals are used. + return sizeof_empty + sizeof_single + sizeof_padding; +} diff --git a/lldb/test/API/lang/cpp/sizeof/Makefile b/lldb/test/API/lang/cpp/sizeof/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/lang/cpp/sizeof/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/sizeof/TestCPPSizeof.py b/lldb/test/API/lang/cpp/sizeof/TestCPPSizeof.py new file mode 100644 index 0000000000000..c6b54de349470 --- /dev/null +++ b/lldb/test/API/lang/cpp/sizeof/TestCPPSizeof.py @@ -0,0 +1,20 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def test(self): + self.build() + self.createTestTarget() + + # Empty structs/classes have size 1 in C++. + self.expect_expr("sizeof(Empty) == sizeof_empty", result_value="true") + self.expect_expr("sizeof(EmptyClass) == sizeof_empty_class", result_value="true") + self.expect_expr("sizeof(SingleMember) == sizeof_single", result_value="true") + self.expect_expr("sizeof(SingleMemberClass) == sizeof_single_class", result_value="true") + self.expect_expr("sizeof(PaddingMember) == sizeof_padding", result_value="true") + self.expect_expr("sizeof(PaddingMemberClass) == sizeof_padding_class", result_value="true") diff --git a/lldb/test/API/lang/cpp/sizeof/main.cpp b/lldb/test/API/lang/cpp/sizeof/main.cpp new file mode 100644 index 0000000000000..6c4da06cbef85 --- /dev/null +++ b/lldb/test/API/lang/cpp/sizeof/main.cpp @@ -0,0 +1,37 @@ +struct Empty {}; +class EmptyClass {}; + +struct SingleMember { + int i; +}; +class SingleMemberClass { + int i; +}; + +struct PaddingMember { + int i; + char c; +}; +class PaddingMemberClass { + int i; + char c; +}; + +const unsigned sizeof_empty = sizeof(Empty); +const unsigned sizeof_empty_class = sizeof(EmptyClass); +const unsigned sizeof_single = sizeof(SingleMember); +const unsigned sizeof_single_class = sizeof(SingleMemberClass); +const unsigned sizeof_padding = sizeof(PaddingMember); +const unsigned sizeof_padding_class = sizeof(PaddingMemberClass); + +int main() { + Empty empty; + EmptyClass empty_class; + SingleMember single; + SingleMemberClass single_class; + PaddingMember padding; + PaddingMemberClass padding_class; + // Make sure globals are used. + return sizeof_empty + sizeof_empty_class + sizeof_single + + sizeof_single_class + sizeof_padding + sizeof_padding_class; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits