Re: [PATCH] Fix ptr-overflow sanopt optimization (PR sanitizer/83392)

[Richard Biener <rguenther at suse dot de>]

On Tue, 13 Mar 2018, Jakub Jelinek wrote:

> Hi!
> 
> The sanopt maybe_optimize_ubsan_ptr_ifn optimization behaves differently
> on 32-bit and on 64-bit targets when using similar arguments maximum or
> minimum of ptrdiff_t or values close to it.
> 
> The problem is that UHWI is 64-bit, regardless of whether addresses are
> 64-bit or 32-bit, so for 32-bit targets get_inner_reference returns
> NULL offset and all the offset is in pbitpos, while on 64-bit targets
> where such large offsets in bytes would fit into shwi, but in bits won't
> fit, we return INTEGER_CST offset and often 0 pbitpos.
> 
> The following patch handles such offset, so that the sanopt behaves the
> same way, and adjusts the testcase (which really should have just 14
> matches, the 3 lines with comment changes are already covered by the
> overflow check on p = b - SMAX;).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2018-03-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/83392
> 	* sanopt.c (maybe_optimize_ubsan_ptr_ifn): Handle also
> 	INTEGER_CST offset, add it together with bitpos / 8 and
> 	sign extend based on POINTER_SIZE.
> 
> 	* c-c++-common/ubsan/ptr-overflow-sanitization-1.c: Adjust expected
> 	check count from 17 to 14.
> 
> --- gcc/sanopt.c.jj	2018-03-02 00:15:54.670780980 +0100
> +++ gcc/sanopt.c	2018-03-13 16:54:49.905621373 +0100
> @@ -486,12 +486,17 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx
>        HOST_WIDE_INT bitpos;
>        base = get_inner_reference (base, &bitsize, &pbitpos, &offset, &mode,
>  				  &unsignedp, &reversep, &volatilep);
> -      if (offset == NULL_TREE
> +      if ((offset == NULL_TREE || TREE_CODE (offset) == INTEGER_CST)
>  	  && DECL_P (base)
>  	  && pbitpos.is_constant (&bitpos))
>  	{
>  	  gcc_assert (!DECL_REGISTER (base));
> -	  offset_int expr_offset = bitpos / BITS_PER_UNIT;
> +	  offset_int expr_offset;
> +	  if (offset)
> +	    expr_offset = wi::to_offset (offset) + bitpos / BITS_PER_UNIT;
> +	  else
> +	    expr_offset = bitpos / BITS_PER_UNIT;
> +	  expr_offset = wi::sext (expr_offset, POINTER_SIZE);
>  	  offset_int total_offset = expr_offset + cur_offset;
>  	  if (total_offset != wi::sext (total_offset, POINTER_SIZE))
>  	    {
> @@ -511,7 +516,7 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx
>  	      && (!is_global_var (base) || decl_binds_to_current_def_p (base)))
>  	    {
>  	      offset_int base_size = wi::to_offset (DECL_SIZE_UNIT (base));
> -	      if (bitpos >= 0
> +	      if (!wi::neg_p (expr_offset)
>  		  && wi::les_p (total_offset, base_size))
>  		{
>  		  if (!wi::neg_p (total_offset)
> @@ -532,7 +537,7 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx
>  	     */
>  
>  	  bool sign_cur_offset = !wi::neg_p (cur_offset);
> -	  bool sign_expr_offset = bitpos >= 0;
> +	  bool sign_expr_offset = !wi::neg_p (expr_offset);
>  
>  	  tree base_addr
>  	    = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (base)), base);
> --- gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c.jj	2017-10-11 22:37:52.798901780 +0200
> +++ gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c	2018-03-13 17:00:18.947808006 +0100
> @@ -25,9 +25,9 @@ void foo(void)
>    p2 = p + 2;
>  
>    p = b - SMAX; /* pointer overflow check is needed */
> -  p2 = p + (SMAX - 2); /* b - 2: pointer overflow check is needed */
> -  p2 = p + (SMAX - 1); /* b - 1: pointer overflow check is needed */
> -  p2 = p + SMAX; /* b: pointer overflow check is needed */
> +  p2 = p + (SMAX - 2); /* b - 2: no need to check this  */
> +  p2 = p + (SMAX - 1); /* b - 1: no need to check this */
> +  p2 = p + SMAX; /* b: no need to check this */
>    p2++; /* b + 1 */
>  
>    p = c;
> @@ -75,4 +75,4 @@ void negative_to_negative (char *ptr)
>    p2 += 5;
>  }
>  
> -/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 17 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 14 "optimized" } } */
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)