All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.