expand: 'conservative' must not bypass valid simplifications

During the expansion of shifts, the variable 'conservative' is used
to inhibit any possible diagnostics (for example, because the needed
information is if the expression is a constant or not).

However, this must not inhibit the simplification of valid shift
expressions. Unfortunately, by moving the validation inside
check_shift_count(), this what was done by commit
	0b73dee01 ("big-shift: move the check into check_shift_count()").

Found through a false positive VLA detected in the Linux kernel.
The array size was computed through min() on a shifted constant value
and sparse complained about it.

Fix this by changing the logic of check_shift_count():
1) moving the test of 'conservative' inside check_shift_count()
   and only issuing warnings if set.
2) moving the warning part in a separate function: warn_shift_count()
3) let check_shift_count() return if the shift count is valid
   so that the simplication can be eluded if not.

Fixes: 0b73dee0171a15800d0a4ae6225b602bf8961599
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

3 files changed, 91 insertions, 8 deletions

diff --git a/expand.c b/expand.c
index e8e50b08..95f9fda6 100644
--- a/expand.c
+++ b/expand.c
@@ -158,19 +158,14 @@ Float:
 	expr->type = EXPR_FVALUE;
 }
 
-static void check_shift_count(struct expression *expr, struct expression *right)
+static void warn_shift_count(struct expression *expr, struct symbol *ctype, long long count)
 {
-	struct symbol *ctype = expr->ctype;
-	long long count = get_longlong(right);
-
 	if (count < 0) {
 		if (!Wshift_count_negative)
 			return;
 		warning(expr->pos, "shift count is negative (%lld)", count);
 		return;
 	}
-	if (count < ctype->bit_size)
-		return;
 	if (ctype->type == SYM_NODE)
 		ctype = ctype->ctype.base_type;
 
@@ -179,6 +174,19 @@ static void check_shift_count(struct expression *expr, struct expression *right)
 	warning(expr->pos, "shift too big (%llu) for type %s", count, show_typename(ctype));
 }
 
+/* Return true if constant shift size is valid */
+static bool check_shift_count(struct expression *expr, struct expression *right)
+{
+	struct symbol *ctype = expr->ctype;
+	long long count = get_longlong(right);
+
+	if (count >= 0 && count < ctype->bit_size)
+		return true;
+	if (!conservative)
+		warn_shift_count(expr, ctype, count);
+	return false;
+}
+
 /*
  * CAREFUL! We need to get the size and sign of the
  * result right!
@@ -197,9 +205,8 @@ static int simplify_int_binop(struct expression *expr, struct symbol *ctype)
 		return 0;
 	r = right->value;
 	if (expr->op == SPECIAL_LEFTSHIFT || expr->op == SPECIAL_RIGHTSHIFT) {
-		if (conservative)
+		if (!check_shift_count(expr, right))
 			return 0;
-		check_shift_count(expr, right);
 	}
 	if (left->type != EXPR_VALUE)
 		return 0;
diff --git a/validation/constexpr-shift.c b/validation/constexpr-shift.c
new file mode 100644
index 00000000..df01b74e
--- /dev/null
+++ b/validation/constexpr-shift.c
@@ -0,0 +1,12 @@
+#define __is_constexpr(x) \
+        (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+
+static void test(int x) {
+	static int b[] = {
+		[__builtin_choose_expr(__is_constexpr(1 << 1), 1, x)] = 0,
+	};
+}
+
+/*
+ * check-name: constexpr-shift
+ */
diff --git a/validation/expand/bad-shift.c b/validation/expand/bad-shift.c
new file mode 100644
index 00000000..22c4341f
--- /dev/null
+++ b/validation/expand/bad-shift.c
@@ -0,0 +1,64 @@
+#define MAX	(sizeof(int) * __CHAR_BIT__)
+
+static int lmax(int a)
+{
+	return 1 << MAX;
+}
+
+static int lneg(int a)
+{
+	return 1 << -1;
+}
+
+static int rmax(int a)
+{
+	return 1 >> MAX;
+}
+
+static int rneg(int a)
+{
+	return 1 >> -1;
+}
+
+/*
+ * check-name: bad-shift
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-start
+lmax:
+.L0:
+	<entry-point>
+	shl.32      %r1 <- $1, $32
+	ret.32      %r1
+
+
+lneg:
+.L2:
+	<entry-point>
+	shl.32      %r3 <- $1, $0xffffffff
+	ret.32      %r3
+
+
+rmax:
+.L4:
+	<entry-point>
+	asr.32      %r5 <- $1, $32
+	ret.32      %r5
+
+
+rneg:
+.L6:
+	<entry-point>
+	asr.32      %r7 <- $1, $0xffffffff
+	ret.32      %r7
+
+
+ * check-output-end
+ *
+ * check-error-start
+expand/bad-shift.c:5:18: warning: shift too big (32) for type int
+expand/bad-shift.c:10:18: warning: shift count is negative (-1)
+expand/bad-shift.c:15:18: warning: shift too big (32) for type int
+expand/bad-shift.c:20:18: warning: shift count is negative (-1)
+ * check-error-end
+ */