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

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

Hi,

This patch implements a suggestion from Andrew Morton to warn
about this kind of obvious mistakes:

void foo(void)
{}

void bar(void)
{}

int main()
{
        if (foo)
                bar();
        return 0;
}

The "if (foo)" part here apparently caused a bug in the kernel
that nobody detected for a whole year :)

g++ already unconditionally warned about this kind of thing, with
the comment that this is a common mistake Pascal and Ada programmers
would make.  The attached patch simply moves the warning from
cp/cvt.c to c-common.c, with 2 an additional lines to work around
the fact that the  C front end has not yet converted (foo) to (&foo)
when it calls c_common_truthvalue_conversion.

Also, one test case needed to be fixed -- we actually had code in
the test suite that triggers the warning.

Booted-'n-tested on i686-pc-linux-gnu with no new regressions.
OK for trunk?

Gr.
Steven

gcc/
	* c-common.c (c_common_truthvalue_conversion): Warn if the
	address of a non-weak function is used as a truth value.

cp/
	* cvt.c (ocp_convert): Move warning to C common code.


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 19:34:37 -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 = build_unary_op (ADDR_EXPR, exp, 0);
+ 
  #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 evaluate as `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 19:34:51 -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))
Index: testsuite/gcc.dg/weak/weak-3.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.dg/weak/weak-3.c,v
retrieving revision 1.3
diff -c -3 -p -r1.3 weak-3.c
*** testsuite/gcc.dg/weak/weak-3.c	9 Jun 2003 17:30:09 -0000	1.3
--- testsuite/gcc.dg/weak/weak-3.c	12 Oct 2003 19:39:04 -0000
*************** extern void * ffoo1f (void);    
*** 54,60 ****
  extern void * ffoox1f (void);
  void * foo1f (void)
  {
!   if (ffoo1f)
      ffoo1f ();
    return 0;
  }
--- 54,60 ----
  extern void * ffoox1f (void);
  void * foo1f (void)
  {
!   if (ffoo1f) /* { dg-warning "" }
      ffoo1f ();
    return 0;
  }