From da8eea69e45246175081d9457c74783609654d95 Mon Sep 17 00:00:00 2001 From: Marcus Nilsson Date: Sat, 2 Jul 2022 20:23:19 +0200 Subject: [PATCH] LibDebug: Make sure current_breakpoint has value before usage Previously this caused a crash when no breakpoint was active since we tried to get value().address before the has_value() check. --- Userland/Libraries/LibDebug/DebugSession.h | 44 +++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/Userland/Libraries/LibDebug/DebugSession.h b/Userland/Libraries/LibDebug/DebugSession.h index 7bdf673169..dde23a1a16 100644 --- a/Userland/Libraries/LibDebug/DebugSession.h +++ b/Userland/Libraries/LibDebug/DebugSession.h @@ -278,29 +278,31 @@ void DebugSession::run(DesiredInitialDebugeeState initial_debugee_state, Callbac bool did_single_step = false; - auto current_breakpoint_address = bit_cast(current_breakpoint.value().address); // Re-enable the breakpoint if it wasn't removed by the user - if (current_breakpoint.has_value() && m_breakpoints.contains(current_breakpoint_address)) { - // The current breakpoint was removed to make it transparent to the user. - // We now want to re-enable it - the code execution flow could hit it again. - // To re-enable the breakpoint, we first perform a single step and execute the - // instruction of the breakpoint, and then redo the INT3 patch in its first byte. + if (current_breakpoint.has_value()) { + auto current_breakpoint_address = bit_cast(current_breakpoint.value().address); + if (m_breakpoints.contains(current_breakpoint_address)) { + // The current breakpoint was removed to make it transparent to the user. + // We now want to re-enable it - the code execution flow could hit it again. + // To re-enable the breakpoint, we first perform a single step and execute the + // instruction of the breakpoint, and then redo the INT3 patch in its first byte. - // If the user manually inserted a breakpoint at the current instruction, - // we need to disable that breakpoint because we want to singlestep over that - // instruction (we re-enable it again later anyways). - if (m_breakpoints.contains(current_breakpoint_address) && m_breakpoints.get(current_breakpoint_address).value().state == BreakPointState::Enabled) { - disable_breakpoint(current_breakpoint.value().address); - } - auto stopped_address = single_step(); - enable_breakpoint(current_breakpoint.value().address); - did_single_step = true; - // If there is another breakpoint after the current one, - // Then we are already on it (because of single_step) - auto breakpoint_at_next_instruction = m_breakpoints.get(stopped_address); - if (breakpoint_at_next_instruction.has_value() - && breakpoint_at_next_instruction.value().state == BreakPointState::Enabled) { - state = State::ConsecutiveBreakpoint; + // If the user manually inserted a breakpoint at the current instruction, + // we need to disable that breakpoint because we want to singlestep over that + // instruction (we re-enable it again later anyways). + if (m_breakpoints.contains(current_breakpoint_address) && m_breakpoints.get(current_breakpoint_address).value().state == BreakPointState::Enabled) { + disable_breakpoint(current_breakpoint.value().address); + } + auto stopped_address = single_step(); + enable_breakpoint(current_breakpoint.value().address); + did_single_step = true; + // If there is another breakpoint after the current one, + // Then we are already on it (because of single_step) + auto breakpoint_at_next_instruction = m_breakpoints.get(stopped_address); + if (breakpoint_at_next_instruction.has_value() + && breakpoint_at_next_instruction.value().state == BreakPointState::Enabled) { + state = State::ConsecutiveBreakpoint; + } } }