[DEXTER] Add support for windows in regression test suite.

[TWeaver]

Summary

Hello fellow Dexter Contributors,

This patch addresses the issue of the regression suite not running on windows hardware. It changes the following things:

• add new dexter regression suite command to lit.cfg.py that makes use of the clang-cl_vs2015 and dbgend builder and debuggers.
• sprinkle the new regression suite command through the feature and tool tests that require them.
• mark certain problem tests on windows
  • There's a couple of tests that fail (or pass) in unexpected ways on Windows.

Problem tests are both the penalty and perfect expect_watch_type.cpp tests. Type information reporting parity is not possible a this time in dexter due to the nature of how different debuggers report type information back to their users.

Happy Reviewing

[Orlando]

LGTM

[davide]

This broke the lldb bot (which also runs the debuginfo tests).

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/13349/

Can you fix it, or tell me how to fix it, or revert?

Thanks,

D

[davide]

Reverted in:

commit 470e82cd06062a3fc3c142782b4d46d711b7c19c (HEAD -> master, origin/master, origin/HEAD)
Author: Davide Italiano <[e-mail]>
Date:   Fri Mar 27 13:12:00 2020 -0700

    Revert "[Dexter] Add support for Windows to regression test suite."
    
    This reverts commit 89025da9f676aebff7daf055824d6fd102a70c34 as
    it breaks the lldb macOS bot.

[rnk]

I synced to a revision where this was landed, ran the tests on Windows, and got these results:

-- Testing: 54 tests, 8 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

Testing Time: 182.54s
  Expected Passes    : 28
  Expected Failures  : 3
  Unsupported Tests  : 23

That's pretty good! Thanks for polishing this up.

I noticed there were a few tests that seemed to hang. The process driving the debugger consumed spun consuming CPU with no memory growth. I think this was one of the XFAILed expect_watch_something.cpp tests, from checking it in process hacker. As a next step, I think it would be good to look into these slow tests. Previously the cdb tests ran in a nominal amount of time, < 20s.

[TWeaver]

I've actually mailed these via the mailing list by accident, here's a copy of what I mailed:

Hi Davide,

sorry about the failures, I'l get right on it, thanks for sharing.

Sorry I missed your comments too.
Tom W

---

Hi Reid,

We have an internal ticket for improving the performance of the regression tests, it's not clear what's causing the slow down at this point in time but we intend to fix it in the near future.

Thanks again,
Tom W

[TWeaver]

Hi Davide,

I've marked tests that were 'REQUIRES: system-linux' before this change to 'UNSUPPORTED: system-darwin',

The failing tests should no longer cause any hangs up on you CI. If you run into any further issues please let me know,

https://reviews.llvm.org/rGb6d2212f5276bcf0e5ef6b5dc0c985b72664ae34

Thanks
Tom W

[TWeaver]

I've also added an internal ticket to add darwin support to the dexter regression suite... I don't think there'll be much appetite in-house for it but if there's a clear need to add it in the future then of course we'll consider putting in the work.

[TWeaver]

@rnk ,

The cdb tests taking longer may be a result of the dexter regression suite running along side them. SFAIK that's not intended behaviour.

The regression suite doesn't run as quick as we'd like right now but it seems as though those tests are running as well as the CDB dexter tests.

To avoid running the dexter regression suite it may be best to point lit at the test directory that only contains the CDB regression suite you're interested in.
At the time of writing I believe this is the dexter-tests directory.

debuginfo-tests/dexter/feature_tests/commands/penalty/expect_program_state.cpp

 // Purpose:
 //      Check that \DexExpectProgramState correctly applies a penalty when
 //      an expected program state is never found.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: not %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -glldb" -- %s \
-// RUN:     | FileCheck %s
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_program_state.cpp:
 
 int GCD(int lhs, int rhs)
 {
     if (rhs == 0)   // DexLabel('check')
         return lhs;
     return GCD(rhs, lhs % rhs);
 }

debuginfo-tests/dexter/feature_tests/commands/penalty/expect_step_kinds.cpp

 // Purpose:
 //      Check that \DexExpectStepKind correctly applies a penalty when
 //      unexpected step kinds are encountered.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: not %dexter_base test --fail-lt 1.0 -w  \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_step_kinds.cpp:
 
 int abs(int i){
     return i < 0? i * -1: i;
 }
 
 int main()
 {

debuginfo-tests/dexter/feature_tests/commands/penalty/expect_step_order.cpp

 // Purpose:
 //      Check that \DexExpectStepOrder correctly applies a penalty for steps
 //      found out of expected order.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: not %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_step_order.cpp:
 
 int main()
 {
     volatile int x = 1; // DexExpectStepOrder(3)
     volatile int y = 1; // DexExpectStepOrder(1)
     volatile int z = 1; // DexExpectStepOrder(2)
     return 0;
 }

debuginfo-tests/dexter/feature_tests/commands/penalty/expect_watch_type.cpp

 // Purpose:
 //      Check that \DexExpectWatchType applies penalties when expected
 //      types are not found and unexpected types are.
 //
-// REQUIRES: system-linux, lldb
+// NOTE: This test passes but not in the expected way on Windows.
+// TODO: Reduce this test's coverage and be more specific about
+// expected behaviour.
 //
-// RUN: not %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_watch_type.cpp:
 
 template<class T>
 class Doubled {
 public:
   Doubled(const T & to_double)
     : m_member(to_double * 2) {}
 

debuginfo-tests/dexter/feature_tests/commands/penalty/expect_watch_value.cpp

 // Purpose:
 //      Check that \DexExpectWatchValue correctly applies a penalty when
 //      expected values are not found.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: not %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_watch_value.cpp:
 
 int main()
 {
     for (int i = 0; i < 3; ++i)
         int a = i; // DexLabel('loop')
     return 0;  // DexLabel('ret')
 }
 
 // DexExpectWatchValue('i', '0', '1', '2', on_line='loop')
 // DexExpectWatchValue('i', '3', on_line='ret')
 // ---------------------^ out of scope

debuginfo-tests/dexter/feature_tests/commands/penalty/unreachable.cpp

 // Purpose:
 //      Check that \DexUnreachable correctly applies a penalty if the command
 //      line is stepped on.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: not %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
 // CHECK: unreachable.cpp:
 
 int
 main()
 {
   return 1;  // DexUnreachable()
 }

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_program_state.cpp

 // Purpose:
 //      Check that \DexExpectWatchValue applies no penalties when expected
 //      program states are found.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -glldb" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_program_state.cpp:
 
 int GCD(int lhs, int rhs)
 {
     if (rhs == 0)
         return lhs; // DexLabel('check')
     return GCD(rhs, lhs % rhs);
 }

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_step_kind/direction.cpp

 // Purpose:
 //      Check that \DexExpectStepKind correctly counts 'VERTICAL_BACKWARD' steps
 //      for a trivial test. Expect one 'VERTICAL_BACKWARD' for every step onto
 //      a lesser source line number in the same function. Expect one
 //      'VERTICAL_FORWARD' for every step onto a greater source line number in
 //      the same function.
 //
-// REQUIRES: system-linux, lldb
+// TODO: The dbgeng debugger does not support column step reporting at present.
+// XFAIL: system-windows
 //
-// RUN: %dexter_base test --fail-lt 1.0 -w  \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: direction.cpp:
 
 int func(int i) {
     return i; // step 7, 9, 11
 }
 
 int main()
 {

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_step_kind/func.cpp

 // Purpose:
 //      Check that \DexExpectStepKind correctly counts 'FUNC' steps for a
 //      trivial test. Expect one 'FUNC' per call to a function which is defined
 //      in one of the source files in the test directory.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w  \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: func.cpp:
 
 int func(int i) {
     return i;
 }
 
 int main()
 {
     func(0);
     func(1);
     return 0;
 }
 
 // main, func, func
 // DexExpectStepKind('FUNC', 3)

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_step_kind/func_external.cpp

 // Purpose:
 //      Check that \DexExpectStepKind correctly counts 'FUNC_EXTERNAL' steps
 //      for a trivial test. Expect one 'FUNC_EXTERNAL' per external call.
 //
-// REQUIRES: system-linux, lldb
-// XFAIL: system-linux
-// This fails right now on my linux machine, needs examining as to why.
+// XFAIL:*
+// This fails right now on my linux and windows machine, needs examining as to
+// why.
 //
-// RUN: %dexter --fail-lt 1.0 -w  \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: func_external.cpp:
 
 #include <cstdlib>
 
 int func(int i){
     return abs(i);
 }
 
 int main()
 {
     func(0);
     func(1);
     return 0;
 }
 
 // DexExpectStepKind('FUNC_EXTERNAL', 2)

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_step_kind/recursive.cpp

 // Purpose:
 //      Check that \DexExpectStepKind correctly handles recursive calls.
 //      Specifically, ensure recursive calls count towards 'FUNC' and not
 //      'VERTICAL_BACKWARD'.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w  \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: recursive.cpp:
 
 int func(int i) {
     if (i > 1)
         return i + func(i - 1);
     return i;
 }
 
 int main()
 {
     return func(3);
 }
 
 // main, func, func, func
 // DexExpectStepKind('FUNC', 4)
 // DexExpectStepKind('VERTICAL_BACKWARD', 0)

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_step_kind/small_loop.cpp

 // Purpose:
 //      Check that \DexExpectStepKind correctly counts function calls in loops
 //      where the last source line in the loop is a call. Expect steps out
 //      of a function to a line before the call to count as 'VERTICAL_BACKWARD'.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w  \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: small_loop.cpp:
 
 int func(int i){
     return i;
 }
 
 int main()
 {
     for (int i = 0; i < 2; ++i) {
         func(i);
     }
     return 0;
 }
 
 // DexExpectStepKind('VERTICAL_BACKWARD', 2)

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_step_order.cpp

 // Purpose:
 //      Check that \DexExpectStepOrder applies no penalty when the expected
 //      order is found.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_step_order.cpp:
 
 int main()
 {
   volatile int x = 1; // DexExpectStepOrder(1)
   volatile int y = 1; // DexExpectStepOrder(2)
   volatile int z = 1; // DexExpectStepOrder(3)
   return 0;
 }

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_watch_type.cpp

 // Purpose:
 //      Check that \DexExpectWatchType applies no penalties when expected
 //      types are found.
 //
-// REQUIRES: system-linux, lldb
+// TODO: On Windows WITH dbgeng, This test takes a long time to run and doesn't evaluate type values
+// in the same manner as LLDB.
+// XFAIL: system-windows
 //
-// RUN: %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_watch_type.cpp:
 
 template<class T>
 class Doubled {
 public:
   Doubled(const T & to_double)
     : m_member(to_double * 2) {}
 

debuginfo-tests/dexter/feature_tests/commands/perfect/expect_watch_value.cpp

 // Purpose:
 //      Check that \DexExpectWatchValue applies no penalties when expected
 //      values are found.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: expect_watch_value.cpp:
 
 unsigned long Factorial(int n) {
     volatile unsigned long fac = 1; // DexLabel('entry')
 
     for (int i = 1; i <= n; ++i)
         fac *= i;                   // DexLabel('loop')
 

debuginfo-tests/dexter/feature_tests/commands/perfect/unreachable.cpp

 // Purpose:
 //    Check that \DexUnreachable has no effect if the command line is never
 //    stepped on.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w \
-// RUN:     --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" -- %s \
-// RUN:     | FileCheck %s
+// RUN: %dexter_regression_test -- %s | FileCheck %s
 // CHECK: unreachable.cpp:
 
 int main()
 {
   return 0;
   return 1; // DexUnreachable()
 }

debuginfo-tests/dexter/feature_tests/subtools/view.cpp

 // Purpose:
 //      Check the `view` subtool works with typical inputs.
 //
-// REQUIRES: system-linux, lldb
-//
-// RUN: %dexter_base test --fail-lt 1.0 -w \
-// RUN:    --builder 'clang' --debugger 'lldb' --cflags "-O0 -g" \
-// RUN:    --results %t -- %s
+// RUN: %dexter_regression_test --results %t -- %s
 //
 // RUN: %dexter_base view %t/view.cpp.dextIR | FileCheck %s
 // CHECK: ## BEGIN
 // CHECK: ## END
 //
 // # [TODO] This doesn't run if FileCheck fails!
 // RUN: rm -rf %t
 
 int main() {
     int a = 0;
     return 0; //DexLabel('ret')
 }
 // DexExpectWatchValue('a', '0', on_line='ret')

debuginfo-tests/lit.cfg.py

if lldb_path is not None:
   dexter_test_cmd += ' --lldb-executable "{}"'.format(lldb_path)
 tools.append(ToolSubst('%dexter', dexter_test_cmd))
 
 # For testing other bits of dexter that aren't under the "test" subcommand,
 # have a %dexter_base substitution.
 dexter_base_cmd = '"{}" "{}"'.format(config.python3_executable, dexter_path)
 tools.append(ToolSubst('%dexter_base', dexter_base_cmd))
 
+# Set up commands for DexTer regression tests.
+# Builder, debugger, optimisation level and several other flags differ
+# depending on whether we're running a unix like or windows os.
+if platform.system() == 'Windows': 
+  dexter_regression_test_builder = '--builder clang-cl_vs2015'
+  dexter_regression_test_debugger = '--debugger dbgeng'
+  dexter_regression_test_cflags = '--cflags "/Zi /Od"'
+  dexter_regression_test_ldflags = '--ldflags "/Zi"'
+else:
+  dexter_regression_test_builder = '--builder clang'
+  dexter_regression_test_debugger = "--debugger lldb"
+  dexter_regression_test_cflags = '--cflags "-O0 -glldb"'
+  dexter_regression_test_ldflags = ''
+
+# Typical command would take the form:
+# ./path_to_py/python.exe ./path_to_dex/dexter.py test --fail-lt 1.0 -w --builder clang --debugger lldb --cflags '-O0 -g'
+dexter_regression_test_command = ' '.join(
+  # "python3", "dexter.py", test, fail_mode, builder, debugger, cflags, ldflags
+  ["{}".format(config.python3_executable),
+  "{}".format(dexter_path),
+  'test',
+  '--fail-lt 1.0 -w',
+  dexter_regression_test_builder,
+  dexter_regression_test_debugger,
+  dexter_regression_test_cflags,
+  dexter_regression_test_ldflags])
+
+tools.append(ToolSubst('%dexter_regression_test', dexter_regression_test_command))
+
 tool_dirs = [config.llvm_tools_dir]
 
 llvm_config.add_tool_substitutions(tools, tool_dirs)
 
 lit.util.usePlatformSdkOnDarwin(config, lit_config)
 
 # available_features: REQUIRES/UNSUPPORTED lit commands look at this list.
 if platform.system() == 'Darwin':
     import subprocess
     xcode_lldb_vers = subprocess.check_output(['xcrun', 'lldb', '--version']).decode("utf-8")
     match = re.search('lldb-(\d+)', xcode_lldb_vers)
     if match:
         apple_lldb_vers = int(match.group(1))
         if apple_lldb_vers < 1000:
             config.available_features.add('apple-lldb-pre-1000')