[Lldb-commits] [lldb] 67c588c - [lldb] Generalize empty record size computation to avoid giving empty C++ structs a size of 0
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 22 04:31:08 PDT 2021
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;
+}
More information about the lldb-commits
mailing list