linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Roskin <proski@gnu.org>
To: linux-sparse@vger.kernel.org
Subject: Re: Bogus error for constant array sizes
Date: Thu, 29 May 2008 18:10:09 -0400	[thread overview]
Message-ID: <1212099009.4265.41.camel@dv> (raw)
In-Reply-To: <1212091282.4265.10.camel@dv>

On Thu, 2008-05-29 at 16:01 -0400, Pavel Roskin wrote:
> Hello!
> 
> I'm running sparse (the current git version) on this file:
> 
> static const int len = 64;
> void foo(void);
> void foo(void)
> {
>         int buf[len];
> }
> 
> sparse test.c 
> test.c:5:10: error: bad constant expression
> 
> But if I remove "const", the error message goes away.

It turns out the initializer with "const" is an "implied cast"!

value->type after constant_symbol_value() call in expand_dereference()
is EXPR_VALUE if const is not used and EXPR_IMPLIED_CAST is const is
used.  It is EXPR_CAST for this input:

static int len = (int)64;
void foo(void);
void foo(void)
{
        int buf[len];
} 

That file would trigger the warning too.  I believe "value" should be
expanded, at least in some cases.  What matters is whether the value
evaluates to a constant at the compile time.

Even if we expand the value, problematic casts are still reported:

static int len = 0x100000000LL;
void foo(void);
void foo(void)
{
        int buf[len];
}

warning: cast truncates bits from constant value (100000000 becomes 0)

This patch fixes the bogus errors.  The testsuite passes.  I ran sparse
on MadWifi where it was detecting those errors, and now it's showing
usable warnings instead.

Please review the patch.  Sparse is still almost voodoo science to me.
Once I understand more, I'll write the patch description :-)

diff --git a/expand.c b/expand.c
index 032f0c5..5aa0a1d 100644
--- a/expand.c
+++ b/expand.c
@@ -565,6 +565,7 @@ static struct expression *constant_symbol_value(struct symbol *sym, int offset)
 	value = sym->initializer;
 	if (!value)
 		return NULL;
+	expand_expression(value);
 	if (value->type == EXPR_INITIALIZER) {
 		struct expression *entry;
 		FOR_EACH_PTR(value->expr_list, entry) {


Another question is why assigning a numeric constant to a constant
variable is an "implied cast".  It should be possible to use numeric
constants (or anything that can be calculated at the compile time) to
initialize both constant and non-constant variables without any implied
casts as long as the value fits the destination type.  Or perhaps the
assignments to non-constant variables should be implied casts.

-- 
Regards,
Pavel Roskin
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

      reply	other threads:[~2008-05-29 22:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-29 20:01 Bogus error for constant array sizes Pavel Roskin
2008-05-29 22:10 ` Pavel Roskin [this message]

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=1212099009.4265.41.camel@dv \
    --to=proski@gnu.org \
    --cc=linux-sparse@vger.kernel.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).