Differential D1967

Attempt to make libcxxrt's dwarf_eh work with strict alignment
ClosedPublic
Actions

Authored by dim on Feb 25 2015, 10:10 PM.

Details

Reviewers

theraven
bapt
andrew
imp
• ian

Commits

rS279307: Make libcxxrt's parsing of DWARF exception handling tables work on

Summary

In the thread starting here:
https://lists.freebsd.org/pipermail/freebsd-arm/2015-January/009998.html

Daisuke Aoyama describes how libcxxrt does not properly handle parsing
an exception table on an RPi (e.g. arm). This is because parts of
dwarf_eh.h read 16 bit, 32 bit and 64 bit values directly from possibly
unaligned addresses.

He posted a workaround patch here:
https://lists.freebsd.org/pipermail/freebsd-arm/2015-January/010014.html

but it is incomplete, as it does not handle the 16 and 64 bit cases, nor
does it work for other architectures with strict alignment.

Here is another attempt, where I put the reading of different sized
objects into their own static inline functions, and try to handle
alignment and endianness properly.

This seems to work on i386 and amd64, but I can't test arm and/or mips
myself.

Also, I now assume the NO_STRICT_ALIGNMENT and LITTLE_ENDIAN__
macros are available, and works as expected. That shoudl probably be
refined before we send this upstream. Unfortunately these are both
non-standard, and there seems to be no reliable way of determining them
portably.

Test Plan

Build and install on various arches, with and without strict alignment,
and with little and big endian ordering.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

dim updated this revision to Diff 3978.Feb 25 2015, 10:10 PM

dim retitled this revision from to Attempt to make libcxxrt's dwarf_eh work with strict alignment.

dim updated this object.

dim edited the test plan for this revision. (Show Details)

dim added reviewers: andrew, bapt, • ian, theraven.

dim added a subscriber: Unknown Object (MLST).

emaste added a subscriber: emaste.Feb 25 2015, 10:29 PM

I'd just do a memcpy version of the read routines. All this #ifdef junk is insane. It will do the right thing every time.

contrib/libcxxrt/dwarf_eh.h
154–163	uint16_t res; memcpy(&res, reinterpret_cast<void >(data), sizeof(uint16_)t); would be better.
176–188	see above for less insane code.
200–221	see above for less insane code.

ah, forgot that LITTLE_ENDIAN isn't universally available (or didn't used to be, that may have changed).

contrib/libcxxrt/dwarf_eh.h
157	Also, LITTLE_ENDIAN isn't universally available (or hasn't been in the past). But there's no need since a memcpy version doesn't need it or NO_STRICT_ALIGNMENT.

dim added inline comments.Feb 25 2015, 10:50 PM

contrib/libcxxrt/dwarf_eh.h
157	But memcpy doesn't handle endianness? Or are we assuming the data has been written in the same endianness as the machine is using?

It doesn't matter because the code already assumed endianness with its preprocessor LITTLE_ENDIAN. The prior code assumed native endinaness as well, since it just did the cast.

memcpy is exactly what you want to replace the old code with.

Replacing the assignments with memcpy. This is indeed much more
concise, thanks Warner. :-)

Simplify even more, by getting rid of the unnecessary 'v' variable.

imp accepted this revision.Feb 25 2015, 11:15 PM

imp added a reviewer: imp.

This revision is now accepted and ready to land.Feb 25 2015, 11:15 PM

tested successfully with kyua which was triggering the bug

Closed by commit rS279307 (authored by @dim).

Revision Contents
Changeset List

Path

Size

contrib/

libcxxrt/

dwarf_eh.h

18 lines

Diff 3982

View Options

contrib/libcxxrt/dwarf_eh.h

Attempt to make libcxxrt's dwarf_eh work with strict alignmentClosedPublicActions