Mark Wielaard <mjw(a)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