| Message ID | 20170621080048.GN2123@tucnak |
|---|---|
| State | New |
| Headers | show |
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 > >
--- 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