Page MenuHomePhabricator

[libunwind] Make it possible to use libunwind without heap.
ClosedPublic

Authored by whitequark on Aug 10 2015, 1:22 AM.

Details

Summary

This patch allows to use libunwind on bare-metal systems that do not include malloc/free by conditionally turning off nonessential functionality that requires these functions.

The disabled functionality includes:

  • the .cfi_remember_state and .cfi_restore_state instructions;
  • the DWARF FDE cache.

The .cfi_{remember,restore}_state instructions don't seem to be used by contemporary compilers. None of the LLVM backends emit it.

The DWARF FDE cache is bypassed if _LIBUNWIND_NO_HEAP is defined. Specifically, entries are never added to it, so the search begins and ends at the statically allocated, empty initial cache.

Such heap-less libunwind on a bare metal system is successfully used in the ARTIQ project, and it is my hope that it will be useful elsewhere.

Diff Detail

Repository
rL LLVM

Event Timeline

whitequark updated this revision to Diff 31639.Aug 10 2015, 1:22 AM
whitequark retitled this revision from to [libunwind] Make it possible to use libunwind without heap..
whitequark updated this object.
whitequark added reviewers: rengolin, kledzik.
whitequark updated this object.
whitequark added a subscriber: llvm-commits.
whitequark updated this revision to Diff 31641.Aug 10 2015, 1:25 AM

Whitespace.

whitequark updated this revision to Diff 31642.Aug 10 2015, 1:27 AM

Whitespace.

compnerd added inline comments.Aug 11 2015, 7:28 PM
src/UnwindCursor.hpp
898 ↗(On Diff #31642)

Please use:

#if !defined(_LIBUNWIND_NO_HEAP)
    DwarfFDECache<A>::add(sects.dso_base, fdeInfo.pcStart, fdeInfo.pcEnd,
                          fdeInfo.fdeStart);
#endif

rather than the empty clause with a semicolon.

Even better would be be make the add operation a no-op if built without support for the heap. You could do that statically, avoiding the mess of conditional compilation.

whitequark added inline comments.Aug 12 2015, 3:53 AM
src/UnwindCursor.hpp
898 ↗(On Diff #31642)

This would break the preceding conditionally included:

if (sects.dwarf_index_section == 0)

I could refactor it.

How would you suggest doing it statically?

compnerd edited edge metadata.Aug 12 2015, 7:59 PM

The FDECache already is templated, take a second parameter which is _LIBUNWIND_NO_HEAP. If disabled, just return.

I've tried to do that, but it appears that you cannot partially specialize a member function. As a result, the object has the annoying property of having undefined references to malloc/free, which the current solution avoids.

Do you think it would be valuable to have no references to malloc/free in no-heap builds? (I think it is: as opposed to having a stub implementation, it allows to catch erroneous references in third-party code earlier.)

If yes, can you suggest a way to conditionally call them that wouldn't involve the preprocessor?

I agree that would be useful. Unfortunately, partial template specialization in this case would be difficult. I would just create a functor that represents the body and is templated on the heap availability.

whitequark updated this revision to Diff 35211.Sep 20 2015, 9:55 PM
whitequark edited edge metadata.

I've tried to refactor this code to use a functor, but the functor would have to somehow gain access to the private fields of DwarfFDECache. Every solution I could imagine was far more contrived than a #ifdef; if you have a good one, I'm all ears.

In light of this and also the fact that there are more #ifdef's anyway in the DWARF parser, I've instead changed the patch to remove the heinous semicolon hack. Now it is just the body of add() which is conditionally included. This looks straightforward enough to me.

What do you think?

whitequark updated this object.

I've realized there is no need to disable any of the _unw_* functions now that DwarfFDECache::add() is a no-op on heapless systems, so I have simplified the patch.

compnerd accepted this revision.Nov 8 2015, 10:51 PM
compnerd edited edge metadata.

I tend to prefer !defined() over ndef. But thats stylistic.

This revision is now accepted and ready to land.Nov 8 2015, 10:51 PM
This revision was automatically updated to reflect the committed changes.