Skip to content

[lldb][test] Add tests for alignof on class with overlapping bases #97068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Michael137
Copy link
Member

Follow-up to #96932

Adds XFAILed test where LLDB incorrectly infers the alignment of a derived class whose bases are overlapping due to [[no_unique_address]]. Specifically, the InferAlignment code-path of the ItaniumRecordLayoutBuilder assumes that overlapping base offsets imply a packed structure and thus sets alignment to 1. See discussion in #93809.

Also adds test where LLDB correctly infers an alignment of 1 when a packed base class is overlapping with other bases.

Lastly, there were a couple of alignof inconsistencies which I encapsulated in an XFAIL-ed packed-alignof.cpp.

Follow-up to llvm#96932

Adds XFAILed test where LLDB incorrectly infers the
alignment of a derived class whose bases are overlapping
due to [[no_unique_address]]. Specifically, the `InferAlignment`
code-path of the `ItaniumRecordLayoutBuilder` assumes that overlapping
base offsets imply a packed structure and thus sets alignment
to 1. See discussion in llvm#93809.

Also adds test where LLDB correctly infers an alignment
of `1` when a packed base class is overlapping with other bases.

Lastly, there were a couple of `alignof` inconsistencies which
I encapsulated in an XFAIL-ed `packed-alignof.cpp`.
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Follow-up to #96932

Adds XFAILed test where LLDB incorrectly infers the alignment of a derived class whose bases are overlapping due to [[no_unique_address]]. Specifically, the InferAlignment code-path of the ItaniumRecordLayoutBuilder assumes that overlapping base offsets imply a packed structure and thus sets alignment to 1. See discussion in #93809.

Also adds test where LLDB correctly infers an alignment of 1 when a packed base class is overlapping with other bases.

Lastly, there were a couple of alignof inconsistencies which I encapsulated in an XFAIL-ed packed-alignof.cpp.


Full diff: https://github.com/llvm/llvm-project/pull/97068.diff

3 Files Affected:

  • (added) lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp (+30)
  • (added) lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp (+41)
  • (modified) lldb/test/Shell/SymbolFile/DWARF/packed.cpp (+14)
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
new file mode 100644
index 0000000000000..634d461e6ff19
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
@@ -0,0 +1,30 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN:   -o "expr alignof(OverlappingDerived)" \
+// RUN:   -o "expr sizeof(OverlappingDerived)" \
+// RUN:   -o exit | FileCheck %s
+
+// CHECK:      (lldb) expr alignof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK:      (lldb) expr sizeof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
+
+struct Empty {};
+
+struct OverlappingBase {
+  [[no_unique_address]] Empty e;
+};
+static_assert(sizeof(OverlappingBase) == 1);
+static_assert(alignof(OverlappingBase) == 1);
+
+struct Base {
+  int mem;
+};
+
+struct OverlappingDerived : Base, OverlappingBase {};
+static_assert(alignof(OverlappingDerived) == 4);
+static_assert(sizeof(OverlappingDerived) == 4);
+
+int main() { OverlappingDerived d; }
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp b/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
new file mode 100644
index 0000000000000..a15e88f0b65df
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
@@ -0,0 +1,41 @@
+// XFAIL: *
+//
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN:   -o "expr alignof(base)" \
+// RUN:   -o "expr alignof(packed_base)" \
+// RUN:   -o "expr alignof(derived)" \
+// RUN:   -o "expr sizeof(derived)" \
+// RUN:   -o exit | FileCheck %s
+
+// CHECK:      (lldb) expr alignof(base)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK:      (lldb) expr alignof(packed_base)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK:      (lldb) expr alignof(derived)
+// CHECK-NEXT: ${{.*}} = 2
+// CHECK:      (lldb) expr sizeof(derived)
+// CHECK-NEXT: ${{.*}} = 16
+
+struct __attribute__((packed)) packed {
+  int x;
+  char y;
+  int z;
+} g_packed_struct;
+
+// LLDB incorrectly calculates alignof(base)
+struct foo {};
+struct base : foo { int x; };
+static_assert(alignof(base) == 4);
+
+// LLDB incorrectly calculates alignof(packed_base)
+struct __attribute__((packed)) packed_base { int x; };
+static_assert(alignof(packed_base) == 1);
+
+struct derived : packed, packed_base {
+  short s;
+} g_derived;
+static_assert(alignof(derived) == 2);
+static_assert(sizeof(derived) == 16);
+
+int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
index 640a2e0454c92..135808f46d7ea 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
@@ -4,6 +4,8 @@
 // RUN:   -o "expr sizeof(packed)" \
 // RUN:   -o "expr alignof(packed_and_aligned)" \
 // RUN:   -o "expr sizeof(packed_and_aligned)" \
+// RUN:   -o "expr alignof(derived)" \
+// RUN:   -o "expr sizeof(derived)" \
 // RUN:   -o exit | FileCheck %s
 
 // CHECK:      (lldb) expr alignof(packed)
@@ -16,6 +18,11 @@
 // CHECK:      (lldb) expr sizeof(packed_and_aligned)
 // CHECK-NEXT: ${{.*}} = 16
 
+// CHECK:      (lldb) expr alignof(derived)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK:      (lldb) expr sizeof(derived)
+// CHECK-NEXT: ${{.*}} = 13
+
 struct __attribute__((packed)) packed {
   int x;
   char y;
@@ -32,4 +39,11 @@ struct __attribute__((packed, aligned(16))) packed_and_aligned {
 static_assert(alignof(packed_and_aligned) == 16);
 static_assert(sizeof(packed_and_aligned) == 16);
 
+struct __attribute__((packed)) packed_base { int x; };
+static_assert(alignof(packed_base) == 1);
+
+struct derived : packed, packed_base {} g_derived;
+static_assert(alignof(derived) == 1);
+static_assert(sizeof(derived) == 13);
+
 int main() {}

@Michael137 Michael137 requested a review from dwblaikie June 28, 2024 15:16
// RUN: -o exit | FileCheck %s

// CHECK: (lldb) expr alignof(OverlappingDerived)
// CHECK-NEXT: ${{.*}} = 4
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to put the checks next to the static_asserts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, done

@Michael137
Copy link
Member Author

Only test failure on buildkite is TestConcurrentVFork.py, which is unrelated. Merging

@Michael137 Michael137 merged commit ed95844 into llvm:main Jun 29, 2024
4 of 6 checks passed
@Michael137 Michael137 deleted the lldb/test/no_unique_address-base-align branch June 29, 2024 07:38
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
…lvm#97068)

Follow-up to llvm#96932

Adds XFAILed test where LLDB incorrectly infers the alignment of a
derived class whose bases are overlapping due to [[no_unique_address]].
Specifically, the `InferAlignment` code-path of the
`ItaniumRecordLayoutBuilder` assumes that overlapping base offsets imply
a packed structure and thus sets alignment to 1. See discussion in
llvm#93809.

Also adds test where LLDB correctly infers an alignment of `1` when a
packed base class is overlapping with other bases.

Lastly, there were a couple of `alignof` inconsistencies which I
encapsulated in an XFAIL-ed `packed-alignof.cpp`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants