From 7e46a721fc7ea46f72a4fcf81062a76d6539f61d Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Fri, 24 Sep 2021 10:27:22 +0100 Subject: [PATCH] Reapply "[Dexter] Improve performance by evaluating expressions only when needed" Fixes issue found on greendragon buildbot, in which an incorrectly indented statement following an if block led to entire frames being dropped instead of simply filtering unneeded watches. This reverts commit 1f44fa3ac17ceacc753019092bc50436c77ddcfa. --- .../dexter/dex/command/CommandBase.py | 3 +++ .../command/commands/DexExpectProgramState.py | 22 +++++++++++++--- .../command/commands/DexExpectWatchBase.py | 8 +++--- .../dexter/dex/debugger/DebuggerBase.py | 15 +++++++++++ .../dexter/dex/debugger/dbgeng/dbgeng.py | 12 ++++++--- .../dexter/dex/debugger/lldb/LLDB.py | 18 +++++++++---- .../dex/debugger/visualstudio/VisualStudio.py | 25 +++++++++++-------- 7 files changed, 78 insertions(+), 25 deletions(-) diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/command/CommandBase.py b/cross-project-tests/debuginfo-tests/dexter/dex/command/CommandBase.py index 49e908623dfd..fdeb97f71f29 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/command/CommandBase.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/command/CommandBase.py @@ -10,8 +10,11 @@ which will then be executed by DExTer during debugging. """ import abc +from collections import namedtuple from typing import List +StepExpectInfo = namedtuple('StepExpectInfo', 'expression, path, frame_idx, line_range') + class CommandBase(object, metaclass=abc.ABCMeta): def __init__(self): self.path = None diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py b/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py index 26f97b61590e..24b760ae9146 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py @@ -10,7 +10,7 @@ during execution. from itertools import chain -from dex.command.CommandBase import CommandBase +from dex.command.CommandBase import CommandBase, StepExpectInfo from dex.dextIR import ProgramState, SourceLocation, StackFrame, DextIR def frame_from_dict(source: dict) -> StackFrame: @@ -56,9 +56,23 @@ class DexExpectProgramState(CommandBase): return __class__.__name__ def get_watches(self): - frame_expects = chain.from_iterable(frame.watches - for frame in self.expected_program_state.frames) - return set(frame_expects) + frame_expects = set() + for idx, frame in enumerate(self.expected_program_state.frames): + path = (frame.location.path if + frame.location and frame.location.path else self.path) + line_range = ( + range(frame.location.lineno, frame.location.lineno + 1) + if frame.location and frame.location.lineno else None) + for watch in frame.watches: + frame_expects.add( + StepExpectInfo( + expression=watch, + path=path, + frame_idx=idx, + line_range=line_range + ) + ) + return frame_expects def eval(self, step_collection: DextIR) -> bool: for step in step_collection.steps: diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py b/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py index e892f01619ca..1c2d544a237d 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py @@ -12,11 +12,13 @@ import abc import difflib import os +from collections import namedtuple -from dex.command.CommandBase import CommandBase +from dex.command.CommandBase import CommandBase, StepExpectInfo from dex.command.StepValueInfo import StepValueInfo + class DexExpectWatchBase(CommandBase): def __init__(self, *args, **kwargs): if len(args) < 2: @@ -68,7 +70,7 @@ class DexExpectWatchBase(CommandBase): def get_watches(self): - return [self.expression] + return [StepExpectInfo(self.expression, self.path, 0, range(self._from_line, self._to_line + 1))] @property def line_range(self): @@ -149,11 +151,11 @@ class DexExpectWatchBase(CommandBase): return differences def eval(self, step_collection): + assert os.path.exists(self.path) for step in step_collection.steps: loc = step.current_location if (loc.path and os.path.exists(loc.path) and - os.path.exists(self.path) and os.path.samefile(loc.path, self.path) and loc.lineno in self.line_range): try: diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerBase.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerBase.py index c31be774d0fa..a795b3b0a28e 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerBase.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerBase.py @@ -13,11 +13,26 @@ import traceback import unittest from types import SimpleNamespace +from dex.command.CommandBase import StepExpectInfo from dex.dextIR import DebuggerIR, FrameIR, LocIR, StepIR, ValueIR from dex.utils.Exceptions import DebuggerException from dex.utils.Exceptions import NotYetLoadedDebuggerException from dex.utils.ReturnCode import ReturnCode +def watch_is_active(watch_info: StepExpectInfo, path, frame_idx, line_no): + _, watch_path, watch_frame_idx, watch_line_range = watch_info + # If this watch should only be active for a specific file... + if watch_path and os.path.isfile(watch_path): + # If the current path does not match the expected file, this watch is + # not active. + if not (path and os.path.isfile(path) and + os.path.samefile(path, watch_path)): + return False + if watch_frame_idx != frame_idx: + return False + if watch_line_range and line_no not in list(watch_line_range): + return False + return True class DebuggerBase(object, metaclass=abc.ABCMeta): def __init__(self, context): diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/dbgeng/dbgeng.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/dbgeng/dbgeng.py index c95aa54f7e6b..2b13398d0f30 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/dbgeng/dbgeng.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/dbgeng/dbgeng.py @@ -9,7 +9,7 @@ import sys import os import platform -from dex.debugger.DebuggerBase import DebuggerBase +from dex.debugger.DebuggerBase import DebuggerBase, watch_is_active from dex.dextIR import FrameIR, LocIR, StepIR, StopReason, ValueIR from dex.dextIR import ProgramState, StackFrame, SourceLocation from dex.utils.Exceptions import DebuggerException, LoadDebuggerException @@ -133,8 +133,14 @@ class DbgEng(DebuggerBase): column=0), watches={}) for expr in map( - lambda watch, idx=i: self.evaluate_expression(watch, idx), - watches): + # Filter out watches that are not active in the current frame, + # and then evaluate all the active watches. + lambda watch_info, idx=i: + self.evaluate_expression(watch_info.expression, idx), + filter( + lambda watch_info, idx=i, line_no=loc.lineno, path=loc.path: + watch_is_active(watch_info, path, idx, line_no), + watches)): state_frame.watches[expr.expression] = expr state_frames.append(state_frame) diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py index e8e893995874..ce47619db135 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py @@ -12,7 +12,7 @@ import os from subprocess import CalledProcessError, check_output, STDOUT import sys -from dex.debugger.DebuggerBase import DebuggerBase +from dex.debugger.DebuggerBase import DebuggerBase, watch_is_active from dex.dextIR import FrameIR, LocIR, StepIR, StopReason, ValueIR from dex.dextIR import StackFrame, SourceLocation, ProgramState from dex.utils.Exceptions import DebuggerException, LoadDebuggerException @@ -208,6 +208,7 @@ class LLDB(DebuggerBase): 'column': sb_line.GetColumn() } loc = LocIR(**loc_dict) + valid_loc_for_watch = loc.path and os.path.exists(loc.path) frame = FrameIR( function=function, is_inlined=sb_frame.IsInlined(), loc=loc) @@ -223,10 +224,17 @@ class LLDB(DebuggerBase): is_inlined=frame.is_inlined, location=SourceLocation(**loc_dict), watches={}) - for expr in map( - lambda watch, idx=i: self.evaluate_expression(watch, idx), - watches): - state_frame.watches[expr.expression] = expr + if valid_loc_for_watch: + for expr in map( + # Filter out watches that are not active in the current frame, + # and then evaluate all the active watches. + lambda watch_info, idx=i: + self.evaluate_expression(watch_info.expression, idx), + filter( + lambda watch_info, idx=i, line_no=loc.lineno, loc_path=loc.path: + watch_is_active(watch_info, loc_path, idx, line_no), + watches)): + state_frame.watches[expr.expression] = expr state_frames.append(state_frame) if len(frames) == 1 and frames[0].function is None: diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/visualstudio/VisualStudio.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/visualstudio/VisualStudio.py index b4558e2d8a50..e36b353cef6f 100644 --- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/visualstudio/VisualStudio.py +++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/visualstudio/VisualStudio.py @@ -11,10 +11,10 @@ import imp import os import sys from pathlib import PurePath -from collections import namedtuple -from collections import defaultdict +from collections import defaultdict, namedtuple -from dex.debugger.DebuggerBase import DebuggerBase +from dex.command.CommandBase import StepExpectInfo +from dex.debugger.DebuggerBase import DebuggerBase, watch_is_active from dex.dextIR import FrameIR, LocIR, StepIR, StopReason, ValueIR from dex.dextIR import StackFrame, SourceLocation, ProgramState from dex.utils.Exceptions import Error, LoadDebuggerException @@ -244,6 +244,9 @@ class VisualStudio(DebuggerBase, metaclass=abc.ABCMeta): # pylint: disable=abst state_frames = [] + loc = LocIR(**self._location) + valid_loc_for_watch = loc.path and os.path.exists(loc.path) + for idx, sf in enumerate(stackframes): frame = FrameIR( function=self._sanitize_function_name(sf.FunctionName), @@ -254,20 +257,20 @@ class VisualStudio(DebuggerBase, metaclass=abc.ABCMeta): # pylint: disable=abst if any(name in fname for name in self.frames_below_main): break - state_frame = StackFrame(function=frame.function, is_inlined=frame.is_inlined, watches={}) - for watch in watches: - state_frame.watches[watch] = self.evaluate_expression( - watch, idx) + if valid_loc_for_watch and idx == 0: + for watch_info in watches: + if watch_is_active(watch_info, loc.path, idx, loc.lineno): + watch_expr = watch_info.expression + state_frame.watches[watch_expr] = self.evaluate_expression(watch_expr, idx) state_frames.append(state_frame) frames.append(frame) - loc = LocIR(**self._location) if frames: frames[0].loc = loc state_frames[0].location = SourceLocation(**self._location) @@ -298,9 +301,11 @@ class VisualStudio(DebuggerBase, metaclass=abc.ABCMeta): # pylint: disable=abst ] def evaluate_expression(self, expression, frame_idx=0) -> ValueIR: - self.set_current_stack_frame(frame_idx) + if frame_idx != 0: + self.set_current_stack_frame(frame_idx) result = self._debugger.GetExpression(expression) - self.set_current_stack_frame(0) + if frame_idx != 0: + self.set_current_stack_frame(0) value = result.Value is_optimized_away = any(s in value for s in [