Re: [RFC PATCH] -fsanitize=pointer-overflow support (PR sanitizer/80998)

[Richard Biener <rguenther@suse.de>]

On Wed, 21 Jun 2017, Jakub Jelinek wrote:

> On Tue, Jun 20, 2017 at 09:41:43AM +0200, Richard Biener wrote:
> > > 2) libcpp/line-map.c has this:
> > > static int
> > > location_adhoc_data_update (void **slot, void *data)
> > > {
> > >   *((char **) slot) += *((int64_t *) data);
> > >   return 1;
> > > }
> > > where the (why int64_t always?, we really need just intptr_t) adjusts
> > > one pointer from an unrelated one (result of realloc).  That is a UB
> > > and actually can trigger this sanitization if the two regions are
> > > far away from each other, e.g. on i686-linux:
> > > ../../libcpp/line-map.c:102:21: runtime error: pointer index expression with base 0x0899e308 overflowed to 0xf74c4ab8
> > > ../../libcpp/line-map.c:102:21: runtime error: pointer index expression with base 0x08add7c0 overflowed to 0xf74c9a08
> > > ../../libcpp/line-map.c:102:21: runtime error: pointer index expression with base 0x092ba308 overflowed to 0xf741cab8
> > > ../../libcpp/line-map.c:102:21: runtime error: pointer index expression with base 0x0a3757c0 overflowed to 0xf7453a08
> > > Shall we perform the addition in uintptr_t instead to make it
> > > implementation defined rather than UB?
> > 
> > Yes.
> 
> Here is a patch for 2), bootstrap-ubsan bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> Note both ptrdiff_t and uintptr_t are already used in libcpp, so I think
> it shouldn't create new portability issues.
> 
> 2017-06-21  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* line-map.c (location_adhoc_data_update): Perform addition in
> 	uintptr_t type rather than char * type.  Read *data using
> 	ptrdiff_t type instead of int64_t.
> 	(get_combined_adhoc_loc): Change offset type to ptrdiff_t from
> 	int64_t.
> 
> --- libcpp/line-map.c.jj	2017-06-19 08:28:18.000000000 +0200
> +++ libcpp/line-map.c	2017-06-20 16:43:25.193063344 +0200
> @@ -99,7 +99,8 @@ location_adhoc_data_eq (const void *l1,
>  static int
>  location_adhoc_data_update (void **slot, void *data)
>  {
> -  *((char **) slot) += *((int64_t *) data);
> +  *((char **) slot)
> +    = (char *) ((uintptr_t) *((char **) slot) + *((ptrdiff_t *) data));
>    return 1;
>  }
>  
> @@ -221,7 +222,7 @@ get_combined_adhoc_loc (struct line_maps
>  	  set->location_adhoc_data_map.allocated)
>  	{
>  	  char *orig_data = (char *) set->location_adhoc_data_map.data;
> -	  int64_t offset;
> +	  ptrdiff_t offset;
>  	  /* Cast away extern "C" from the type of xrealloc.  */
>  	  line_map_realloc reallocator = (set->reallocator
>  					  ? set->reallocator
> 
> 
> 	Jakub
> 
> 

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