Re: [PATCH] Fix will always overflow warnings (PR middle-end/40340)

[Richard Guenther <rguenther at suse dot de>]

On Fri, 5 Jun 2009, Jakub Jelinek wrote:

> Hi!
> 
> While gcc 4.3 warned for -D_FORTIFY_SOURCE={1,2} compile time detected
> buffer overflow both when the call to memcpy/memset/etc. was from
> non-inlined or inlined function, gcc 4.4/trunk only warns if it is
> not in an inlined function.  The problem is that tree_nonartificial_location
> is buggy, it follows the wrong chain (BLOCK_SUPERCONTEXT instead of
> BLOCK_ABSTRACT_ORIGIN).  The patch below fixes this by implementing
> tree_nonartificial_location on top of the correct
> block_nonartificial_location.  With -O2 -g -Wall that change fixes this,
> unfortunately with -g0/-g1 tree-ssa-live.c prunes some blocks that causes
> {block,tree}_nonartificial_location to no longer find the traces of the
> artificial function.  The tree-ssa-live.c change just avoids pruning
> the outer blocks corresponding to artificial functions.  I think that
> shouldn't have too big memory consumption effects, as artificial functions
> are quite rare and usually not stacked, so we are talking about just one
> level of extra BLOCKs for -D_FORTIFY_SOURCE={,2} protected string/memory
> inline wrappers.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.4?

Ok.

Thanks,
Richard.

> 2009-06-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/40340
> 	* tree-ssa-live.c (remove_unused_scope_block_p): Don't prune
> 	inlined_function_outer_scope_p blocks for artificial inlines
> 	even at -g0/-g1.
> 	* tree.c (tree_nonartificial_location): Rewrite using
> 	block_nonartificial_location.
> 
> 	* gcc.dg/pr40340-1.c: New test.
> 	* gcc.dg/pr40340-2.c: New test.
> 	* gcc.dg/pr40340-3.c: New test.
> 	* gcc.dg/pr40340-4.c: New test.
> 	* gcc.dg/pr40340-5.c: New test.
> 	* gcc.dg/pr40340.h: New file.
> 
> --- gcc/tree-ssa-live.c.jj	2009-05-30 10:13:04.000000000 +0200
> +++ gcc/tree-ssa-live.c	2009-06-05 12:17:10.000000000 +0200
> @@ -525,7 +525,25 @@ remove_unused_scope_block_p (tree scope)
>     /* For terse debug info we can eliminate info on unused variables.  */
>     else if (debug_info_level == DINFO_LEVEL_NONE
>  	    || debug_info_level == DINFO_LEVEL_TERSE)
> -     ;
> +     {
> +       /* Even for -g0/-g1 don't prune outer scopes from artificial
> +	  functions, otherwise diagnostics using tree_nonartificial_location
> +	  will not be emitted properly.  */
> +       if (inlined_function_outer_scope_p (scope))
> +	 {
> +	   tree ao = scope;
> +
> +	   while (ao
> +		  && TREE_CODE (ao) == BLOCK
> +		  && BLOCK_ABSTRACT_ORIGIN (ao) != ao)
> +	     ao = BLOCK_ABSTRACT_ORIGIN (ao);
> +	   if (ao
> +	       && TREE_CODE (ao) == FUNCTION_DECL
> +	       && DECL_DECLARED_INLINE_P (ao)
> +	       && lookup_attribute ("artificial", DECL_ATTRIBUTES (ao)))
> +	     unused = false;
> +	 }
> +     }
>     else if (BLOCK_VARS (scope) || BLOCK_NUM_NONLOCALIZED_VARS (scope))
>       unused = false;
>     /* See if this block is important for representation of inlined function.
> --- gcc/tree.c.jj	2009-05-30 10:13:04.000000000 +0200
> +++ gcc/tree.c	2009-06-05 12:20:37.000000000 +0200
> @@ -9224,32 +9224,12 @@ block_nonartificial_location (tree block
>  location_t
>  tree_nonartificial_location (tree exp)
>  {
> -  tree block = TREE_BLOCK (exp);
> +  location_t *loc = block_nonartificial_location (TREE_BLOCK (exp));
>  
> -  while (block
> -	 && TREE_CODE (block) == BLOCK
> -	 && BLOCK_ABSTRACT_ORIGIN (block))
> -    {
> -      tree ao = BLOCK_ABSTRACT_ORIGIN (block);
> -
> -      do
> -	{
> -	  if (TREE_CODE (ao) == FUNCTION_DECL
> -	      && DECL_DECLARED_INLINE_P (ao)
> -	      && lookup_attribute ("artificial", DECL_ATTRIBUTES (ao)))
> -	    return BLOCK_SOURCE_LOCATION (block);
> -	  else if (TREE_CODE (ao) == BLOCK
> -		   && BLOCK_SUPERCONTEXT (ao) != ao)
> -	    ao = BLOCK_SUPERCONTEXT (ao);
> -	  else
> -	    break;
> -	}
> -      while (ao);
> -
> -      block = BLOCK_SUPERCONTEXT (block);
> -    }
> -
> -  return EXPR_LOCATION (exp);
> +  if (loc)
> +    return *loc;
> +  else
> +    return EXPR_LOCATION (exp);
>  }
>  
>  
> --- gcc/testsuite/gcc.dg/pr40340-1.c.jj	2009-06-05 13:04:37.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr40340-1.c	2009-06-05 13:20:06.000000000 +0200
> @@ -0,0 +1,24 @@
> +/* PR middle-end/40340 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -Wno-system-headers" } */
> +
> +#include "pr40340.h"
> +
> +static inline
> +__attribute__ ((always_inline))
> +void
> +test (char *p)
> +{
> +  memset (p, 0, 6);
> +}
> +
> +int
> +main (void)
> +{
> +  char buf[4];
> +  test (buf);
> +  return 0;
> +}
> +
> +/* { dg-warning "will always overflow destination buffer" "" { target *-*-* } 10 } */
> +/* { dg-message "file included" "In file included" { target *-*-* } 0 } */
> --- gcc/testsuite/gcc.dg/pr40340-2.c.jj	2009-06-05 13:04:37.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr40340-2.c	2009-06-05 13:20:24.000000000 +0200
> @@ -0,0 +1,16 @@
> +/* PR middle-end/40340 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -Wno-system-headers" } */
> +
> +#include "pr40340.h"
> +
> +int
> +main (void)
> +{
> +  char buf[4];
> +  memset (buf, 0, 6);
> +  return 0;
> +}
> +
> +/* { dg-warning "will always overflow destination buffer" "" { target *-*-* } 10 } */
> +/* { dg-message "file included" "In file included" { target *-*-* } 0 } */
> --- gcc/testsuite/gcc.dg/pr40340-3.c.jj	2009-06-05 13:04:37.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr40340-3.c	2009-06-05 13:21:07.000000000 +0200
> @@ -0,0 +1,15 @@
> +/* PR middle-end/40340 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -Wno-system-headers" } */
> +
> +#define TEST2
> +#include "pr40340.h"
> +
> +int
> +main (void)
> +{
> +  test2 ();
> +  return 0;
> +}
> +
> +/* { dg-bogus "will always overflow destination buffer" "" { target *-*-* } 10 } */
> --- gcc/testsuite/gcc.dg/pr40340-4.c.jj	2009-06-05 13:04:37.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr40340-4.c	2009-06-05 13:22:42.000000000 +0200
> @@ -0,0 +1,16 @@
> +/* PR middle-end/40340 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -Wno-system-headers -g" } */
> +
> +#define TEST3
> +#include "pr40340.h"
> +
> +int
> +main (void)
> +{
> +  char buf[4];
> +  test3 (buf);
> +  return 0;
> +}
> +
> +/* { dg-bogus "will always overflow destination buffer" "" { target *-*-* } 10 } */
> --- gcc/testsuite/gcc.dg/pr40340-5.c.jj	2009-06-05 13:04:37.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr40340-5.c	2009-06-05 13:24:23.000000000 +0200
> @@ -0,0 +1,17 @@
> +/* PR middle-end/40340 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -Wsystem-headers -g" } */
> +
> +#define TEST3
> +#include "pr40340.h"
> +
> +int
> +main (void)
> +{
> +  char buf[4];
> +  test3 (buf);
> +  return 0;
> +}
> +
> +/* { dg-warning "will always overflow destination buffer" "" { target *-*-* } 10 } */
> +/* { dg-message "file included" "In file included" { target *-*-* } 0 } */
> --- gcc/testsuite/gcc.dg/pr40340.h.jj	2009-06-05 13:04:34.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr40340.h	2009-06-05 13:24:49.000000000 +0200
> @@ -0,0 +1,31 @@
> +#pragma GCC system_header
> +typedef __SIZE_TYPE__ size_t;
> +extern void *memset (void *s, int c, size_t n)
> +  __attribute__ ((nothrow, nonnull (1)));
> +extern inline
> +__attribute__ ((always_inline, artificial, gnu_inline, nothrow))
> +void *
> +memset (void *dest, int ch, size_t len)
> +{
> +  return __builtin___memset_chk (dest, ch, len,
> +				 __builtin_object_size (dest, 0));
> +}
> +
> +#ifdef TEST2
> +static void
> +__attribute__ ((noinline))
> +test2 (void)
> +{
> +  char buf[4];
> +  memset (buf, 0, 6);
> +}
> +#endif
> +
> +#ifdef TEST3
> +static inline void
> +__attribute__ ((always_inline))
> +test3 (char *p)
> +{
> +  memset (p, 0, 6);
> +}
> +#endif
> 
> 	Jakub
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex