linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Bogus error for constant array sizes
@ 2008-05-29 20:01 Pavel Roskin
  2008-05-29 22:10 ` Pavel Roskin
  0 siblings, 1 reply; 2+ messages in thread
From: Pavel Roskin @ 2008-05-29 20:01 UTC (permalink / raw)
  To: linux-sparse

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.

The message comes from file expand.c, function __get_expression_value().
expr->type is checked to be equal EXPR_VALUE (1), but it's EXPR_PREOP
(9).

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Bogus error for constant array sizes
  2008-05-29 20:01 Bogus error for constant array sizes Pavel Roskin
@ 2008-05-29 22:10 ` Pavel Roskin
  0 siblings, 0 replies; 2+ messages in thread
From: Pavel Roskin @ 2008-05-29 22:10 UTC (permalink / raw)
  To: linux-sparse

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2008-05-29 22:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-29 20:01 Bogus error for constant array sizes Pavel Roskin
2008-05-29 22:10 ` Pavel Roskin

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).