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