[dexter] Change --source-root-dir and add --debugger-use-relative-paths

[Orlando]

Summary

Note: This is based on top of D99651 which addds the DexDeclareFile command. DexDeclareFile(path) sets subsequent commands' path attributes to path.

We want to use DexDeclareFile to specify paths relative to a project root directory. The option --source-root-dir, prior to this patch, causes dexter to strip the path prefix from commands before passing them to a debugger, and appends the prefix to file paths returned from a debugger. This patch changes the behviour of --source-root-dir. Relative paths in commands, made possible with DexDeclareFile(relative/path), are appended to the --source-root-dir directory.

A new option, --debugger-use-relative-paths, can be used alongside --source-root-dir to reproduce the old behaviour: all paths passed to the debugger will be made relative to --source-root-dir.

I've added a regression test debuginfo-tests/dexter/feature_tests/commands/perfect/dex_declare_file/precompiled_binary_different_dir/dex_commands/source_root_dir.dex for this new behaviour, and modified the existing --source-root-dir regression and unit tests to use --debugger-use-relative-paths.

[Orlando]

Ping

[chrisjackson]

One comment related nit, otherwise lgtm.

debuginfo-tests/dexter/dex/command/ParseCommand.py

282

I was going to say 'if the option has a value' is unclear, but the code is sufficiently clear that I think this comment can be removed.

[Orlando]

+Remove unnecessary comment.

[jmorse]

LGTM with a nit

debuginfo-tests/dexter/dex/command/ParseCommand.py

281–285

Doesn't need the parentheses around the tertiary expression, right?

284

Fee fi fo fum, I smell the need for PurePath, coming to a Dexter near you soon.

[Orlando]

Thanks both for the reviews.

debuginfo-tests/dexter/dex/command/ParseCommand.py

284

Agreed. That code is from D99651 so I'll wait for it to land and then see what can be done.

[Orlando]

+ Rebased on the latest version of D99651 (which has been approved now, so should be its final form)

debuginfo-tests/dexter/dex/command/ParseCommand.py

281–285

They are necessary because the expression is spread over multiple lines.

debuginfo-tests/dexter/dex/command/ParseCommand.py

if label.eval() in labels:
         err.src = label.raw_text
         # Don't both trying to point to it since we're only printing the raw
         # command, which isn't much text.
         err.caret = ''
         raise err
     labels[label.eval()] = label.get_line()
 
 
-def _find_all_commands_in_file(path, file_lines, valid_commands):
+def _find_all_commands_in_file(path, file_lines, valid_commands, source_root_dir):
     labels = {} # dict of {name: line}.
     cmd_path = path
     declared_files = set()
     commands = defaultdict(dict)
     paren_balance = 0
     region_start = TextPoint(0, 0)
 
     for region_start.line in range(len(file_lines)):
for region_start.line in range(len(file_lines)):
                 err_point.char += len(command_name)
                 raise format_parse_err(str(e), path, file_lines, err_point)
             else:
                 if type(command) is DexLabel:
                     add_line_label(labels, command, path, cmd_point.get_lineno())
                 elif type(command) is DexDeclareFile:
                     cmd_path = command.declared_file
                     if not os.path.isabs(cmd_path):
-                        source_dir = os.path.dirname(path)
+                        source_dir = (source_root_dir if source_root_dir else
+                                      os.path.dirname(path))
                         cmd_path = os.path.join(source_dir, cmd_path)
                     # TODO: keep stored paths as PurePaths for 'longer'.
                     cmd_path = str(PurePath(cmd_path))
                     declared_files.add(cmd_path)
                 assert (path, cmd_point) not in commands[command_name], (
                     command_name, commands[command_name])
                 commands[command_name][path, cmd_point] = command
 
     if paren_balance != 0:
         # This err should always point to the end of the command name.
         err_point = copy(cmd_point)
         err_point.char += len(command_name)
         msg = "Unbalanced parenthesis starting here"
         raise format_parse_err(msg, path, file_lines, err_point)
     return dict(commands), declared_files
 
-def _find_all_commands(test_files):
+def _find_all_commands(test_files, source_root_dir):
     commands = defaultdict(dict)
     valid_commands = _get_valid_commands()
     new_source_files = set()
     for test_file in test_files:
         with open(test_file) as fp:
             lines = fp.readlines()
-        file_commands, declared_files = _find_all_commands_in_file(test_file,
-                                                  lines, valid_commands)
+        file_commands, declared_files = _find_all_commands_in_file(
+            test_file, lines, valid_commands, source_root_dir)
         for command_name in file_commands:
             commands[command_name].update(file_commands[command_name])
         new_source_files |= declared_files
 
     return dict(commands), new_source_files
 
-def get_command_infos(test_files):
+def get_command_infos(test_files, source_root_dir):
   with Timer('parsing commands'):
       try:
-          commands, new_source_files = _find_all_commands(test_files)
+          commands, new_source_files = _find_all_commands(test_files, source_root_dir)
           command_infos = OrderedDict()
           for command_type in commands:
               for command in commands[command_type].values():
                   if command_type not in command_infos:
                       command_infos[command_type] = []
                   command_infos[command_type].append(command)
           return OrderedDict(command_infos), new_source_files
       except CommandParseError as e:
class TestParseCommand(unittest.TestCase):
 
 
     def _find_all_commands_in_lines(self, lines):
         """Use DExTer parsing methods to find all the mock commands in lines.
 
         Returns:
             { cmd_name: { (path, line): command_obj } }
         """
-        cmds, declared_files = _find_all_commands_in_file(__file__, lines, self.valid_commands)
+        cmds, declared_files = _find_all_commands_in_file(__file__, lines, self.valid_commands, None)
         return cmds
 
 
     def _find_all_mock_values_in_lines(self, lines):
         """Use DExTer parsing methods to find all mock command values in lines.
 
         Returns:
             values (list(str)): MockCmd values found in lines.

debuginfo-tests/dexter/dex/debugger/DebuggerBase.py

class DebuggerBase(object, metaclass=abc.ABCMeta):
     def frames_below_main(self):
         pass
 
     @abc.abstractmethod
     def evaluate_expression(self, expression, frame_idx=0) -> ValueIR:
         pass
 
     def _external_to_debug_path(self, path):
+        if not self.options.debugger_use_relative_paths:
+            return path
         root_dir = self.options.source_root_dir
         if not root_dir or not path:
             return path
         assert path.startswith(root_dir)
         return path[len(root_dir):].lstrip(os.path.sep)
 
     def _debug_to_external_path(self, path):
+        if not self.options.debugger_use_relative_paths:
+            return path
         if not path or not self.options.source_root_dir:
             return path
         for file in self.options.source_files:
             if path.endswith(self._external_to_debug_path(file)):
                 return file
         return path
 
 class TestDebuggerBase(unittest.TestCase):
def _new_step(self, paths):
                 loc=LocIR(path=path, lineno=0, column=0)) for path in paths
         ]
         return StepIR(step_index=0, stop_reason=None, frames=frames)
 
     def _step_paths(self, step):
         return [frame.loc.path for frame in step.frames]
 
     def test_add_breakpoint_no_source_root_dir(self):
+        self.options.debugger_use_relative_paths = True
         self.options.source_root_dir = ''
         self.dbg.add_breakpoint('/root/some_file', 12)
         self.assertEqual('/root/some_file', self.dbg.breakpoint_file)
 
     def test_add_breakpoint_with_source_root_dir(self):
+        self.options.debugger_use_relative_paths = True
         self.options.source_root_dir = '/my_root'
         self.dbg.add_breakpoint('/my_root/some_file', 12)
         self.assertEqual('some_file', self.dbg.breakpoint_file)
 
     def test_add_breakpoint_with_source_root_dir_slash_suffix(self):
+        self.options.debugger_use_relative_paths = True
         self.options.source_root_dir = '/my_root/'
         self.dbg.add_breakpoint('/my_root/some_file', 12)
         self.assertEqual('some_file', self.dbg.breakpoint_file)
 
     def test_get_step_info_no_source_root_dir(self):
+        self.options.debugger_use_relative_paths = True
         self.dbg.step_info = self._new_step(['/root/some_file'])
         self.assertEqual(['/root/some_file'],
             self._step_paths(self.dbg.get_step_info([], 0)))
 
     def test_get_step_info_no_frames(self):
+        self.options.debugger_use_relative_paths = True
         self.options.source_root_dir = '/my_root'
         self.dbg.step_info = self._new_step([])
         self.assertEqual([],
             self._step_paths(self.dbg.get_step_info([], 0)))
 
     def test_get_step_info(self):
+        self.options.debugger_use_relative_paths = True
         self.options.source_root_dir = '/my_root'
         self.options.source_files = ['/my_root/some_file']
         self.dbg.step_info = self._new_step(
             [None, '/other/file', '/dbg/some_file'])
         self.assertEqual([None, '/other/file', '/my_root/some_file'],
             self._step_paths(self.dbg.get_step_info([], 0)))

debuginfo-tests/dexter/dex/debugger/Debuggers.py

parser.add_argument(
         type=str,
         metavar='<architecture>',
         default=None,
         display_default=defaults.arch,
         help='target architecture')
     defaults.source_root_dir = ''
     parser.add_argument(
         '--source-root-dir',
+        type=str,
+        metavar='<directory>',
         default=None,
-        help='prefix path to ignore when matching debug info and source files.')
-
+        help='source root directory')
+    parser.add_argument(
+        '--debugger-use-relative-paths',
+        action='store_true',
+        default=False,
+        help='pass the debugger paths relative to --source-root-dir')
 
 def handle_debugger_tool_base_options(context, defaults):  # noqa
     options = context.options
 
     if options.lldb_executable is None:
         options.lldb_executable = defaults.lldb_executable
     else:
         if getattr(options, 'debugger', 'lldb') != 'lldb':
else:
             _warn_meaningless_option(context, '--arch')
 
     if options.show_debugger is None:
         options.show_debugger = defaults.show_debugger
     else:
         if options.debugger == 'lldb':
             _warn_meaningless_option(context, '--show-debugger')
 
+    if options.source_root_dir != None:
+        if not os.path.isabs(options.source_root_dir):
+            raise ToolArgumentError(f'<d>--source-root-dir: expected absolute path, got</> <r>"{options.source_root_dir}"</>')
+        if not os.path.isdir(options.source_root_dir):
+            raise ToolArgumentError(f'<d>--source-root-dir: could not find directory</> <r>"{options.source_root_dir}"</>')
+
+    if options.debugger_use_relative_paths:
+        if not options.source_root_dir:
+            raise ToolArgumentError(f'<d>--debugger-relative-paths</> <r>requires --source-root-dir</>')
 
 def run_debugger_subprocess(debugger_controller, working_dir_path):
     with NamedTemporaryFile(
             dir=working_dir_path, delete=False, mode='wb') as fp:
         pickle.dump(debugger_controller, fp, protocol=pickle.HIGHEST_PROTOCOL)
         controller_path = fp.name
 
     dexter_py = os.path.basename(sys.argv[0])

debuginfo-tests/dexter/dex/tools/clang_opt_bisect/Tool.py

def handle_options(self, defaults):
                         "supported " % options.builder)
         super(Tool, self).handle_options(defaults)
 
     def _init_debugger_controller(self):
         step_collection = DextIR(
             executable_path=self.context.options.executable,
             source_paths=self.context.options.source_files,
             dexter_version=self.context.version)
+
         step_collection.commands, new_source_files = get_command_infos(
-            self.context.options.test_files)
+            self.context.options.source_files, self.context.options.source_root_dir)
         self.context.options.source_files.extend(list(new_source_files))
+
         debugger_controller = DefaultController(self.context, step_collection)
         return debugger_controller
 
     def _run_test(self, test_name):  # noqa
         options = self.context.options
 
         per_pass_score = []
         current_bisect_pass_summary = defaultdict(list)

debuginfo-tests/dexter/dex/tools/test/Tool.py

class Tool(TestToolBase):
 
     def _init_debugger_controller(self):
         step_collection = DextIR(
             executable_path=self.context.options.executable,
             source_paths=self.context.options.source_files,
             dexter_version=self.context.version)
 
         step_collection.commands, new_source_files = get_command_infos(
-            self.context.options.test_files)
+            self.context.options.test_files, self.context.options.source_root_dir)
 
         self.context.options.source_files.extend(list(new_source_files))
 
         if 'DexLimitSteps' in step_collection.commands:
             debugger_controller = ConditionalController(self.context, step_collection)
         else:
             debugger_controller = DefaultController(self.context, step_collection)
 

debuginfo-tests/dexter/feature_tests/commands/perfect/dex_declare_file/precompiled_binary_different_dir/dex_commands/source_root_dir.dex

+## Purpose:
+##    Check that \DexDeclareFile's file declaration can be made relative to the
+##    --source-root-dir path.
+
+# REQUIRES: lldb
+# UNSUPPORTED: system-darwin
+
+# RUN: %clang %S/../source/test.cpp -O0 -g -o %t
+# RUN: %dexter_regression_test --binary %t \
+# RUN:   --source-root-dir="%S/../source" -- %s | FileCheck %s
+# RUN: rm %t
+
+# CHECK: source_root_dir.dex: (1.0000)
+
+## ../source/test.cpp
+## 1. int main() {
+## 2.   int result = 0;
+## 3.   return result;
+## 4. }
+
+## test.cpp is found in ../source, which we set as the source-root-dir.
+DexDeclareFile('test.cpp')
+DexExpectWatchValue('result', 0, on_line=3)

debuginfo-tests/dexter/feature_tests/subtools/test/source-root-dir.cpp

 // REQUIRES: lldb
 // UNSUPPORTED: system-windows
 //
 // RUN: %dexter --fail-lt 1.0 -w \
 // RUN:     --builder 'clang' --debugger 'lldb' \
 // RUN:     --cflags "-O0 -glldb -fdebug-prefix-map=%S=/changed" \
-// RUN:     --source-root-dir=%S -- %s
+// RUN:     --source-root-dir=%S --debugger-use-relative-paths -- %s
 
 #include <stdio.h>
 int main() {
   int x = 42;
   printf("hello world: %d\n", x); // DexLabel('check')
 }
 
 // DexExpectWatchValue('x', 42, on_line=ref('check'))