There have been some ICEs in the past for msp430-elf with the large
memory model (20-bit pointers), caused by SET_{DECL,TYPE}_ALIGN being called
with an argument which resolves to 20 i.e. POINTER_SIZE,
or the default value of TARGET_VTABLE_ENTRY_ALIGN (which is POINTER_SIZE).
The attached patch adds an assertion that SET_{DECL,TYPE}_ALIGN is called with
a value which is a power of 2, or 0, for targets which support a partial int
mode. This should catch issues in the future with non-power of 2
alignments being set, which could propagate into serious problems later in the
compilation process.
If the filtering to only perform this check for targets supporting a partial
int mode is unnecessary, I can remove that so CHECK_POW2_OR_ZEROP always
expands to check_pow2_or_zerop.
Successfully bootstrapped and regtested on x86_64-pc-linux-gnu and
msp430-elf/-mlarge.
Ok for trunk, or does this have to wait for GCC10 Stage 1?
>From b44723988dfb0bb9e8c647dd86aeba762ebdf84d Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <joze...@mittosystems.com>
Date: Tue, 18 Dec 2018 11:14:35 +0000
Subject: [PATCH] Check requested alignment in SET_{DECL,TYPE}_ALIGN is
pow2_or_zerop before aligning on targets with partial int modes
2018-12-30 Jozef Lawrynowicz <joze...@mittosystems.com>
gcc/ChangeLog:
* tree.h (check_pow2_or_zerop): New.
(CHECK_POW2_OR_ZEROP): New.
(SET_TYPE_ALIGN): Call CHECK_POW2_OR_ZEROP before setting alignment.
(SET_DECL_ALIGN): Call CHECK_POW2_OR_ZEROP before setting alignment.
---
gcc/tree.h | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/gcc/tree.h b/gcc/tree.h
index ed37e54..e1188b7 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -74,6 +74,27 @@ as_internal_fn (combined_fn code)
return internal_fn (int (code) - int (END_BUILTINS));
}
+/* Historically there have been attempts to call SET_DECL/TYPE_ALIGN with
+ POINTER_SIZE as the alignment. Alignment is expected to always be a power
+ of 2, so aligning to POINTER_SIZE on targets that use a partial integer
+ mode for pointers will cause problems.
+ So for targets that support a partial integer mode, check the requested
+ alignment is 0 or a power of 2 before aligning. */
+#if defined(HAVE_PQImode) || defined(HAVE_PHImode) \
+ || defined(HAVE_PSImode) || defined(HAVE_PDImode)
+inline HOST_WIDE_INT
+check_pow2_or_zerop (HOST_WIDE_INT x)
+{
+ gcc_assert (pow2_or_zerop (x));
+ return x;
+}
+
+#define CHECK_POW2_OR_ZEROP(X) check_pow2_or_zerop (X)
+
+#else /* !defined(HAVE_P{Q,H,S,D}Imode) */
+#define CHECK_POW2_OR_ZEROP(X) X
+#endif
+
/* Macros for initializing `tree_contains_struct'. */
#define MARK_TS_BASE(C) \
(tree_contains_struct[C][TS_BASE] = true)
@@ -1993,7 +2014,7 @@ extern machine_mode vector_type_mode (const_tree);
/* Specify that TYPE_ALIGN(NODE) is X. */
#define SET_TYPE_ALIGN(NODE, X) \
- (TYPE_CHECK (NODE)->type_common.align = ffs_hwi (X))
+ (TYPE_CHECK (NODE)->type_common.align = ffs_hwi (CHECK_POW2_OR_ZEROP (X)))
/* 1 if the alignment for this type was requested by "aligned" attribute,
0 if it is the default for this type. */
@@ -2444,7 +2465,8 @@ extern machine_mode vector_type_mode (const_tree);
? ((unsigned)1) << ((NODE)->decl_common.align - 1) : 0)
/* Specify that DECL_ALIGN(NODE) is X. */
#define SET_DECL_ALIGN(NODE, X) \
- (DECL_COMMON_CHECK (NODE)->decl_common.align = ffs_hwi (X))
+ (DECL_COMMON_CHECK (NODE)->decl_common.align \
+ = ffs_hwi (CHECK_POW2_OR_ZEROP (X)))
/* The minimum alignment necessary for the datum, in bits, without
warning. */
--
2.7.4
