This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH 2/3] Simplify and inline get_uleb128 and get_sleb128
- From: Petr Machata <pmachata at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Wed, 23 Apr 2014 12:17:13 +0200
- Subject: Re: [PATCH 2/3] Simplify and inline get_uleb128 and get_sleb128
Mark Wielaard <mjw@redhat.com> 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