diff mbox series

c++: Don't assume full alignment for possible base subobjects [PR103303]

Message ID 88E4F6FF-AD0B-489D-9A28-666D8DA8FCEC@thalesgroup.com
State New
Headers
Series c++: Don't assume full alignment for possible base subobjects [PR103303] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed

Commit Message

POLISHCHUK Illia Sept. 30, 2026, 3:37 p.m. UTC 
  c++: Don't assume full alignment for possible base subobjects [PR103303]

When a class has a virtual base that needs more alignment than the rest
of the class, its alignment as a base (the Itanium ABI "nvalign",
CLASSTYPE_ALIGN) is smaller than TYPE_ALIGN.  A base subobject of such a
class, e.g. a virtual base of a derived class, may therefore be placed at
an address that is only aligned to nvalign.  But accesses through 'this'
or any other pointer to the class were given the full TYPE_ALIGN, so e.g.
the SLP vectorizer used movaps to store adjacent members in the base
constructor, which faults at runtime.

Fix this by giving an INDIRECT_REF of such a class type a variant of the
type with the base alignment, unless the class is final or the reference
is obviously to a complete object.  This is done at gimplification time
since fixup_type_variants resets the alignment of type variants whenever
a member function is lazily declared.

        PR c++/103303

gcc/cp/ChangeLog:

        * cp-gimplify.cc (cp_gimplify_expr) <case INDIRECT_REF>: Use the
        alignment of the class as a base for references to a class that
        might be a base subobject.

gcc/testsuite/ChangeLog:

        * g++.dg/torture/pr103303.C: New test.
        * g++.target/i386/pr103303.C: New test.

Signed-off-by: Illia Polishchuk <illia.polishchuk@thalesgroup.com>

Bootstrapped and regtested on x86_64-pc-linux-gnu, no new regressions.
OK for trunk?
From c1dbba6d9a4f3fc3e02aa0c59ab598ba0356859c Mon Sep 17 00:00:00 2001
From: Illia Polishchuk <illia.polishchuk@thalesgroup.com>
Date: Tue, 29 Sep 2026 20:15:31 -0700
Subject: [PATCH] c++: Don't assume full alignment for possible base subobjects
 [PR103303]

When a class has a virtual base that needs more alignment than the rest
of the class, its alignment as a base (the Itanium ABI "nvalign",
CLASSTYPE_ALIGN) is smaller than TYPE_ALIGN.  A base subobject of such a
class, e.g. a virtual base of a derived class, may therefore be placed at
an address that is only aligned to nvalign.  But accesses through 'this'
or any other pointer to the class were given the full TYPE_ALIGN, so e.g.
the SLP vectorizer used movaps to store adjacent members in the base
constructor, which faults at runtime.

Fix this by giving an INDIRECT_REF of such a class type a variant of the
type with the base alignment, unless the class is final or the reference
is obviously to a complete object.  This is done at gimplification time
since fixup_type_variants resets the alignment of type variants whenever
a member function is lazily declared.

	PR c++/103303

gcc/cp/ChangeLog:

	* cp-gimplify.cc (cp_gimplify_expr) <case INDIRECT_REF>: Use the
	alignment of the class as a base for references to a class that
	might be a base subobject.

gcc/testsuite/ChangeLog:

	* g++.dg/torture/pr103303.C: New test.
	* g++.target/i386/pr103303.C: New test.

Signed-off-by: Illia Polishchuk <illia.polishchuk@thalesgroup.com>
---
Bootstrapped and regtested on x86_64-pc-linux-gnu, no new regressions.
OK for trunk?

 gcc/cp/cp-gimplify.cc                    | 30 ++++++++++++++++++++
 gcc/testsuite/g++.dg/torture/pr103303.C  | 36 ++++++++++++++++++++++++
 gcc/testsuite/g++.target/i386/pr103303.C | 31 ++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr103303.C
 create mode 100644 gcc/testsuite/g++.target/i386/pr103303.C

Comments

Jason Merrill Oct. 15, 2026, 1:19 p.m. UTC | #1 
On 9/30/26 11:37 AM, POLISHCHUK Illia wrote:
> c++: Don't assume full alignment for possible base subobjects [PR103303]

Thanks!

> When a class has a virtual base that needs more alignment than the rest
> of the class, its alignment as a base (the Itanium ABI "nvalign",
> CLASSTYPE_ALIGN) is smaller than TYPE_ALIGN.  A base subobject of such a
> class, e.g. a virtual base of a derived class, may therefore be placed at
> an address that is only aligned to nvalign.  But accesses through 'this'
> or any other pointer to the class were given the full TYPE_ALIGN, so e.g.
> the SLP vectorizer used movaps to store adjacent members in the base
> constructor, which faults at runtime.
> 
> Fix this by giving an INDIRECT_REF of such a class type a variant of the
> type with the base alignment, unless the class is final or the reference
> is obviously to a complete object.

Makes sense, though let's try to use resolves_to_fixed_type_p rather 
than duplicating the checks for final/*&x.

> This is done at gimplification time
> since fixup_type_variants resets the alignment of type variants whenever
> a member function is lazily declared.

Let's fix that rather than work around it.  A fixup_type_variants from 
lazily_declare_fn is only trying to update TYPE_FIELDS; it seems like 
only the calls from finish_struct_bits and trees_in::read_class_def 
should mess with alignment.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 0fc44592c..b91437b77 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1067,6 +1067,36 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
       ret = GS_UNHANDLED;
       break;
 
+    case INDIRECT_REF:
+      /* A pointer to a non-final class might point to a base subobject,
+	 which is only guaranteed the alignment of the class as a base (the
+	 ABI "nvalign").  That is less than TYPE_ALIGN if a virtual base
+	 needs more alignment than the rest of the class, so don't let the
+	 middle end assume more (PR c++/103303).  This is done here rather
+	 than in cp_genericize because fixup_type_variants would reset the
+	 alignment of the variant type while the class is still open to
+	 lazily declared members.  */
+      {
+	tree type = TREE_TYPE (*expr_p);
+	tree op = TREE_OPERAND (*expr_p, 0);
+	STRIP_NOPS (op);
+	if (CLASS_TYPE_P (type)
+	    /* *&x refers to the complete object x.  */
+	    && !(TREE_CODE (op) == ADDR_EXPR
+		 && same_type_ignoring_top_level_qualifiers_p
+		      (type, TREE_TYPE (TREE_OPERAND (op, 0))))
+	    && COMPLETE_TYPE_P (type)
+	    && !CLASSTYPE_FINAL (type)
+	    && CLASSTYPE_AS_BASE (type)
+	    && CLASSTYPE_ALIGN (type) < TYPE_ALIGN (type)
+	    /* Respect an alignment the user specified on a variant.  */
+	    && TYPE_ALIGN (type) == TYPE_ALIGN (TYPE_MAIN_VARIANT (type)))
+	  TREE_TYPE (*expr_p) = build_aligned_type (type,
+						    CLASSTYPE_ALIGN (type));
+      }
+      ret = GS_UNHANDLED;
+      break;
+
     case PTRMEM_CST:
       *expr_p = cplus_expand_constant (*expr_p);
       if (TREE_CODE (*expr_p) == PTRMEM_CST)
diff --git a/gcc/testsuite/g++.dg/torture/pr103303.C b/gcc/testsuite/g++.dg/torture/pr103303.C
new file mode 100644
index 000000000..d22cd5de2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr103303.C
@@ -0,0 +1,36 @@ 
+// PR c++/103303
+// { dg-do run }
+// { dg-additional-options "-ftree-slp-vectorize" }
+
+// A class whose virtual base needs more alignment than the rest of the class
+// can be a base subobject at an address that is only aligned to its
+// non-virtual alignment; accesses through 'this' must not assume more.
+
+struct alignas(16) A16 { virtual ~A16 () {} long l[2]; };
+struct B8 { virtual ~B8 () {} };
+
+struct S : virtual B8, virtual A16
+{
+  __attribute__((noipa)) S () : v1 (0), v2 (0) {}
+  __attribute__((noipa)) void set (long x) { v1 = x; v2 = x; }
+  long dummy;
+  long v1;
+  long v2;
+};
+
+struct P { virtual ~P () {} };
+struct Pad { long a = 0, b = 0; };
+
+struct D : P, Pad, virtual S { virtual ~D () {} };
+
+static_assert (alignof (S) == 16, "");
+
+int
+main ()
+{
+  D d;
+  S *s = &d;
+  s->set (42);
+  if (s->v1 != 42 || s->v2 != 42)
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.target/i386/pr103303.C b/gcc/testsuite/g++.target/i386/pr103303.C
new file mode 100644
index 000000000..1ce99011e
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr103303.C
@@ -0,0 +1,31 @@ 
+// PR c++/103303
+// { dg-do compile { target c++11 } }
+// { dg-options "-O3 -msse2" }
+
+struct alignas(16) A16 { virtual ~A16 () {} long l[2]; };
+struct B8 { virtual ~B8 () {} };
+
+struct S : virtual B8, virtual A16
+{
+  S ();
+  long dummy;
+  long v1;
+  long v2;
+};
+
+// 'this' is only guaranteed to be 8-byte aligned in the base constructor.
+S::S () : v1 (0), v2 (0) {}
+
+// But a final class can't be a base subobject.
+struct F final : virtual B8, virtual A16
+{
+  F ();
+  long dummy;
+  long v1;
+  long v2;
+};
+
+F::F () : v1 (0), v2 (0) {}
+
+// { dg-final { scan-assembler-times "movups\[^\n\r\]*16\\(%rdi\\)" 2 { target { ! ia32 } } } }
+// { dg-final { scan-assembler-times "movaps\[^\n\r\]*16\\(%rdi\\)" 2 { target { ! ia32 } } } }