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 <[email protected]>
>
> * 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 <[email protected]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
