* warning when a function's address is tested?
@ 2003-10-12 16:04 Andrew Morton
2003-10-12 16:08 ` Falk Hueffner
2003-10-12 16:27 ` Jeff Sturm
0 siblings, 2 replies; 11+ messages in thread
From: Andrew Morton @ 2003-10-12 16:04 UTC (permalink / raw)
To: gcc
We just found a silly bug in the Linux kernel:
- if (current_is_kswapd)
+ if (current_is_kswapd())
It was there for a year. It is a fairly easy mistake to make, and it would
be nice if the compiler could generate a warning. I don't think there are
likely to be legitimate uses?
void foo(void)
{}
void bar(void)
{}
int main()
{
if (foo)
bar();
return 0;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:04 warning when a function's address is tested? Andrew Morton
@ 2003-10-12 16:08 ` Falk Hueffner
2003-10-12 16:57 ` Steven Bosscher
2003-10-12 16:27 ` Jeff Sturm
1 sibling, 1 reply; 11+ messages in thread
From: Falk Hueffner @ 2003-10-12 16:08 UTC (permalink / raw)
To: Andrew Morton; +Cc: gcc
Andrew Morton <akpm@osdl.org> writes:
> We just found a silly bug in the Linux kernel:
>
> - if (current_is_kswapd)
> + if (current_is_kswapd())
>
> It was there for a year. It is a fairly easy mistake to make, and
> it would be nice if the compiler could generate a warning. I don't
> think there are likely to be legitimate uses?
Looks like a good idea. The C++ frontend already has this warning, so
it shouldn't be too hard.
--
Falk
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:04 warning when a function's address is tested? Andrew Morton
2003-10-12 16:08 ` Falk Hueffner
@ 2003-10-12 16:27 ` Jeff Sturm
2003-10-12 16:46 ` John Levon
2003-10-12 16:56 ` Falk Hueffner
1 sibling, 2 replies; 11+ messages in thread
From: Jeff Sturm @ 2003-10-12 16:27 UTC (permalink / raw)
To: Andrew Morton; +Cc: gcc
On Sun, 12 Oct 2003, Andrew Morton wrote:
> - if (current_is_kswapd)
> + if (current_is_kswapd())
>
> It was there for a year. It is a fairly easy mistake to make, and it would
> be nice if the compiler could generate a warning. I don't think there are
> likely to be legitimate uses?
One legitimate use is to test for undefined weak symbols. That's not an
argument against the warning however... more often than not this is surely
a mistake.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:27 ` Jeff Sturm
@ 2003-10-12 16:46 ` John Levon
2003-10-12 16:56 ` Falk Hueffner
1 sibling, 0 replies; 11+ messages in thread
From: John Levon @ 2003-10-12 16:46 UTC (permalink / raw)
To: Jeff Sturm; +Cc: Andrew Morton, gcc
On Sun, Oct 12, 2003 at 12:00:15PM -0400, Jeff Sturm wrote:
> One legitimate use is to test for undefined weak symbols. That's not an
> argument against the warning however... more often than not this is surely
> a mistake.
And the code in cp/cvt.c explicitly checks for this and doesn't warn
anyway.
john
--
Khendon's Law:
If the same point is made twice by the same person, the thread is over.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:27 ` Jeff Sturm
2003-10-12 16:46 ` John Levon
@ 2003-10-12 16:56 ` Falk Hueffner
2003-10-12 17:37 ` Jeff Sturm
1 sibling, 1 reply; 11+ messages in thread
From: Falk Hueffner @ 2003-10-12 16:56 UTC (permalink / raw)
To: Jeff Sturm; +Cc: Andrew Morton, gcc
Jeff Sturm <jsturm@one-point.com> writes:
> On Sun, 12 Oct 2003, Andrew Morton wrote:
> > - if (current_is_kswapd)
> > + if (current_is_kswapd())
> >
> > It was there for a year. It is a fairly easy mistake to make, and it would
> > be nice if the compiler could generate a warning. I don't think there are
> > likely to be legitimate uses?
>
> One legitimate use is to test for undefined weak symbols.
But gcc knows that from __attribute__((weak)) and can suppress the
warning in this case (like the C++ frontend does).
--
Falk
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:08 ` Falk Hueffner
@ 2003-10-12 16:57 ` Steven Bosscher
2003-10-12 18:38 ` Zack Weinberg
0 siblings, 1 reply; 11+ messages in thread
From: Steven Bosscher @ 2003-10-12 16:57 UTC (permalink / raw)
To: Falk Hueffner; +Cc: Andrew Morton, gcc
Op zo 12-10-2003, om 14:11 schreef Falk Hueffner:
> Andrew Morton <akpm@osdl.org> writes:
>
> > We just found a silly bug in the Linux kernel:
> >
> > - if (current_is_kswapd)
> > + if (current_is_kswapd())
> >
> > It was there for a year. It is a fairly easy mistake to make, and
> > it would be nice if the compiler could generate a warning. I don't
> > think there are likely to be legitimate uses?
>
> Looks like a good idea. The C++ frontend already has this warning, so
> it shouldn't be too hard.
It is a bit tricky since for
void foo(void) {}
void bar(void) {}
int main() { if (foo) bar(); }
the C++ front end wraps foo in an ADDR_EXPR, but the C front end does
not (where IMHO it should...). Something like the attached patch should
work, I'll propose it on gcc-patches after I've tested it.
Gr.
Steven
Index: c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.462
diff -c -3 -p -r1.462 c-common.c
*** c-common.c 9 Oct 2003 08:38:43 -0000 1.462
--- c-common.c 12 Oct 2003 15:59:13 -0000
*************** c_common_truthvalue_conversion (tree exp
*** 2602,2607 ****
--- 2602,2610 ----
if (TREE_CODE (expr) == ERROR_MARK)
return expr;
+ if (TREE_CODE (expr) == FUNCTION_DECL)
+ expr = build1 (ADDR_EXPR, ptr_type_node, expr);
+
#if 0 /* This appears to be wrong for C++. */
/* These really should return error_mark_node after 2.4 is stable.
But not all callers handle ERROR_MARK properly. */
*************** c_common_truthvalue_conversion (tree exp
*** 2647,2663 ****
return real_zerop (expr) ? truthvalue_false_node : truthvalue_true_node;
case ADDR_EXPR:
! /* If we are taking the address of an external decl, it might be zero
! if it is weak, so we cannot optimize. */
! if (DECL_P (TREE_OPERAND (expr, 0))
! && DECL_EXTERNAL (TREE_OPERAND (expr, 0)))
! break;
!
! if (TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 0)))
! return build (COMPOUND_EXPR, truthvalue_type_node,
! TREE_OPERAND (expr, 0), truthvalue_true_node);
! else
! return truthvalue_true_node;
case COMPLEX_EXPR:
return build_binary_op ((TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 1))
--- 2650,2678 ----
return real_zerop (expr) ? truthvalue_false_node : truthvalue_true_node;
case ADDR_EXPR:
! {
! if (TREE_CODE (TREE_OPERAND (expr, 0)) == FUNCTION_DECL
! && ! DECL_WEAK (TREE_OPERAND (expr, 0)))
! {
! /* Common Ada/Pascal programmer's mistake. We always warn
! about this since it is so bad. */
! warning ("the address of `%D', will always be `true'",
! TREE_OPERAND (expr, 0));
! return truthvalue_true_node;
! }
!
! /* If we are taking the address of an external decl, it might be
! zero if it is weak, so we cannot optimize. */
! if (DECL_P (TREE_OPERAND (expr, 0))
! && DECL_EXTERNAL (TREE_OPERAND (expr, 0)))
! break;
!
! if (TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 0)))
! return build (COMPOUND_EXPR, truthvalue_type_node,
! TREE_OPERAND (expr, 0), truthvalue_true_node);
! else
! return truthvalue_true_node;
! }
case COMPLEX_EXPR:
return build_binary_op ((TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 1))
Index: cp/cvt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cvt.c,v
retrieving revision 1.145
diff -c -3 -p -r1.145 cvt.c
*** cp/cvt.c 5 Sep 2003 08:24:19 -0000 1.145
--- cp/cvt.c 12 Oct 2003 15:59:24 -0000
*************** ocp_convert (tree type, tree expr, int c
*** 694,713 ****
return error_mark_node;
}
if (code == BOOLEAN_TYPE)
! {
! tree fn = NULL_TREE;
- /* Common Ada/Pascal programmer's mistake. We always warn
- about this since it is so bad. */
- if (TREE_CODE (expr) == FUNCTION_DECL)
- fn = expr;
- else if (TREE_CODE (expr) == ADDR_EXPR
- && TREE_CODE (TREE_OPERAND (expr, 0)) == FUNCTION_DECL)
- fn = TREE_OPERAND (expr, 0);
- if (fn && !DECL_WEAK (fn))
- warning ("the address of `%D', will always be `true'", fn);
- return cp_truthvalue_conversion (e);
- }
return fold (convert_to_integer (type, e));
}
if (POINTER_TYPE_P (type) || TYPE_PTR_TO_MEMBER_P (type))
--- 694,701 ----
return error_mark_node;
}
if (code == BOOLEAN_TYPE)
! return cp_truthvalue_conversion (e);
return fold (convert_to_integer (type, e));
}
if (POINTER_TYPE_P (type) || TYPE_PTR_TO_MEMBER_P (type))
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:56 ` Falk Hueffner
@ 2003-10-12 17:37 ` Jeff Sturm
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Sturm @ 2003-10-12 17:37 UTC (permalink / raw)
To: Falk Hueffner; +Cc: Andrew Morton, gcc
On 12 Oct 2003, Falk Hueffner wrote:
> > One legitimate use is to test for undefined weak symbols.
>
> But gcc knows that from __attribute__((weak)) and can suppress the
> warning in this case (like the C++ frontend does).
Thanks, didn't know c++ did that. So, never mind...
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 16:57 ` Steven Bosscher
@ 2003-10-12 18:38 ` Zack Weinberg
2003-10-12 23:14 ` Gabriel Dos Reis
0 siblings, 1 reply; 11+ messages in thread
From: Zack Weinberg @ 2003-10-12 18:38 UTC (permalink / raw)
To: Steven Bosscher; +Cc: Falk Hueffner, Andrew Morton, gcc
Steven Bosscher <s.bosscher@student.tudelft.nl> writes:
> Something like the attached patch should work, I'll propose it on
> gcc-patches after I've tested it.
Informally speaking, the patch looks good - just a couple of notes:
1) there's now code in the C front end that handles a bare
FUNCTION_DECL - it should be found and removed.
2) this sentence
> ! warning ("the address of `%D', will always be `true'",
> ! TREE_OPERAND (expr, 0));
is awkward English - suggest removing the comma and the quotes around 'true'.
zw
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 18:38 ` Zack Weinberg
@ 2003-10-12 23:14 ` Gabriel Dos Reis
2003-10-12 23:58 ` Zack Weinberg
0 siblings, 1 reply; 11+ messages in thread
From: Gabriel Dos Reis @ 2003-10-12 23:14 UTC (permalink / raw)
To: Zack Weinberg; +Cc: Steven Bosscher, Falk Hueffner, Andrew Morton, gcc
"Zack Weinberg" <zack@codesourcery.com> writes:
| > ! warning ("the address of `%D', will always be `true'",
| > ! TREE_OPERAND (expr, 0));
|
| is awkward English - suggest removing the comma and the quotes around 'true'.
"the address of '%D' will always evaluate to true"
?
-- Gaby
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 23:14 ` Gabriel Dos Reis
@ 2003-10-12 23:58 ` Zack Weinberg
2003-10-13 0:46 ` Gabriel Dos Reis
0 siblings, 1 reply; 11+ messages in thread
From: Zack Weinberg @ 2003-10-12 23:58 UTC (permalink / raw)
To: Gabriel Dos Reis; +Cc: Steven Bosscher, Falk Hueffner, Andrew Morton, gcc
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> "Zack Weinberg" <zack@codesourcery.com> writes:
>
> | > ! warning ("the address of `%D', will always be `true'",
> | > ! TREE_OPERAND (expr, 0));
> |
> | is awkward English - suggest removing the comma and the quotes around 'true'.
>
>
> "the address of '%D' will always evaluate to true"
That's a better choice of verb, but then I think you want to put the
quotes around 'true' back, and maybe use 'as' instead of 'to'.
zw
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: warning when a function's address is tested?
2003-10-12 23:58 ` Zack Weinberg
@ 2003-10-13 0:46 ` Gabriel Dos Reis
0 siblings, 0 replies; 11+ messages in thread
From: Gabriel Dos Reis @ 2003-10-13 0:46 UTC (permalink / raw)
To: Zack Weinberg; +Cc: Steven Bosscher, Falk Hueffner, Andrew Morton, gcc
"Zack Weinberg" <zack@codesourcery.com> writes:
| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
|
| > "Zack Weinberg" <zack@codesourcery.com> writes:
| >
| > | > ! warning ("the address of `%D', will always be `true'",
| > | > ! TREE_OPERAND (expr, 0));
| > |
| > | is awkward English - suggest removing the comma and the quotes around 'true'.
| >
| >
| > "the address of '%D' will always evaluate to true"
|
| That's a better choice of verb, but then I think you want to put the
| quotes around 'true' back, and maybe use 'as' instead of 'to'.
I agree with your suggestion.
Thanks,
-- Gaby
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2003-10-12 18:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-12 16:04 warning when a function's address is tested? Andrew Morton
2003-10-12 16:08 ` Falk Hueffner
2003-10-12 16:57 ` Steven Bosscher
2003-10-12 18:38 ` Zack Weinberg
2003-10-12 23:14 ` Gabriel Dos Reis
2003-10-12 23:58 ` Zack Weinberg
2003-10-13 0:46 ` Gabriel Dos Reis
2003-10-12 16:27 ` Jeff Sturm
2003-10-12 16:46 ` John Levon
2003-10-12 16:56 ` Falk Hueffner
2003-10-12 17:37 ` Jeff Sturm
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).