RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 23:47:45 +00:00
Code Issues Activity Actions

HackStudio: Make Editor ask Debugger if a breakpoint was added/removed

Browse source 
Rather than adding/removing a breakpoint indicator, and then telling the
debugger about it and hoping it works, let the debugger tell us if it
succeeded and then use that to update the indicator.

This prevents the user from adding breakpoints to invalid locations
while the debugger is running. It also avoids a couple of scary
VERIFY()s. We still allow creating breakpoints in invalid locations
while the debugger is *not* running.
This commit is contained in:
Sam Atkins 2024-01-11 14:57:57 +00:00 • committed by Andrew Kaster
parent 3910efb80b
commit ad59fb7cf0
3 changed files with 43 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

56
Userland/DevTools/HackStudio/Debugger/Debugger.cpp
View file

@ -1,5 +1,6 @@
/* /*
* Copyright (c) 2020, Itamar S. <itamar8910@gmail.com> * Copyright (c) 2020, Itamar S. <itamar8910@gmail.com>
* Copyright (c) 2024, Sam Atkins <atkinssj@serenityos.org>
* *
* SPDX-License-Identifier: BSD-2-Clause * SPDX-License-Identifier: BSD-2-Clause
*/ */
@ -48,36 +49,44 @@ Debugger::Debugger(
pthread_cond_init(&m_ui_action_cond, nullptr); pthread_cond_init(&m_ui_action_cond, nullptr);
} }
void Debugger::on_breakpoint_change(ByteString const& file, size_t line, BreakpointChange change_type) bool Debugger::change_breakpoint(ByteString const& file, size_t line, BreakpointChange change_type)
{ {
auto position = create_source_position(file, line); auto position = create_source_position(file, line);
if (change_type == BreakpointChange::Added) {
m_breakpoints.append(position);
} else {
m_breakpoints.remove_all_matching([&](Debug::DebugInfo::SourcePosition const& val) { return val == position; });
}
auto session = Debugger::the().session(); auto session = Debugger::the().session();
if (!session) if (session) {
return; auto address = session->get_address_from_source_position(position.file_path, position.line_number);
if (!address.has_value()) {
dbgln("Warning: couldn't get instruction address from source");
return false;
}
auto address = session->get_address_from_source_position(position.file_path, position.line_number); switch (change_type) {
if (!address.has_value()) { case BreakpointChange::Added:
dbgln("Warning: couldn't get instruction address from source"); if (session->insert_breakpoint(address.value().address)) {
// TODO: Currently, the GUI will indicate that a breakpoint was inserted/removed at this line, m_breakpoints.append(position);
// regardless of whether we actually succeeded to insert it. (For example a breakpoint on a comment, or an include statement). return true;
// We should indicate failure via a return value from this function, and not update the breakpoint GUI if we fail. }
return; break;
case BreakpointChange::Removed:
if (session->remove_breakpoint(address.value().address)) {
m_breakpoints.remove_all_matching([&](Debug::DebugInfo::SourcePosition const& val) { return val == position; });
return true;
}
break;
}
return false;
} }
if (change_type == BreakpointChange::Added) { // No active session, so just modify our internal list of breakpoints
bool success = session->insert_breakpoint(address.value().address); switch (change_type) {
VERIFY(success); case BreakpointChange::Added:
} else { m_breakpoints.append(position);
bool success = session->remove_breakpoint(address.value().address); return true;
VERIFY(success); case BreakpointChange::Removed:
m_breakpoints.remove_all_matching([&](Debug::DebugInfo::SourcePosition const& val) { return val == position; });
return true;
} }
VERIFY_NOT_REACHED();
} }
bool Debugger::set_execution_position(ByteString const& file, size_t line) bool Debugger::set_execution_position(ByteString const& file, size_t line)
@ -125,6 +134,7 @@ void Debugger::start()
bool success = m_debug_session->insert_breakpoint(address.value().address); bool success = m_debug_session->insert_breakpoint(address.value().address);
VERIFY(success); VERIFY(success);
} else { } else {
// FIXME: Report the invalid breakpoint to the GUI somehow.
dbgln("couldn't insert breakpoint"); dbgln("couldn't insert breakpoint");
} }
} }

2
Userland/DevTools/HackStudio/Debugger/Debugger.h
View file

@ -34,7 +34,7 @@ public:
static bool is_initialized(); static bool is_initialized();
void on_breakpoint_change(ByteString const& file, size_t line, BreakpointChange change_type); [[nodiscard]] bool change_breakpoint(ByteString const& file, size_t line, BreakpointChange change_type);
bool set_execution_position(ByteString const& file, size_t line); bool set_execution_position(ByteString const& file, size_t line);
void set_executable_path(ByteString const& path) { m_executable_path = path; } void set_executable_path(ByteString const& path) { m_executable_path = path; }

15
Userland/DevTools/HackStudio/Editor.cpp
View file

@ -1,6 +1,7 @@
/* /*
* Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org> * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
* Copyright (c) 2018-2022, the SerenityOS developers. * Copyright (c) 2018-2022, the SerenityOS developers.
* Copyright (c) 2023-2024, Sam Atkins <atkinssj@serenityos.org>
* *
* SPDX-License-Identifier: BSD-2-Clause * SPDX-License-Identifier: BSD-2-Clause
*/ */
@ -774,9 +775,10 @@ void Editor::set_semantic_syntax_highlighting(bool value)
ErrorOr<void> Editor::add_breakpoint(size_t line_number) ErrorOr<void> Editor::add_breakpoint(size_t line_number)
{ {
if (!breakpoint_lines().contains_slow(line_number)) { if (!breakpoint_lines().contains_slow(line_number)) {
add_gutter_indicator(m_breakpoint_indicator_id, line_number); if (Debugger::the().change_breakpoint(wrapper().filename_title(), line_number, BreakpointChange::Added)) {
TRY(breakpoint_lines().try_append(line_number)); add_gutter_indicator(m_breakpoint_indicator_id, line_number);
Debugger::the().on_breakpoint_change(wrapper().filename_title(), line_number, BreakpointChange::Added); TRY(breakpoint_lines().try_append(line_number));
}
} }
return {}; return {};
@ -784,9 +786,10 @@ ErrorOr<void> Editor::add_breakpoint(size_t line_number)
void Editor::remove_breakpoint(size_t line_number) void Editor::remove_breakpoint(size_t line_number)
{ {
remove_gutter_indicator(m_breakpoint_indicator_id, line_number); if (Debugger::the().change_breakpoint(wrapper().filename_title(), line_number, BreakpointChange::Removed)) {
breakpoint_lines().remove_first_matching([&](size_t line) { return line == line_number; }); remove_gutter_indicator(m_breakpoint_indicator_id, line_number);
Debugger::the().on_breakpoint_change(wrapper().filename_title(), line_number, BreakpointChange::Removed); breakpoint_lines().remove_first_matching([&](size_t line) { return line == line_number; });
}
} }
ErrorOr<void> Editor::update_git_diff_indicators() ErrorOr<void> Editor::update_git_diff_indicators()