[Lldb-commits] [lldb] 63b0f8c - [RecordLayout] Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[via lldb-commits]

Author: shafik
Date: 2020-07-08T10:07:15-07:00
New Revision: 63b0f8c788d8c6978feb099fd6db8fe219c4d166

URL: https://github.com/llvm/llvm-project/commit/63b0f8c788d8c6978feb099fd6db8fe219c4d166
DIFF: https://github.com/llvm/llvm-project/commit/63b0f8c788d8c6978feb099fd6db8fe219c4d166.diff

LOG: [RecordLayout] Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

Currently the ItaniumRecordLayoutBuilder when laying out base classes has the virtual
and non-virtual bases mixed up when pulling the base class layouts from the external source.

This came up in an LLDB bug where on arm64 because of differences in how it deals with
tail padding would layout the bases differently without the correct layout from the
external source (LLDB). This would result in some fields being off by 4 bytes.

Differential Revision: https://reviews.llvm.org/D83008

Added: 
    lldb/test/API/lang/cpp/alignas_base_class/Makefile
    lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
    lldb/test/API/lang/cpp/alignas_base_class/main.cpp

Modified: 
    clang/lib/AST/RecordLayoutBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index b44957a35279..d56c7e2ab8c0 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1187,11 +1187,10 @@ ItaniumRecordLayoutBuilder::LayoutBase(const BaseSubobjectInfo *Base) {
   // Query the external layout to see if it provides an offset.
   bool HasExternalLayout = false;
   if (UseExternalLayout) {
-    // FIXME: This appears to be reversed.
     if (Base->IsVirtual)
-      HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset);
-    else
       HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset);
+    else
+      HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset);
   }
 
   // Clang <= 6 incorrectly applied the 'packed' attribute to base classes.

diff  --git a/lldb/test/API/lang/cpp/alignas_base_class/Makefile b/lldb/test/API/lang/cpp/alignas_base_class/Makefile
new file mode 100644
index 000000000000..99998b20bcb0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/alignas_base_class/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
new file mode 100644
index 000000000000..25f37ab7fe5f
--- /dev/null
+++ b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
@@ -0,0 +1,16 @@
+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__)
+
+    @no_debug_info_test
+    def test(self):
+        self.build()
+        self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+        # The offset of f2 should be 8 because of `alignas(8)`.
+        self.expect_expr("(intptr_t)&d3g.f2 - (intptr_t)&d3g", result_value="8")

diff  --git a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
new file mode 100644
index 000000000000..8dfced6c784e
--- /dev/null
+++ b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
@@ -0,0 +1,13 @@
+struct B1 {
+  char f1;
+};
+
+struct alignas(8) B2 {
+  char f2;
+};
+
+struct D : B1, B2 {};
+
+D d3g;
+
+int main() {}