linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: linux-sparse@vger.kernel.org
Cc: Christopher Li <sparse@chrisli.org>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: [PATCH] fix: __builtin_bswap{16,32,64}() constantness
Date: Tue, 20 Jun 2017 18:42:53 +0200	[thread overview]
Message-ID: <20170620164253.76292-1-luc.vanoostenryck@gmail.com> (raw)

When expanding those builtins, we directly excludes a non-constant
arg (via the expression expansion cost) and the to retrieve the
value we use const_expression_value().

But const_expression_value() is to be used for expressions that
qualify as 'integer constant expression' as per the C standard.
However we want to be able to use this builtin with all integer
expressions having a known value at compile time, even those
not qualified as true 'integer constant expression'.

Fix this by using get_expression_value_silent() instead of
const_expression_value().

Note: this bug was found by testing sparse on the git tree
      on ARM64.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 builtin.c                           | 4 +++-
 validation/builtin-bswap-constant.c | 6 ++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/builtin.c b/builtin.c
index dbb81032b..4f2a106cb 100644
--- a/builtin.c
+++ b/builtin.c
@@ -179,13 +179,15 @@ static struct symbol_op choose_op = {
 /* The argument is constant and valid if the cost is zero */
 static int expand_bswap(struct expression *expr, int cost)
 {
+	struct expression *arg;
 	long long val;
 
 	if (cost)
 		return cost;
 
 	/* the arguments number & type have already been checked */
-	val = const_expression_value(first_expression(expr->args));
+	arg = first_expression(expr->args);
+	val = get_expression_value_silent(arg);
 	switch (expr->ctype->bit_size) {
 	case 16: expr->value = __builtin_bswap16(val); break;
 	case 32: expr->value = __builtin_bswap32(val); break;
diff --git a/validation/builtin-bswap-constant.c b/validation/builtin-bswap-constant.c
index 4560f67e9..18574d4c5 100644
--- a/validation/builtin-bswap-constant.c
+++ b/validation/builtin-bswap-constant.c
@@ -16,6 +16,12 @@ unsigned long long bswap64(void)
 	return __builtin_bswap64(0x123456789abcdef0ULL);
 }
 
+unsigned int half_constant(void);
+unsigned int half_constant(void)
+{
+	int v = 0x12345678;
+	return __builtin_bswap32(v);
+}
 /*
  * check-name: builtin-bswap-constant
  * check-command: test-linearize $file
-- 
2.13.0


             reply	other threads:[~2017-06-20 16:43 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-20 16:42 Luc Van Oostenryck [this message]
2017-06-21  4:50 ` [PATCH] fix: __builtin_bswap{16,32,64}() constantness Christopher Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170620164253.76292-1-luc.vanoostenryck@gmail.com \
    --to=luc.vanoostenryck@gmail.com \
    --cc=linux-sparse@vger.kernel.org \
    --cc=sparse@chrisli.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).