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;
