libunwind: [ARM] EHABI getInfoFromEHABISection can fail if last .ARM.exidx table entry contains unwind information

[Quuxplusone]

Bugzilla Link	PR31091
Status	NEW
Importance	P normal
Reported by	Peter Smith (smithp352@googlemail.com)
Reported on	2016-11-21 08:16:59 -0800
Last modified on	2016-11-28 04:14:58 -0800
Version	3.9
Hardware	PC Linux
CC	compnerd@compnerd.org, emaste@freebsd.org, jroelofs@jroelofs.com, llvm-bugs@lists.llvm.org, mclow.lists@gmail.com, rafael@espindo.la, ruiu@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

If the last entry of the .ARM.exidx table contains unwinding information
libunwind will be unable to find it.

I have been testing out lld's ARM port against the libcxxabi tests and found a
few exceptions test failures. These failures would disappear if I reordered the
functions so that no function that threw and caught an exception was after main.

I tracked the point of failure down to getInfoFromEHABISection:

template <typename A, typename R>
bool UnwindCursor<A, R>::getInfoFromEHABISection(
    pint_t pc,
    const UnwindInfoSections &sects) {
  EHABISectionIterator<A> begin =
      EHABISectionIterator<A>::begin(_addressSpace, sects);
  EHABISectionIterator<A> end =
      EHABISectionIterator<A>::end(_addressSpace, sects);

  EHABISectionIterator<A> itNextPC = std::upper_bound(begin, end, pc);
  if (itNextPC == begin || itNextPC == end)
    return false;
  EHABISectionIterator<A> itThisPC = itNextPC - 1;

  pint_t thisPC = itThisPC.functionAddress();
  pint_t nextPC = itNextPC.functionAddress();
  pint_t indexDataAddr = itThisPC.dataAddress();

If the last entry in the table contains the pc then upper_bound will return end
and return false. When I reorder the table entries main is last and the tests
don't throw through main so all thrown exceptions are caught as expected.

It turns out that ld.bfd inserts a sentinel EXIDX_CANTUNWIND .ARM.exidx entry
as the last entry in the table (usually the address of rodata is used) which
means that this implementation will always work when ld.bfd is used.

A sentinel entry is useful as without it the scope of the last table entry is
the start of the program to the end of the address space (unless some other
information from the runtime is used to find the highest acceptable PC).
However I cannot find anywhere within the EHABI specification
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf
that requires the presence of a sentinel entry. Unless I've missed something I
think that the libunwind implementation isn't compliant with the specification.

As ld.bfd always adds a terminating table entry this can't be reproduced with
it.

I used the lld ARM port using libcxx libcxxabi and libunwind running
catch_pointer_nullptr.pass.cpp (the templated tests are generated after main).

It may be possible, even beneficial to get lld to add a sentinel terminating
table entry in the same way, however I think it is still worth raising this as
there are other third party linkers that may use libunwind and won't
necessarily add a sentinel.

[Quuxplusone]

I agree that adding a sentinel is probably beneficial. How easy/hard is it to implement it in LLD?

[Quuxplusone]

The two ways I can think of to implement are:
- Hard code it by reserving an extra 8 bytes of space in the Output Section and
writing the sentinel in directly.
- Create a EXIDX_CANTUNWIND .ARM.exidx InputSection that has a link order
dependency on the first InputSection with a higher address than the highest
executable code section. The rest will drop out naturally.

The first option is the simplest and allows an arbitrary address to be used for
the highest address as we are writing the values directly.

The second option is cleaner and would also allow us to generate
EXIDX_CANTUNWIND entries for sections without .ARM.exidx sections. This is done
by Gold and ld.bfd as it makes intentional uses of libunwind to unwind the
stack give better error messages.

It may be more difficult to implement as I'm not sure how easy it is to add an
InputSection with relocations to an existing InputFile.

[Quuxplusone]

I lean towards hardcoding it because it's probably easier to understand. I was thinking something like this. What do you think? https://reviews.llvm.org/D26915

[Quuxplusone]

I lean towards hardcoding it because it's probably easier to understand. I was thinking something like this. What do you think? https://reviews.llvm.org/D26915

[Quuxplusone]

Agreed, I can start work on a patch tomorrow.

[Quuxplusone]

I've posted a lld patch to add a sentinel at https://reviews.llvm.org/D26977 this works for standard links, but does not currently work for linker scripts. I'm investigating that right now.