Vulkan/Debug: Expose Frames from Thread
Previously Frame was an Thread-internally managed structure, with special accessors for the variable containers of the top most frame.
With this change, the Frame can be modified by calling `Thread::update()`. This allows for more powerful edits of the frame, while still preserving a mutex lock over the structure.
Bug: b/145351270
Change-Id: I81e87884e26bdd6f22b47e8f71b37bbfc91c7830
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/39882
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Vulkan/Debug/Context.cpp b/src/Vulkan/Debug/Context.cpp
index d11c130..1f51617 100644
--- a/src/Vulkan/Debug/Context.cpp
+++ b/src/Vulkan/Debug/Context.cpp
@@ -293,9 +293,9 @@
}
std::shared_ptr<Frame> Context::Lock::createFrame(
- const std::shared_ptr<File> &file)
+ const std::shared_ptr<File> &file, std::string function)
{
- auto frame = std::make_shared<Frame>(ctx->nextFrameID++);
+ auto frame = std::make_shared<Frame>(ctx->nextFrameID++, std::move(function));
ctx->frames.add(frame->id, frame);
frame->arguments = createScope(file);
frame->locals = createScope(file);
diff --git a/src/Vulkan/Debug/Context.hpp b/src/Vulkan/Debug/Context.hpp
index 790587d..4c4d8af 100644
--- a/src/Vulkan/Debug/Context.hpp
+++ b/src/Vulkan/Debug/Context.hpp
@@ -89,9 +89,10 @@
// files() returns the full list of files.
std::vector<std::shared_ptr<File>> files();
- // createFrame() returns a new frame for the given file.
+ // createFrame() returns a new frame for the given file and function
+ // name.
std::shared_ptr<Frame> createFrame(
- const std::shared_ptr<File> &file);
+ const std::shared_ptr<File> &file, std::string function);
// get() returns the frame with the given ID, or null if the frame
// does not exist or no longer has any external shared_ptr references.
diff --git a/src/Vulkan/Debug/Location.hpp b/src/Vulkan/Debug/Location.hpp
index 3990472..c924a5a 100644
--- a/src/Vulkan/Debug/Location.hpp
+++ b/src/Vulkan/Debug/Location.hpp
@@ -26,17 +26,32 @@
struct Location
{
Location() = default;
- inline Location(int line, const std::shared_ptr<File> &file);
+ inline Location(const std::shared_ptr<File> &file, int line, int column = 0);
- int line = 0; // 1 based. 0 represents no line.
+ inline bool operator==(const Location &o) const;
+ inline bool operator!=(const Location &o) const;
+
std::shared_ptr<File> file;
+ int line = 0; // 1 based. 0 represents no line.
+ int column = 0; // 1 based. 0 represents no particular column.
};
-Location::Location(int line, const std::shared_ptr<File> &file)
- : line(line)
- , file(file)
+Location::Location(const std::shared_ptr<File> &file, int line, int column /* = 0 */)
+ : file(file)
+ , line(line)
+ , column(column)
{}
+bool Location::operator==(const Location &o) const
+{
+ return file == o.file && line == o.line && column == o.column;
+}
+
+bool Location::operator!=(const Location &o) const
+{
+ return !(*this == o);
+}
+
} // namespace dbg
} // namespace vk
diff --git a/src/Vulkan/Debug/Server.cpp b/src/Vulkan/Debug/Server.cpp
index 244f5fc..f004fa8 100644
--- a/src/Vulkan/Debug/Server.cpp
+++ b/src/Vulkan/Debug/Server.cpp
@@ -198,8 +198,9 @@
dap::StackTraceResponse response;
response.totalFrames = stack.size();
response.stackFrames.reserve(stack.size());
- for(auto const &frame : stack)
+ for(int i = static_cast<int>(stack.size()) - 1; i >= 0; i--)
{
+ auto const &frame = stack[i];
auto const &loc = frame.location;
dap::StackFrame sf;
sf.column = 0;
diff --git a/src/Vulkan/Debug/Thread.cpp b/src/Vulkan/Debug/Thread.cpp
index a542e37..753b7db 100644
--- a/src/Vulkan/Debug/Thread.cpp
+++ b/src/Vulkan/Debug/Thread.cpp
@@ -38,10 +38,9 @@
return name_;
}
-void Thread::update(const Location &location)
+void Thread::onLocationUpdate(std::unique_lock<std::mutex> &lock)
{
- std::unique_lock<std::mutex> lock(mutex);
- frames.back()->location = location;
+ auto location = frames.back()->location;
if(state_ == State::Running)
{
@@ -79,11 +78,10 @@
void Thread::enter(Context::Lock &ctxlck, const std::shared_ptr<File> &file, const std::string &function)
{
- auto frame = ctxlck.createFrame(file);
+ auto frame = ctxlck.createFrame(file, function);
auto isFunctionBreakpoint = ctxlck.isFunctionBreakpoint(function);
std::unique_lock<std::mutex> lock(mutex);
- frame->function = function;
frames.push_back(frame);
if(isFunctionBreakpoint)
{
@@ -98,28 +96,22 @@
frames.pop_back();
}
-std::shared_ptr<VariableContainer> Thread::registers() const
+void Thread::update(std::function<void(Frame &)> f)
{
std::unique_lock<std::mutex> lock(mutex);
- return frames.back()->registers->variables;
+ auto &frame = *frames.back();
+ auto oldLocation = frame.location;
+ f(frame);
+ if(frame.location != oldLocation)
+ {
+ onLocationUpdate(lock);
+ }
}
-std::shared_ptr<VariableContainer> Thread::locals() const
+Frame Thread::frame() const
{
std::unique_lock<std::mutex> lock(mutex);
- return frames.back()->locals->variables;
-}
-
-std::shared_ptr<VariableContainer> Thread::arguments() const
-{
- std::unique_lock<std::mutex> lock(mutex);
- return frames.back()->arguments->variables;
-}
-
-std::shared_ptr<VariableContainer> Thread::hovers() const
-{
- std::unique_lock<std::mutex> lock(mutex);
- return frames.back()->hovers->variables;
+ return *frames.back();
}
std::vector<Frame> Thread::stack() const
diff --git a/src/Vulkan/Debug/Thread.hpp b/src/Vulkan/Debug/Thread.hpp
index 77d7425..a59f47f 100644
--- a/src/Vulkan/Debug/Thread.hpp
+++ b/src/Vulkan/Debug/Thread.hpp
@@ -70,13 +70,13 @@
public:
using ID = dbg::ID<Frame>;
- inline Frame(ID id);
+ inline Frame(ID id, std::string function);
// The unique identifier of the stack frame.
const ID id;
// The name of function for this stack frame.
- std::string function;
+ const std::string function;
// The current execution location within the stack frame.
Location location;
@@ -94,8 +94,9 @@
std::shared_ptr<Scope> hovers;
};
-Frame::Frame(ID id)
+Frame::Frame(ID id, std::string function)
: id(id)
+ , function(std::move(function))
{}
// Thread holds the state for a single thread of execution.
@@ -127,17 +128,8 @@
// exit() pops the thread's stack frame.
void exit();
- // registers() returns the thread's current stack frame's register variables.
- std::shared_ptr<VariableContainer> registers() const;
-
- // locals() returns the thread's current stack frame's local variables.
- std::shared_ptr<VariableContainer> locals() const;
-
- // arguments() returns the thread's current stack frame's argument variables.
- std::shared_ptr<VariableContainer> arguments() const;
-
- // hovers() returns the thread's current stack frame's hover variables.
- std::shared_ptr<VariableContainer> hovers() const;
+ // frame() returns a copy of the thread's top most stack frame.
+ Frame frame() const;
// stack() returns a copy of the thread's current stack frames.
std::vector<Frame> stack() const;
@@ -145,6 +137,11 @@
// state() returns the current thread's state.
State state() const;
+ // update() calls f to modify the top most frame of the stack.
+ // If the frame's location is changed, update() potentially blocks until the
+ // thread is resumed with one of the methods below.
+ void update(std::function<void(Frame &)> f);
+
// resume() resumes execution of the thread by unblocking a call to
// update() and setting the thread's state to State::Running.
void resume();
@@ -170,16 +167,14 @@
// suspend execution again.
void stepOut();
- // update() updates the current stack frame's location, and potentially
- // blocks until the thread is resumed with one of the methods above.
- void update(const Location &location);
-
// The unique identifier of the thread.
const ID id;
private:
EventListener *const broadcast;
+ void onLocationUpdate(std::unique_lock<std::mutex> &lock);
+
mutable std::mutex mutex;
std::string name_;
std::vector<std::shared_ptr<Frame>> frames;
diff --git a/src/Vulkan/VkCommandBuffer.cpp b/src/Vulkan/VkCommandBuffer.cpp
index 186b1ec..b40886a 100644
--- a/src/Vulkan/VkCommandBuffer.cpp
+++ b/src/Vulkan/VkCommandBuffer.cpp
@@ -1785,7 +1785,9 @@
#ifdef ENABLE_VK_DEBUGGER
if(debuggerThread)
{
- debuggerThread->update({ line++, debuggerFile });
+ debuggerThread->update([&](vk::dbg::Frame &frame) {
+ frame.location = { debuggerFile, line++, 0 };
+ });
}
#endif // ENABLE_VK_DEBUGGER
