* [PATCH] fix jump-misses-init warnings
@ 2009-09-04 14:36 Giuseppe Scrivano
2009-09-14 7:37 ` Ludovic Courtès
0 siblings, 1 reply; 4+ messages in thread
From: Giuseppe Scrivano @ 2009-09-04 14:36 UTC (permalink / raw)
To: bug-guile
Hello!
I reported some problems compiling guile with new versions of GCC. The
reason of these problems is that new GCC versions check for declaration
in the current scope that are missed by a goto.
This is the GCC inline code describing the jump-misses-init warning:
/* If there are any decls in label_vars->decls_in_scope, then this
goto has missed the declaration of the decl. This happens for a
case like
int i = 1;
lab:
...
goto lab;
Issue a warning or error. */
It is exactly the pattern present in some files, causing the compiling
failure because warnings are treated as errors:
In file included from eval.c:4148:0:
eval.i.c: In function ‘deval’:
eval.i.c:1104:2: error: jump skips variable initialization
eval.i.c:1043:6: note: label ‘handle_a_macro’ defined here
eval.i.c:1011:8: note: ‘orig_sym’ declared here
My proposal is to move such variables causing this warning to the upper
scope as in the attached patch.
I have checked if my changes introduced regressions ("make check"), but
all tests were completed successfully.
Cheers,
Giuseppe
From 234fc0860ba35a7e5da714586bdf4b9244e8dffc Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscrivano@gnu.org>
Date: Fri, 4 Sep 2009 16:22:07 +0200
Subject: [PATCH] Avoid the jump-misses-init compiler warning
* libguile/conv-uinteger.i.c (SCM_TO_TYPE_PROTO): Move the declaration
of `n' to the upper scope and rename other variables with the same name.
* libguile/eval.i.c (dispatch): Do the same with `orig_sym'.
(SCM_GPROC1): Do the same with `xx'.
(scm_max): Likewise.
(scm_min): Likewise.
(scm_i_divide): Likewise.
* srfi/srfi-1.c (scm_srfi1_filter_map): Do the same with `proc_tramp'.
(scm_srfi1_list_index): Do the same with `pred_tramp'.
---
libguile/conv-uinteger.i.c | 27 ++++++++++---------
libguile/eval.i.c | 3 +-
libguile/numbers.c | 60 +++++++++++++++++++++++--------------------
srfi/srfi-1.c | 20 ++++++++------
4 files changed, 59 insertions(+), 51 deletions(-)
diff --git a/libguile/conv-uinteger.i.c b/libguile/conv-uinteger.i.c
index 52f49f7..b08fdab 100644
--- a/libguile/conv-uinteger.i.c
+++ b/libguile/conv-uinteger.i.c
@@ -26,9 +26,10 @@
TYPE
SCM_TO_TYPE_PROTO (SCM val)
{
+ scm_t_signed_bits n;
if (SCM_I_INUMP (val))
{
- scm_t_signed_bits n = SCM_I_INUM (val);
+ n = SCM_I_INUM (val);
if (n >= 0
&& ((scm_t_uintmax)n) >= TYPE_MIN && ((scm_t_uintmax)n) <= TYPE_MAX)
return n;
@@ -49,17 +50,17 @@ SCM_TO_TYPE_PROTO (SCM val)
{
if (mpz_fits_ulong_p (SCM_I_BIG_MPZ (val)))
{
- unsigned long n = mpz_get_ui (SCM_I_BIG_MPZ (val));
+ unsigned long nn = mpz_get_ui (SCM_I_BIG_MPZ (val));
#if SIZEOF_TYPE != 0 && SIZEOF_TYPE > SCM_SIZEOF_LONG
- return n;
+ return nn;
#else
#if TYPE_MIN == 0
- if (n <= TYPE_MAX)
- return n;
+ if (nn <= TYPE_MAX)
+ return nn;
#else /* TYPE_MIN != 0 */
- if (n >= TYPE_MIN && n <= TYPE_MAX)
- return n;
+ if (nn >= TYPE_MIN && nn <= TYPE_MAX)
+ return nn;
#endif /* TYPE_MIN != 0 */
else
goto out_of_range;
@@ -71,7 +72,7 @@ SCM_TO_TYPE_PROTO (SCM val)
}
else
{
- scm_t_uintmax n;
+ scm_t_uintmax nn;
size_t count;
if (mpz_sgn (SCM_I_BIG_MPZ (val)) < 0)
@@ -81,14 +82,14 @@ SCM_TO_TYPE_PROTO (SCM val)
> CHAR_BIT*sizeof (TYPE))
goto out_of_range;
- mpz_export (&n, &count, 1, sizeof (TYPE), 0, 0, SCM_I_BIG_MPZ (val));
+ mpz_export (&nn, &count, 1, sizeof (TYPE), 0, 0, SCM_I_BIG_MPZ (val));
#if TYPE_MIN == 0
- if (n <= TYPE_MAX)
- return n;
+ if (nn <= TYPE_MAX)
+ return nn;
#else /* TYPE_MIN != 0 */
- if (n >= TYPE_MIN && n <= TYPE_MAX)
- return n;
+ if (nn >= TYPE_MIN && nn <= TYPE_MAX)
+ return nn;
#endif /* TYPE_MIN != 0 */
else
goto out_of_range;
diff --git a/libguile/eval.i.c b/libguile/eval.i.c
index 25abf6c..d0f669d 100644
--- a/libguile/eval.i.c
+++ b/libguile/eval.i.c
@@ -1000,6 +1000,7 @@ dispatch:
}
else
{
+ SCM orig_sym;
if (SCM_VARIABLEP (SCM_CAR (x)))
proc = SCM_VARIABLE_REF (SCM_CAR (x));
else if (SCM_ILOCP (SCM_CAR (x)))
@@ -1008,7 +1009,7 @@ dispatch:
proc = CEVAL (SCM_CAR (x), env);
else if (scm_is_symbol (SCM_CAR (x)))
{
- SCM orig_sym = SCM_CAR (x);
+ orig_sym = SCM_CAR (x);
{
SCM *location = scm_lookupcar1 (x, env, 1);
if (location == NULL)
diff --git a/libguile/numbers.c b/libguile/numbers.c
index 5812576..bd3d2c0 100644
--- a/libguile/numbers.c
+++ b/libguile/numbers.c
@@ -3460,10 +3460,11 @@ SCM_GPROC1 (s_less_p, "<", scm_tc7_rpsubr, scm_less_p, g_less_p);
SCM
scm_less_p (SCM x, SCM y)
{
+ long xx;
again:
if (SCM_I_INUMP (x))
{
- long xx = SCM_I_INUM (x);
+ xx = SCM_I_INUM (x);
if (SCM_I_INUMP (y))
{
long yy = SCM_I_INUM (y);
@@ -3723,6 +3724,7 @@ SCM_GPROC1 (s_max, "max", scm_tc7_asubr, scm_max, g_max);
SCM
scm_max (SCM x, SCM y)
{
+ long xx;
if (SCM_UNBNDP (y))
{
if (SCM_UNBNDP (x))
@@ -3735,7 +3737,7 @@ scm_max (SCM x, SCM y)
if (SCM_I_INUMP (x))
{
- long xx = SCM_I_INUM (x);
+ xx = SCM_I_INUM (x);
if (SCM_I_INUMP (y))
{
long yy = SCM_I_INUM (y);
@@ -3777,12 +3779,12 @@ scm_max (SCM x, SCM y)
}
else if (SCM_REALP (y))
{
- /* if y==NaN then xx>yy is false, so we return the NaN y */
- double xx, yy;
+ /* if y==NaN then xxd>yyd is false, so we return the NaN y */
+ double xxd, yyd;
big_real:
- xx = scm_i_big2dbl (x);
- yy = SCM_REAL_VALUE (y);
- return (xx > yy ? scm_from_double (xx) : y);
+ xxd = scm_i_big2dbl (x);
+ yyd = SCM_REAL_VALUE (y);
+ return (xxd > yyd ? scm_from_double (xxd) : y);
}
else if (SCM_FRACTIONP (y))
{
@@ -3810,14 +3812,14 @@ scm_max (SCM x, SCM y)
if y==NaN then ">" is false and we return NaN
calling isnan is unavoidable, since it's the only way to know
which of x or y causes any compares to be false */
- double xx = SCM_REAL_VALUE (x);
- return (xisnan (xx) || xx > SCM_REAL_VALUE (y)) ? x : y;
+ double xxd = SCM_REAL_VALUE (x);
+ return (xisnan (xxd) || xxd > SCM_REAL_VALUE (y)) ? x : y;
}
else if (SCM_FRACTIONP (y))
{
- double yy = scm_i_fraction2double (y);
- double xx = SCM_REAL_VALUE (x);
- return (xx < yy) ? scm_from_double (yy) : x;
+ double yyd = scm_i_fraction2double (y);
+ double xxd = SCM_REAL_VALUE (x);
+ return (xxd < yyd) ? scm_from_double (yyd) : x;
}
else
SCM_WTA_DISPATCH_2 (g_max, x, y, SCM_ARGn, s_max);
@@ -3834,8 +3836,8 @@ scm_max (SCM x, SCM y)
}
else if (SCM_REALP (y))
{
- double xx = scm_i_fraction2double (x);
- return (xx < SCM_REAL_VALUE (y)) ? y : scm_from_double (xx);
+ double xxd = scm_i_fraction2double (x);
+ return (xxd < SCM_REAL_VALUE (y)) ? y : scm_from_double (xxd);
}
else if (SCM_FRACTIONP (y))
{
@@ -3855,6 +3857,7 @@ SCM_GPROC1 (s_min, "min", scm_tc7_asubr, scm_min, g_min);
SCM
scm_min (SCM x, SCM y)
{
+ long xx;
if (SCM_UNBNDP (y))
{
if (SCM_UNBNDP (x))
@@ -3867,7 +3870,7 @@ scm_min (SCM x, SCM y)
if (SCM_I_INUMP (x))
{
- long xx = SCM_I_INUM (x);
+ xx = SCM_I_INUM (x);
if (SCM_I_INUMP (y))
{
long yy = SCM_I_INUM (y);
@@ -3909,12 +3912,12 @@ scm_min (SCM x, SCM y)
}
else if (SCM_REALP (y))
{
- /* if y==NaN then xx<yy is false, so we return the NaN y */
- double xx, yy;
+ /* if y==NaN then xxd<yyd is false, so we return the NaN y */
+ double xxd, yyd;
big_real:
- xx = scm_i_big2dbl (x);
- yy = SCM_REAL_VALUE (y);
- return (xx < yy ? scm_from_double (xx) : y);
+ xxd = scm_i_big2dbl (x);
+ yyd = SCM_REAL_VALUE (y);
+ return (xxd < yyd ? scm_from_double (xxd) : y);
}
else if (SCM_FRACTIONP (y))
{
@@ -3942,14 +3945,14 @@ scm_min (SCM x, SCM y)
if y==NaN then "<" is false and we return NaN
calling isnan is unavoidable, since it's the only way to know
which of x or y causes any compares to be false */
- double xx = SCM_REAL_VALUE (x);
- return (xisnan (xx) || xx < SCM_REAL_VALUE (y)) ? x : y;
+ double xxd = SCM_REAL_VALUE (x);
+ return (xisnan (xxd) || xxd < SCM_REAL_VALUE (y)) ? x : y;
}
else if (SCM_FRACTIONP (y))
{
- double yy = scm_i_fraction2double (y);
- double xx = SCM_REAL_VALUE (x);
- return (yy < xx) ? scm_from_double (yy) : x;
+ double yyd = scm_i_fraction2double (y);
+ double xxd = SCM_REAL_VALUE (x);
+ return (yyd < xxd) ? scm_from_double (yyd) : x;
}
else
SCM_WTA_DISPATCH_2 (g_min, x, y, SCM_ARGn, s_min);
@@ -3966,8 +3969,8 @@ scm_min (SCM x, SCM y)
}
else if (SCM_REALP (y))
{
- double xx = scm_i_fraction2double (x);
- return (SCM_REAL_VALUE (y) < xx) ? y : scm_from_double (xx);
+ double xxd = scm_i_fraction2double (x);
+ return (SCM_REAL_VALUE (y) < xxd) ? y : scm_from_double (xxd);
}
else if (SCM_FRACTIONP (y))
{
@@ -4648,6 +4651,7 @@ static SCM
scm_i_divide (SCM x, SCM y, int inexact)
{
double a;
+ long xx;
if (SCM_UNLIKELY (SCM_UNBNDP (y)))
{
@@ -4711,7 +4715,7 @@ scm_i_divide (SCM x, SCM y, int inexact)
if (SCM_LIKELY (SCM_I_INUMP (x)))
{
- long xx = SCM_I_INUM (x);
+ xx = SCM_I_INUM (x);
if (SCM_LIKELY (SCM_I_INUMP (y)))
{
long yy = SCM_I_INUM (y);
diff --git a/srfi/srfi-1.c b/srfi/srfi-1.c
index 02f46fc..5ef35f0 100644
--- a/srfi/srfi-1.c
+++ b/srfi/srfi-1.c
@@ -809,6 +809,7 @@ SCM_DEFINE (scm_srfi1_filter_map, "filter-map", 2, 0, 1,
{
SCM ret, *loc, elem, newcell, lst;
int argnum;
+ scm_t_trampoline_1 proc_tramp;
SCM_VALIDATE_REST_ARGUMENT (rest);
@@ -818,7 +819,7 @@ SCM_DEFINE (scm_srfi1_filter_map, "filter-map", 2, 0, 1,
if (scm_is_null (rest))
{
/* one list */
- scm_t_trampoline_1 proc_tramp = scm_trampoline_1 (proc);
+ proc_tramp = scm_trampoline_1 (proc);
SCM_ASSERT (proc_tramp, proc, SCM_ARG1, FUNC_NAME);
for ( ; scm_is_pair (list1); list1 = SCM_CDR (list1))
@@ -840,9 +841,9 @@ SCM_DEFINE (scm_srfi1_filter_map, "filter-map", 2, 0, 1,
else if (scm_is_null (SCM_CDR (rest)))
{
/* two lists */
- scm_t_trampoline_2 proc_tramp = scm_trampoline_2 (proc);
+ scm_t_trampoline_2 proc_tramp2 = scm_trampoline_2 (proc);
SCM list2 = SCM_CAR (rest);
- SCM_ASSERT (proc_tramp, proc, SCM_ARG1, FUNC_NAME);
+ SCM_ASSERT (proc_tramp2, proc, SCM_ARG1, FUNC_NAME);
for (;;)
{
@@ -854,7 +855,7 @@ SCM_DEFINE (scm_srfi1_filter_map, "filter-map", 2, 0, 1,
argnum = 3;
goto check_lst_and_done;
}
- elem = proc_tramp (proc, SCM_CAR (list1), SCM_CAR (list2));
+ elem = proc_tramp2 (proc, SCM_CAR (list1), SCM_CAR (list2));
if (scm_is_true (elem))
{
newcell = scm_cons (elem, SCM_EOL);
@@ -1112,12 +1113,13 @@ SCM_DEFINE (scm_srfi1_list_index, "list-index", 2, 0, 1,
long n = 0;
SCM lst;
int argnum;
+ scm_t_trampoline_1 pred_tramp;
SCM_VALIDATE_REST_ARGUMENT (rest);
if (scm_is_null (rest))
{
/* one list */
- scm_t_trampoline_1 pred_tramp = scm_trampoline_1 (pred);
+ pred_tramp = scm_trampoline_1 (pred);
SCM_ASSERT (pred_tramp, pred, SCM_ARG1, FUNC_NAME);
for ( ; scm_is_pair (list1); n++, list1 = SCM_CDR (list1))
@@ -1133,8 +1135,8 @@ SCM_DEFINE (scm_srfi1_list_index, "list-index", 2, 0, 1,
{
/* two lists */
SCM list2 = SCM_CAR (rest);
- scm_t_trampoline_2 pred_tramp = scm_trampoline_2 (pred);
- SCM_ASSERT (pred_tramp, pred, SCM_ARG1, FUNC_NAME);
+ scm_t_trampoline_2 pred_tramp2 = scm_trampoline_2 (pred);
+ SCM_ASSERT (pred_tramp2, pred, SCM_ARG1, FUNC_NAME);
for ( ; ; n++)
{
@@ -1146,8 +1148,8 @@ SCM_DEFINE (scm_srfi1_list_index, "list-index", 2, 0, 1,
argnum = 3;
break;
}
- if (scm_is_true (pred_tramp (pred,
- SCM_CAR (list1), SCM_CAR (list2))))
+ if (scm_is_true (pred_tramp2 (pred,
+ SCM_CAR (list1), SCM_CAR (list2))))
return SCM_I_MAKINUM (n);
list1 = SCM_CDR (list1);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] fix jump-misses-init warnings
2009-09-04 14:36 [PATCH] fix jump-misses-init warnings Giuseppe Scrivano
@ 2009-09-14 7:37 ` Ludovic Courtès
2009-09-14 8:21 ` Ken Raeburn
0 siblings, 1 reply; 4+ messages in thread
From: Ludovic Courtès @ 2009-09-14 7:37 UTC (permalink / raw)
To: bug-guile
Hi Giuseppe,
Sorry for the late reply.
Giuseppe Scrivano <gscrivano@gnu.org> writes:
> I reported some problems compiling guile with new versions of GCC. The
> reason of these problems is that new GCC versions check for declaration
> in the current scope that are missed by a goto.
Which version is that? Is the warning enabled by ‘-Wall’? I’m using
4.4.1, which is the latest released version AIUI, without having this
problem.
> --- a/libguile/conv-uinteger.i.c
> +++ b/libguile/conv-uinteger.i.c
> @@ -26,9 +26,10 @@
> TYPE
> SCM_TO_TYPE_PROTO (SCM val)
> {
> + scm_t_signed_bits n;
> if (SCM_I_INUMP (val))
> {
> - scm_t_signed_bits n = SCM_I_INUM (val);
> + n = SCM_I_INUM (val);
> if (n >= 0
> && ((scm_t_uintmax)n) >= TYPE_MIN && ((scm_t_uintmax)n) <= TYPE_MAX)
> return n;
Why does it help to move locals to the enclosing scope? I’d expect the
opposite.
Thanks for the report!
Ludo’.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fix jump-misses-init warnings
2009-09-14 7:37 ` Ludovic Courtès
@ 2009-09-14 8:21 ` Ken Raeburn
2009-09-14 14:44 ` Ludovic Courtès
0 siblings, 1 reply; 4+ messages in thread
From: Ken Raeburn @ 2009-09-14 8:21 UTC (permalink / raw)
To: bug-guile
On Sep 14, 2009, at 03:37, Ludovic Courtès wrote:
> Why does it help to move locals to the enclosing scope? I’d expect
> the
> opposite.
Jumping over the initializer is a simpler analysis than used-before-
set. It may also yield different results, if the "goto" that branches
into the block and bypasses the initializer can now be reached through
a path that retains a value of the variable in question stored by a
previous pass through that block entering at the top. You could still
warn "possibly used before set", unless that's the only way to get to
that goto statement.
If the logic of the function requires recalculating the value on any
entry to the block, moving the declaration to an outer block is just
masking the bug, not fixing it.
In this case, though, nothing after the label uses the value of the
variable for which the initialization is bypassed, so it seems okay.
Though, I'd think the declaration could be left in the same scope, and
just split into a declaration without initializer and a separate
assignment statement; that would probably suppress the warning without
triggering a used-before-set warning, and wouldn't require renaming
all the other "n" variables. (If it doesn't, I think it might be a
smaller change to rename the variable for which the declaration is
being moved, rather than all the others.)
Ken
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix jump-misses-init warnings
2009-09-14 8:21 ` Ken Raeburn
@ 2009-09-14 14:44 ` Ludovic Courtès
0 siblings, 0 replies; 4+ messages in thread
From: Ludovic Courtès @ 2009-09-14 14:44 UTC (permalink / raw)
To: bug-guile
Hi,
Ken Raeburn <raeburn@raeburn.org> writes:
> In this case, though, nothing after the label uses the value of the
> variable for which the initialization is bypassed, so it seems okay.
> Though, I'd think the declaration could be left in the same scope, and
> just split into a declaration without initializer and a separate
> assignment statement; that would probably suppress the warning without
> triggering a used-before-set warning, and wouldn't require renaming
> all the other "n" variables. (If it doesn't, I think it might be a
> smaller change to rename the variable for which the declaration is
> being moved, rather than all the others.)
I would prefer it. Giuseppe: can you check whether it works for you?
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-09-14 14:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-04 14:36 [PATCH] fix jump-misses-init warnings Giuseppe Scrivano
2009-09-14 7:37 ` Ludovic Courtès
2009-09-14 8:21 ` Ken Raeburn
2009-09-14 14:44 ` Ludovic Courtès
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 read-only IMAP folder(s) and NNTP newsgroup(s).