Fix ReactorDebugInfo off by one line issues Before this commit, when debugging with the ReactorDebugInfo feature, variables were being incorrectly displayed in the debugger. For instance, given: Int a = 1; Int b = 2; The debugger would not display 'a', and would display 'b' as containing the value '1'. The reason for this was that while the Int constructor for 'a' was executing, we invoke boost::stacktrace() to determine the variable's source location (file and line number); however, the stack trace would return the return address of the constructor call, which would be on the next line -- that of 'Int b'. To fix this, knowing that all ReactorDebugInfo emits come from function calls in Reactor code, we assume that all backtrace addresses point to the next instruction following the call, thus we subtract 1 from these addresses. When resolving the file and line number, this gets back the previous instruction's location. There are likely corner cases where this doesn't work, but it's better than what we have now. Bug: b/161821289 Change-Id: Ie92ead393f321fc1a26fc019d6450bfddb7a5e24 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/46868 Reviewed-by: Ben Clayton <bclayton@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com> Kokoro-Result: kokoro <noreply+kokoro@google.com> Tested-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/ReactorDebugInfo.cpp b/src/Reactor/ReactorDebugInfo.cpp index 60d3f8d..1884999 100644 --- a/src/Reactor/ReactorDebugInfo.cpp +++ b/src/Reactor/ReactorDebugInfo.cpp
@@ -49,6 +49,8 @@ Backtrace getCallerBacktrace(size_t limit /* = 0 */) { + namespace bs = boost::stacktrace; + auto shouldSkipFile = [](const std::string &fileSR) { return fileSR.empty() || endswith_lower(fileSR, "ReactorDebugInfo.cpp") || @@ -58,15 +60,31 @@ endswith_lower(fileSR, "stacktrace.hpp"); }; - std::vector<Location> locations; + auto offsetStackFrames = [](const bs::stacktrace &st) { + // Return a stack trace with every stack frame address offset by -1. We do this so that we get + // back the location of the function call, and not the location following it. We need this since + // all debug info emits are the result of a function call. Note that technically we shouldn't + // perform this offsetting on the top-most stack frame, but it doesn't matter as we discard it + // anyway (see shouldSkipFile). - namespace bs = boost::stacktrace; + std::vector<bs::frame> result; + result.reserve(st.size()); + + for(bs::frame frame : st) + { + result.emplace_back(reinterpret_cast<void *>(reinterpret_cast<size_t>(frame.address()) - 1)); + } + + return result; + }; + + std::vector<Location> locations; // Cache to avoid expensive stacktrace lookups, especially since our use-case results in looking up the // same call stack addresses many times. static std::unordered_map<bs::frame::native_frame_ptr_t, Location> cache; - for(bs::frame frame : bs::stacktrace()) + for(bs::frame frame : offsetStackFrames(bs::stacktrace())) { Location location;