Page MenuHomePhabricator

Change lldb_assert() to use llvm::sys::PrintBacktrace
AbandonedPublic

Authored by zturner on Mar 4 2015, 5:06 PM.

Details

Reviewers
None
Summary

This way we get backtraces on all platforms, and in a consistent format.

Diff Detail

Event Timeline

zturner updated this revision to Diff 21246.Mar 4 2015, 5:06 PM
zturner retitled this revision from to Change lldb_assert() to use llvm::sys::PrintBacktrace.
zturner updated this object.
zturner edited the test plan for this revision. (Show Details)
zturner added a reviewer: granata.enrico.
zturner added a subscriber: Unknown Object (MLST).
granata.enrico added inline comments.Mar 4 2015, 5:15 PM
source/Utility/LLDBAssert.cpp
14

I would not do this.
Printing to a Stream is the LLDB way to do this, no reason for switching to this LLVM API

36

Printing to stderr is probably a good idea
But, again, I prefer to stick to the LLDB host layer

It's probably fine to reimplement Host::Backtrace() in terms of LLVM APIs if it can be done generally and with decent performance, but I don't see much in terms of added value in this change

Hmm, I'm not sure I agree. Whether it prints to a Stream or directly to
stderr is kind of an implementation detail. Not very important since it
just ends up to stdout or stderr anwyay and we don't do anything else with
the backtrace except print it and throw it away.

llvm already has functionality built in to serve exactly this purpose. Why
shouldn't we use it? Not only are we sure that it's implemented on all
platforms that LLVM supports, but the format is consistent on all of these
platforms, and anyway why reinvent the wheel?

granata.enrico resigned from this revision.Oct 15 2015, 1:34 PM
granata.enrico removed a reviewer: granata.enrico.
zturner abandoned this revision.Oct 15 2015, 1:48 PM