[PATCH 2/3] Simplify and inline get_uleb128 and get_sleb128

Mark Wielaard mjw at redhat.com
Tue Apr 22 14:55:41 UTC 2014


On Thu, 2013-12-12 at 11:06 -0800, Josh Stone wrote:
> On 12/12/2013 04:13 AM, Petr Machata wrote:
> > Josh Stone <jistone at redhat.com> writes:
> >> +#define get_sleb128_step(var, addr, nth)			  \
> >>    do {							  \
> >> +    unsigned char __b = *(addr)++;				  \
> >> +    if (likely ((__b & 0x80) == 0))				  \
> >>        {							  \
> >> +	   struct { signed int i:7; } __s = { .i = __b };	  \
> >> +	   (var) |= (typeof (var)) __s.i << ((nth) * 7);	  \
> > 
> > Oh, the bitfield trick is clever!
> 
> I should give credit, I found that trick here:
> http://graphics.stanford.edu/~seander/bithacks.html#FixedSignExtend
> 
> The former code was trying to sign-extend after the value was shifted
> and combined, which as a variable width is harder.  I really like that
> this way the compiler is fully aware that this is a sign extension,
> rather than being a side effect of ORing bits or left-right shifts.

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.

The attached patch is very much not clever, but does what is intended in
a well-defined way (it is basically what the DWARF spec gives as pseudo
code). Does anybody see a better way?

Thanks,

Mark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libdw-get_sleb128_step-Remove-undefined-behavior.patch
Type: text/x-patch
Size: 1707 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/elfutils-devel/attachments/20140422/0f2a2495/attachment.bin>


More information about the elfutils-devel mailing list