This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Always use unwind tables in tests
ClosedPublic

Authored by peter.smith on Sep 5 2017, 9:49 AM.

Details

Summary

For many targets setting -fno-exceptions will prevent unwinding tables from being generated for the test programs. As libunwind depends on the tables to unwind the stack several tests will fail.

This change always adds -funwind-tables so that even when -fno-exceptions is set unwind tables are generated. On x86_64 clang will add this option implicitly as the ABI requires it, I've tested this with Arm and AArch64.

This is an attempt at fixing pr33858 https://bugs.llvm.org/show_bug.cgi?id=33858. I'm not experienced in this area so there may well be better ways of doing this.

Diff Detail

Event Timeline

peter.smith created this revision.Sep 5 2017, 9:49 AM
jroelofs added inline comments.Sep 5 2017, 10:15 AM
test/libunwind/test/config.py
46

I don't think that's correct. It should be:

if not self.get_lit_bool('enable_exceptions', True):
    self.cxx.compile_flags += ['-fno-exceptions', '-DLIBUNWIND_HAS_NO_EXCEPTIONS']

i.e: treat 'enable_exceptions' as True if it's not found, and only add the compile flags turning exceptions off when it's known to be False.

Yes you are correct, thanks for pointing that out. I had somewhat naively assumed the True and False was the test. I've updated the diff.

Has anyone had any more thoughts on this or is there a different approach that I can try to fix this pr?

I can report that this patch clears two test failures for MIPS when D38110 is applied.

rengolin accepted this revision.Oct 25 2017, 11:50 AM
rengolin added a subscriber: joerg.

Hi Peter,

This has been marked as a blocker to 5.0.1. I'm approving it in light of three facts:

  1. Our long discussions from many angles have pointed us to this solution in particular
  2. You have addressed @joerg's comments
  3. If also fixes Mips errors, as stated by @sdardis

Please update PR33858 with the commit revision, so we can backport to the release_50 branch.

Thanks!

This revision is now accepted and ready to land.Oct 25 2017, 11:50 AM
This revision was automatically updated to reflect the committed changes.