Re: warning when a function's address is tested?

[Steven Bosscher <s dot bosscher at student dot tudelft dot nl>]

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