Index: llvm/utils/lit/lit/LitConfig.py =================================================================== --- llvm/utils/lit/lit/LitConfig.py +++ llvm/utils/lit/lit/LitConfig.py @@ -37,6 +37,7 @@ maxIndividualTestTime=0, parallelism_groups={}, per_test_coverage=False, + pdb=False, ): # The name of the test runner. self.progname = progname @@ -48,6 +49,7 @@ self.valgrindUserArgs = list(valgrindArgs) self.noExecute = noExecute self.debug = debug + self.pdb = pdb self.isWindows = bool(isWindows) self.order = order self.params = dict(params) Index: llvm/utils/lit/lit/TestRunner.py =================================================================== --- llvm/utils/lit/lit/TestRunner.py +++ llvm/utils/lit/lit/TestRunner.py @@ -1019,7 +1019,6 @@ return exitCode - def formatOutput(title, data, limit=None): if not data.strip(): return "" @@ -1478,6 +1477,21 @@ return f"at line {self.start_line_number}" return f"from line {self.start_line_number} to {self.end_line_number}" + def asPythonDirective(self): + """ + Return a PythonDirective equivalent to this directive, or just return + this directive if it's already a PythonDirective. + """ + assert False, "expected method to be called on derived class" + + def continuePythonDirective(self, pythonDir): + """ + Append lines to pythonDir that are equivalent to this directive. Each + implementation of this function should probably call + pythonDir.doNotRequireIndentation() as its first step. + """ + assert False, "expected method to be called on derived class" + class CommandDirective(ExpandableScriptDirective): """ @@ -1504,6 +1518,33 @@ # so '\' is never hidden here. return self.command[-1] == "\\" + def _makePythonLitRun(self): + match = re.match(kPdbgRegex, self.command) + cmd = match.group(2) if match else self.command + return f"lit.run({repr(cmd)})" + + def _addBlankLinesToPythonDirective(self, pythonDir): + for i in range(self.start_line_number + 1, self.end_line_number + 1): + res = pythonDir.add_continuation(i, pythonDir.keyword, "") + assert res, "expected success adding blank line as PYTHON " \ + "continuation" + + def asPythonDirective(self): + pythonDir = PythonDirective(self.start_line_number, + self.start_line_number, + PythonDirective.getDefaultKeyword(), + self._makePythonLitRun()) + self._addBlankLinesToPythonDirective(pythonDir) + return pythonDir + + def continuePythonDirective(self, pythonDir): + pythonDir.doNotRequireIndentation() + res = pythonDir.add_continuation(self.start_line_number, + pythonDir.keyword, + self._makePythonLitRun()) + assert res, "expected success adding RUN line as PYTHON continuation" + self._addBlankLinesToPythonDirective(pythonDir) + class SubstDirective(ExpandableScriptDirective): """ @@ -1583,49 +1624,80 @@ f"alphanumeric characters, hyphens, underscores, and colons" ) - def adjust_substitutions(self, substitutions): - """ - Modify the specified substitution list as specified by this directive. - """ - assert ( - not self.needs_continuation() - ), "expected directive continuations to be parsed before applying" - value_repl = self.value.replace("\\", "\\\\") - existing = [i for i, subst in enumerate(substitutions) if self.name in subst[0]] + def _makePythonLitCall(self): + name = repr(self.name) + value = repr(self.value) + func = "define" if self.new_subst else "redefine" + return f"lit.{func}({name}, {value})" + + def _addBlankLinesToPythonDirective(self, pythonDir): + for i in range(self.start_line_number + 1, self.end_line_number + 1): + res = pythonDir.add_continuation(i, pythonDir.keyword, "") + assert res, "expected success adding blank line as PYTHON " \ + "continuation" + + def asPythonDirective(self): + pythonDir = PythonDirective(self.start_line_number, + self.start_line_number, + PythonDirective.getDefaultKeyword(), + self._makePythonLitCall()) + self._addBlankLinesToPythonDirective(pythonDir) + return pythonDir + + def continuePythonDirective(self, pythonDir): + pythonDir.doNotRequireIndentation() + res = pythonDir.add_continuation(self.start_line_number, + pythonDir.keyword, + self._makePythonLitCall()) + assert res, "expected success adding DEFINE/REDEFINE line as PYTHON " \ + "continuation" + self._addBlankLinesToPythonDirective(pythonDir) + + @staticmethod + def applySubstitution(substitutions, what, newSubst, name, value): + value_repl = value.replace("\\", "\\\\") + existing = [i for i, subst in enumerate(substitutions) if name in subst[0]] existing_res = "".join( "\nExisting pattern: " + substitutions[i][0] for i in existing ) - if self.new_subst: + if newSubst: if existing: raise ValueError( - f"Substitution whose pattern contains '{self.name}' is " - f"already defined before '{self.keyword}' directive " - f"{self.get_location()}" + f"Substitution whose pattern contains '{name}' is already " + f"defined before {what}" f"{existing_res}" ) - substitutions.insert(0, (self.name, value_repl)) + substitutions.insert(0, (name, value_repl)) return if len(existing) > 1: raise ValueError( - f"Multiple substitutions whose patterns contain '{self.name}' " - f"are defined before '{self.keyword}' directive " - f"{self.get_location()}" + f"Multiple substitutions whose patterns contain '{name}' are " + f"defined before {what}" f"{existing_res}" ) if not existing: raise ValueError( - f"No substitution for '{self.name}' is defined before " - f"'{self.keyword}' directive {self.get_location()}" + f"No substitution for '{name}' is defined before {what}" ) - if substitutions[existing[0]][0] != self.name: + if substitutions[existing[0]][0] != name: raise ValueError( - f"Existing substitution whose pattern contains '{self.name}' " - f"does not have the pattern specified by '{self.keyword}' " - f"directive {self.get_location()}\n" - f"Expected pattern: {self.name}" + f"Existing substitution whose pattern contains '{name}' does " + f"not have the pattern specified by {what}\n" + f"Expected pattern: {name}" f"{existing_res}" ) - substitutions[existing[0]] = (self.name, value_repl) + substitutions[existing[0]] = (name, value_repl) + + def apply(self, substitutions): + """ + Modify the specified substitution list as specified by this directive. + """ + assert ( + not self.needs_continuation() + ), "expected directive continuations to be parsed before applying" + what = f"'{self.keyword}' directive {self.get_location()}" + self.applySubstitution(substitutions, what, self.new_subst, self.name, + self.value) class PythonDirective(ExpandableScriptDirective): @@ -1639,6 +1711,10 @@ continuation lines. """ + @staticmethod + def getDefaultKeyword(): + return "PYTHON:" + def __init__(self, start_line_number, end_line_number, keyword, line): super().__init__(start_line_number, end_line_number, keyword) # Add blank lines so python diagnostics produce correct line numbers. @@ -1676,8 +1752,31 @@ # compiled separately. return False + def doNotRequireIndentation(self): + """ + Do not require indentation on future continuation lines. This is useful + when merging PYTHON blocks such that, in each, the indentation from the + first line was already verified as present and was already stripped from + subsequent lines. Normal python indentation is still required by the + python interprerter. + """ + self.indent = "" + + def asPythonDirective(self): + return self + + def continuePythonDirective(self, pythonDir): + pythonDir.doNotRequireIndentation() + for i, line in enumerate(self.body.splitlines()): + lineNumber = i + 1 + if lineNumber < self.start_line_number: + assert line == "", f"expected blank line before start line" + continue + res = pythonDir.add_continuation(lineNumber, self.keyword, line) + assert res, "expected success merging PYTHON directives" + @staticmethod - def executePython(what, filename, pythonCode, python_dict, log): + def executePython(what, filename, pythonCode, python_dict, log, usePdb): """ Execute python code passed as an argument. """ @@ -1690,15 +1789,27 @@ tb = e.__traceback__.tb_next traceText = "".join(traceback.format_exception(None, e, tb)) raise ScriptFatal(f"error compiling {what}:\n{traceText}") - log(f"# executed {what}\n") - outIO = StringIO() - errIO = StringIO() + if usePdb: + print(f"# started executing {what}") + else: + log(f"# executed {what}\n") + outIO = StringIO() + errIO = StringIO() traceText = "" try: - with contextlib.redirect_stdout(outIO), contextlib.redirect_stderr( - errIO - ): - exec(c, python_dict) + if usePdb: + import pdb, time + pdb.run(c, python_dict) + # If the user typed "quit" (as opposed to "continue") at the pdb + # prompt, Ctrl-C can now successfully terminate lit. However, + # give the user a little time to hit Ctrl-C before the above + # pdb.run is reached again and pdb starts intercepting Ctrl-C + # again. + time.sleep(1) + else: + with contextlib.redirect_stdout(outIO), \ + contextlib.redirect_stderr(errIO): + exec(c, python_dict) except ScriptFail: raise except Exception as e: @@ -1706,14 +1817,24 @@ # user with lit internals is just a distraction. tb = e.__traceback__.tb_next traceText = "".join(traceback.format_exception(None, e, tb)) + import bdb + if isinstance(e, bdb.BdbQuit): + traceText += "\nNote: It looks like you are trying to use a " \ + "python debugger, like pdb.\n" \ + "Try passing --pdb to lit, perhaps via the " \ + "LIT_OPTS environment variable.\n" raise ScriptFail(1, None) finally: - outText = outIO.getvalue() - errText = errIO.getvalue() + traceText - log(formatOutput(f"stdout from {what}", outText)) - log(formatOutput(f"stderr from {what}", errText)) + if usePdb: + print(traceText, end="") + print(f"# finished executing {what}") + else: + outText = outIO.getvalue() + errText = errIO.getvalue() + traceText + log(formatOutput(f"stdout from {what}", outText)) + log(formatOutput(f"stderr from {what}", errText)) - def execute(self, filename, python_dict, log): + def execute(self, filename, python_dict, log, usePdb): """ Execute the python code in this directive. """ @@ -1723,6 +1844,7 @@ self.body, python_dict, log, + usePdb ) @@ -1875,7 +1997,7 @@ output = [] for directive in script: if isinstance(directive, SubstDirective): - directive.adjust_substitutions(substitutions) + directive.apply(substitutions) else: if isinstance(directive, CommandDirective): line = directive.command @@ -2291,6 +2413,7 @@ useExternalSh, script, tmpBase, + substitutions=[], substitutionApplier=lambda x: x, ): # scriptPart has the same constraints as script for _runShTest except @@ -2323,9 +2446,6 @@ # directives. Symbols only required for lit iternals should be declared # outside it. - # TODO: In the future, extend the 'lit' API to be able to *write* - # substitutions? - @staticmethod def has(feature): """ @@ -2342,6 +2462,31 @@ """ return substitutionApplier([text])[0] + @staticmethod + def define(name, value): + """ + Define new substitution as if it appeared in a 'DEFINE:' directive. + """ + try: + what = "lit.define call" + SubstDirective.applySubstitution(substitutions, what, True, + name, value) + except ValueError as e: + raise ScriptFatal(str(e)) from None + + @staticmethod + def redefine(name, value): + """ + Redefine existing substitution as if it appeared in a 'REDEFINE:' + directive. + """ + try: + what = "lit.redefine call" + SubstDirective.applySubstitution(substitutions, what, False, + name, value) + except ValueError as e: + raise ScriptFatal(str(e)) from None + @staticmethod def run(cmd): """ @@ -2391,7 +2536,8 @@ with open(prologue) as f: what = f"config.prologue='{prologue}'" PythonDirective.executePython( - what, prologue, f.read(), python_dict, addStdout + what, prologue, f.read(), python_dict, addStdout, + litConfig.pdb ) # Execute the script. scriptRemaining = script @@ -2401,7 +2547,8 @@ if not isinstance(pythonDir, PythonDirective): break pythonDir.execute( - test.getSourcePath(), python_dict, addStdout + test.getSourcePath(), python_dict, addStdout, + litConfig.pdb ) else: scriptRemaining = [] @@ -2438,7 +2585,11 @@ execdir = os.path.dirname(test.getExecPath()) attempts = test.allowed_retries + 1 for i in range(attempts): + if litConfig.pdb: + print(f"{'*' * 20} STARTED DEBUGGING '{test.getFullName()}' {'*' * 20}") res = runOnce(execdir) + if litConfig.pdb: + print(f"{'*' * 20} FINISHED DEBUGGING '{test.getFullName()}' {'*' * 20}") out, err, exitCode, timeoutInfo, status = res if status != Test.FAIL: break @@ -2496,11 +2647,31 @@ recursion_limit=test.config.recursiveExpansionLimit, ) + # If --pdb, convert RUN, DEFINE, and REDEFINE directives to lit object + # calls in PYTHON directives so they can be tracked by pdb. We try to put + # them all in the same PYTHON block so they're within the same pdb.run + # call. + if litConfig.pdb: + oldScript = script + script = [] + for cmd in oldScript: + if isinstance(cmd, str): + # We don't attempt to support debugging of preamble commands. + # They have no source file location. + script.append(cmd) + else: + assert isinstance(cmd, ExpandableScriptDirective) + if not script or not isinstance(script[-1], PythonDirective): + script.append(cmd.asPythonDirective()) + else: + cmd.continuePythonDirective(script[-1]) + return _runShTest( test, litConfig, useExternalSh, script, tmpBase, + substitutions=substitutions, substitutionApplier=substitutionApplier, ) Index: llvm/utils/lit/lit/cl_arguments.py =================================================================== --- llvm/utils/lit/lit/cl_arguments.py +++ llvm/utils/lit/lit/cl_arguments.py @@ -293,6 +293,11 @@ help="Show all features used in the test suite (in XFAIL, UNSUPPORTED and REQUIRES) and exit", action="store_true", ) + debug_group.add_argument( + "--pdb", + help="Enable debugging lit tests with python's standard debugger, pdb", + action="store_true", + ) # LIT is special: environment variables override command line arguments. env_args = shlex.split(os.environ.get("LIT_OPTS", "")) Index: llvm/utils/lit/lit/display.py =================================================================== --- llvm/utils/lit/lit/display.py +++ llvm/utils/lit/lit/display.py @@ -10,7 +10,7 @@ header = "-- Testing: %d%s tests, %d workers --" % (num_tests, of_total, workers) progress_bar = None - if opts.succinct and opts.useProgressBar: + if opts.succinct and opts.useProgressBar and not opts.pdb: import lit.ProgressBar try: Index: llvm/utils/lit/lit/main.py =================================================================== --- llvm/utils/lit/lit/main.py +++ llvm/utils/lit/lit/main.py @@ -36,6 +36,7 @@ valgrindArgs=opts.valgrindArgs, noExecute=opts.noExecute, debug=opts.debug, + pdb=opts.pdb, isWindows=is_windows, order=opts.order, params=params, @@ -244,10 +245,26 @@ def run_tests(tests, lit_config, opts, discovered_tests): workers = min(len(tests), opts.workers) + timeout = opts.timeout + if lit_config.pdb: + if workers != 1: + lit_config.warning( + "Ignoring requested parallelism because debugging with pdb was " + "requested" + ) + # workers = 1 still creates a subprocess, which prevents pdb from + # successfully debugging tests. + workers = 0 + if timeout: + lit_config.warning( + "Ignoring timeout because debugging with pdb was requested" + ) + timeout = None + display = lit.display.create_display(opts, tests, discovered_tests, workers) run = lit.run.Run( - tests, lit_config, workers, display.update, opts.max_failures, opts.timeout + tests, lit_config, workers, display.update, opts.max_failures, timeout ) display.print_header() Index: llvm/utils/lit/lit/run.py =================================================================== --- llvm/utils/lit/lit/run.py +++ llvm/utils/lit/lit/run.py @@ -27,7 +27,7 @@ self.progress_callback = progress_callback self.max_failures = max_failures self.timeout = timeout - assert workers > 0 + assert workers >= 0 def execute(self): """ @@ -41,6 +41,9 @@ If timeout is non-None, it should be a time in seconds after which to stop executing tests. + If workers is 0, tests will be executed sequentially in the same + process, and timeout must be None. + Returns the elapsed testing time. Upon completion, each test in the run will have its result @@ -49,6 +52,9 @@ """ self.failures = 0 + assert self.workers > 0 or not self.timeout, \ + "expected at least one worker to be able to apply timeout" + # Larger timeouts (one year, positive infinity) don't work on Windows. one_week = 7 * 24 * 60 * 60 # days * hours * minutes * seconds timeout = self.timeout or one_week @@ -63,6 +69,13 @@ test.setResult(skipped) def _execute(self, deadline): + if self.workers == 0: + for test in self.tests: + result = lit.worker._execute(test, self.lit_config) + test.setResult(result) + self.progress_callback(test) + return + self._increase_process_limit() semaphores = { Index: llvm/utils/lit/lit/worker.py =================================================================== --- llvm/utils/lit/lit/worker.py +++ llvm/utils/lit/lit/worker.py @@ -61,7 +61,7 @@ return _parallelism_semaphores.get(pg, NopSemaphore()) -# Do not inline! Directly used by LitTestCase.py +# Do not inline! Directly used by LitTestCase.py and run.py def _execute(test, lit_config): start = time.time() result = _execute_test_handle_errors(test, lit_config) @@ -75,7 +75,12 @@ try: result = test.config.test_format.execute(test, lit_config) return _adapt_result(result) - except: + except BaseException as e: + # lit_config.pdb disables multiprocessing and thus graceful handling of + # Ctrl-C. Let KeyboardInterrupt exceptions propagate from here so the + # user can easily terminate lit. + if lit_config.pdb and isinstance(e, KeyboardInterrupt): + raise if lit_config.debug: raise output = "Exception during script execution:\n"