Mark Wielaard <[email protected]> writes:
> Sadly the neat trick triggers undefined behavior since we are trying to
> left shift a negative value. Even though it appears to work currently I
> am slightly afraid a compiler optimization might take advantage of this
> in the future (since it is undefined behavior it could just assume
> negative values won't occur) especially since this code is inlined in a
> lot of places, causing hard to diagnose errors.
Ouch. Yeah, I agree, it is essentially a matter of time before this
whole thing is optimized away or something.
> diff --git a/libdw/memory-access.h b/libdw/memory-access.h
> index d0ee63c..c6e4bdc 100644
> --- a/libdw/memory-access.h
> +++ b/libdw/memory-access.h
> @@ -70,8 +70,9 @@ __libdw_get_uleb128 (const unsigned char **addrp)
> unsigned char __b = *(addr)++; \
> if (likely ((__b & 0x80) == 0)) \
> {
> \
> - struct { signed int i:7; } __s = { .i = __b }; \
> - (var) |= (typeof (var)) __s.i << ((nth) * 7); \
> + (var) |= (typeof (var)) (__b & 0x7f) << ((nth) * 7); \
> + if ((((nth) + 1) < 8 * sizeof (var)) && (__b & 0x40)) \
> + (var) |= -(((uint64_t) 1) << (((nth) + 1) * 7)); \
> return (var); \
> }
> \
> (var) |= (typeof (var)) (__b & 0x7f) << ((nth) * 7); \
Wouldn't something like this get us off the hook as well?
- (var) |= (typeof (var)) __s.i << ((nth) * 7); \
+ (var) |= (typeof (var)) \
+ (((uint64_t) (typeof (var)) __s.i) << ((nth) * 7)); \
We are really only using the bitfield trick to avoid having to
sign-extend by hand, but we can shift unsigned without losing anything.
Thanks,
PM
