[debuginfo-tests] a11117a - [dexter] Remove unnecessary double check on conditional breakpoints
via llvm-commits
llvm-commits at lists.llvm.org
Mon May 17 01:03:01 PDT 2021
Author: OCHyams
Date: 2021-05-17T09:01:55+01:00
New Revision: a11117a7c0a23f37bdee5c611bbaac22e89c775d
URL: https://github.com/llvm/llvm-project/commit/a11117a7c0a23f37bdee5c611bbaac22e89c775d
DIFF: https://github.com/llvm/llvm-project/commit/a11117a7c0a23f37bdee5c611bbaac22e89c775d.diff
LOG: [dexter] Remove unnecessary double check on conditional breakpoints
Remove the `ConditionalController._conditional_met` method. This was missed in
the recent ConditionalController refactor (D98699). We don't need to check that
the conditions for a conditional breakpoint have been met because
`DebuggerBase.get_triggered_breakpoint_ids` returns the set of ids for
breakpoints which have been triggered.
To get the "triggered breakpoints" from lldb we use `GetStopReasonDataCount`
and `GetStopReasonDataAtIndex`. It seems that these functions count all
breakpoints associated with the location which lldb has stopped at, regardless
of their condition. i.e. Even if we have two breakpoints at the same source
location that have mutually exclusive conditions, both will be found this way
when either condition is true. To get around this, we store a map of breakpoint
{id: condition} `_breakpoint_conditions` and evaluate the conditions of the
triggered breakpoints to filter the set down to those which are unconditional
or have a condition which evaluates to true.
Essentially we are just moving the condition double check from a general
debugger controller into the lldb specific wrapper. This tidy up will help make
upcoming patches simpler.
Reviewed By: chrisjackson
Differential Revision: https://reviews.llvm.org/D101431
Added:
Modified:
debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py
debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
Removed:
################################################################################
diff --git a/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py b/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py
index e225a48bcb66a..379f00f688586 100644
--- a/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py
+++ b/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py
@@ -77,13 +77,6 @@ def _set_conditional_bps(self):
cond_expr)
self._conditional_bp_handles[id] = cbp
- def _conditional_met(self, cbp):
- for cond_expr in cbp.get_conditional_expression_list():
- valueIR = self.debugger.evaluate_expression(cond_expr)
- if valueIR.type_name == 'bool' and valueIR.value == 'true':
- return True
- return False
-
def _run_debugger_custom(self):
# TODO: Add conditional and unconditional breakpoint support to dbgeng.
if self.debugger.get_name() == 'dbgeng':
@@ -116,15 +109,12 @@ def _run_debugger_custom(self):
# This is an unconditional bp. Mark it for removal.
bp_to_delete.append(bp_id)
continue
- # We have triggered a breakpoint with a condition. Check that
- # the condition has been met.
- if self._conditional_met(cbp):
- # Add a range of unconditional breakpoints covering the
- # lines requested in the DexLimitSteps command. Ignore
- # first line as that's the conditional bp we just hit and
- # include the final line.
- for line in range(cbp.range_from + 1, cbp.range_to + 1):
- self.debugger.add_breakpoint(cbp.path, line)
+ # Add a range of unconditional breakpoints covering the lines
+ # requested in the DexLimitSteps command. Ignore first line as
+ # that's the conditional bp we just hit and include the final
+ # line.
+ for line in range(cbp.range_from + 1, cbp.range_to + 1):
+ self.debugger.add_breakpoint(cbp.path, line)
# Remove any unconditional breakpoints we just hit.
for bp_id in bp_to_delete:
diff --git a/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py b/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
index 324467dd0819a..e8e8939958743 100644
--- a/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
+++ b/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
@@ -26,6 +26,9 @@ def __init__(self, context, *args):
self._target = None
self._process = None
self._thread = None
+ # Map {id (int): condition (str)} for breakpoints which have a
+ # condition. See get_triggered_breakpoint_ids usage for more info.
+ self._breakpoint_conditions = {}
super(LLDB, self).__init__(context, *args)
def _custom_init(self):
@@ -104,37 +107,67 @@ def clear_breakpoints(self):
self._target.DeleteAllBreakpoints()
def _add_breakpoint(self, file_, line):
- bp = self._target.BreakpointCreateByLocation(file_, line)
- if not bp:
- raise DebuggerException(
- 'could not add breakpoint [{}:{}]'.format(file_, line))
- return bp.GetID()
+ return self._add_conditional_breakpoint(file_, line, None)
def _add_conditional_breakpoint(self, file_, line, condition):
bp = self._target.BreakpointCreateByLocation(file_, line)
- if bp:
- bp.SetCondition(condition)
- else:
+ if not bp:
raise DebuggerException(
'could not add breakpoint [{}:{}]'.format(file_, line))
- return bp.GetID()
+ id = bp.GetID()
+ if condition:
+ bp.SetCondition(condition)
+ assert id not in self._breakpoint_conditions
+ self._breakpoint_conditions[id] = condition
+ return id
+
+ def _evaulate_breakpoint_condition(self, id):
+ """Evaluate the breakpoint condition and return the result.
+
+ Returns True if a conditional breakpoint with the specified id cannot
+ be found (i.e. assume it is an unconditional breakpoint).
+ """
+ try:
+ condition = self._breakpoint_conditions[id]
+ except KeyError:
+ # This must be an unconditional breakpoint.
+ return True
+ valueIR = self.evaluate_expression(condition)
+ return valueIR.type_name == 'bool' and valueIR.value == 'true'
def get_triggered_breakpoint_ids(self):
# Breakpoints can only have been triggered if we've hit one.
stop_reason = self._translate_stop_reason(self._thread.GetStopReason())
if stop_reason != StopReason.BREAKPOINT:
return []
+ breakpoint_ids = set()
+ # When the stop reason is eStopReasonBreakpoint, GetStopReasonDataCount
+ # counts all breakpoints associated with the location that lldb has
+ # stopped at, regardless of their condition. I.e. Even if we have two
+ # breakpoints at the same source location that have mutually exclusive
+ # conditions, both will be counted by GetStopReasonDataCount when
+ # either condition is true. Check each breakpoint condition manually to
+ # filter the list down to breakpoints that have caused this stop.
+ #
# Breakpoints have two data parts: Breakpoint ID, Location ID. We're
# only interested in the Breakpoint ID so we skip every other item.
- return set([self._thread.GetStopReasonDataAtIndex(i)
- for i in range(0, self._thread.GetStopReasonDataCount(), 2)])
+ for i in range(0, self._thread.GetStopReasonDataCount(), 2):
+ id = self._thread.GetStopReasonDataAtIndex(i)
+ if self._evaulate_breakpoint_condition(id):
+ breakpoint_ids.add(id)
+ return breakpoint_ids
def delete_breakpoint(self, id):
bp = self._target.FindBreakpointByID(id)
if not bp:
# The ID is not valid.
raise KeyError
- self._target.BreakpointDelete(bp.GetID())
+ try:
+ del self._breakpoint_conditions[id]
+ except KeyError:
+ # This must be an unconditional breakpoint.
+ pass
+ self._target.BreakpointDelete(id)
def launch(self):
self._process = self._target.LaunchSimple(None, None, os.getcwd())
More information about the llvm-commits
mailing list