Commit 0e9a5646 authored by Kenton Varda's avatar Kenton Varda

Improve stack trace accuracy by subtracting 1 from each return address.

This should prevent stack traces from spurriously pointing at the line after the one where the call actually happened.
parent 3174079c
......@@ -138,6 +138,42 @@ TEST(Exception, ScopeSuccessFail) {
}
#endif
#if __GNUG__
kj::String testStackTrace() __attribute__((noinline));
#elif _MSC_VER
__declspec(noinline) kj::String testStackTrace();
#endif
kj::String testStackTrace() {
// getStackTrace() normally skips its immediate caller, so we wrap it in another layer.
return getStackTrace();
}
KJ_TEST("getStackTrace() returns correct line number, not line + 1") {
// Backtraces normally produce the return address of each stack frame, but that's usually the
// address immediately after the one that made the call. As a result, it used to be that stack
// traces often pointed to the line after the one that made a call, which was confusing. This
// checks that this bug is fixed.
//
// This is not a very robust test, because:
// 1) Since symbolic stack traces are not available in many situations (e.g. release builds
// lacking debug symbols, systems where addr2line isn't present, etc.), we only check that
// the stack trace does *not* contain the *wrong* value, rather than checking that it does
// contain the right one.
// 2) This test only detects the problem if the call instruction to testStackTrace() is the
// *last* instruction attributed to its line of code. Whether or not this is true seems to be
// dependent on obscure complier behavior. For example, below, it could only be the case if
// RVO is applied -- but in my testing, RVO does seem to be applied here. I tried several
// variations involving passing via an output parameter or a global variable rather than
// returning, but found some variations detected the problem and others didn't, essentially
// at random.
auto trace = testStackTrace();
auto wrong = kj::str("exception-test.c++:", __LINE__);
KJ_ASSERT(strstr(trace.cStr(), wrong.cStr()) == nullptr, trace, wrong);
}
} // namespace
} // namespace _ (private)
} // namespace kj
......@@ -172,6 +172,16 @@ ArrayPtr<void* const> getStackTrace(ArrayPtr<void*> space, uint ignoreCount) {
return getStackTrace(space, ignoreCount, GetCurrentThread(), context);
#elif KJ_HAS_BACKTRACE
size_t size = backtrace(space.begin(), space.size());
for (auto& addr: space.slice(0, size)) {
// The addresses produced by backtrace() are return addresses, which means they point to the
// instruction immediately after the call. Invoking addr2line on these can be confusing because
// it often points to the next line. If the next instruction is inlined from another function,
// the trace can be extra-confusing, since now it claims to be in a function that was not
// actually on the call stack. If we subtract 1 from each address, though, we get a much more
// reasonable trace. This may cause the addresses to be invalid instruction pointers if the
// instructions were multi-byte, but it appears addr2line is able to cope with this.
addr = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(addr) - 1);
}
return space.slice(kj::min(ignoreCount + 1, size), size);
#else
return nullptr;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment