[LLDB] On Windows, fix tests

[aganea]

Summary

This patch fixes a few issues seen when running ninja check-lldb in a Release build:

• Some binaries couldn't be found (such as lldb-vscode.exe), because .exe wasn't appended to the file name.
• Many tests used to fail since the OS error messages are not emitted in English, because of our installed French locale.
• Also, because of our codepage being Windows-1252, python failed to decode the messages.

After this patch, all tests pass with VS2017.
When building with VS2019 or Clang 10, the following tests still do not pass:

lldb-api :: python_api/lldbutil/iter/TestLLDBIterator.py
lldb-shell :: SymbolFile/PDB/enums-layout.test
lldb-shell :: SymbolFile/PDB/typedefs.test
lldb-unit :: tools/lldb-server/tests/./LLDBServerTests.exe/StandardStartupTest.TestStopReplyContainsThreadPcs

For the first three tests above, this has to do with the debug info not being emitted as expected. I'm not sure about the last one.

[labath]

Is there a way to force a program to use a specific locale, ignoring the system-wide setting (something like export LC_ALL=C on unix)? Hardcoding the error message for all locales is not a viable long term strategy... :/

[amccarth]

It's interesting that I haven't encountered some of these errors. There are five _other_ lldb tests that do fail for me. I have a fix in the works for some of those.

I agree with @labath that including error message patterns in various languages isn't scalable. Since dotest,py is starting up the processes under test, perhaps there's a way to force it to use a particular locale. Besides the language of system error messages, locales can change the format of numbers, dates, etc.

Unfortunately, I don't think it's as simple as an environment variable. I expect this is driven by the user's (or system default) locales settings as tracked by Windows. That's distinct from the C runtime concept of locales.

Maybe dotest.py could change its own Windows locale setting and the processes it spawns would inherit that. I don't know if that would work, but I don't see a good alternative.

lldb/unittests/Utility/StatusTest.cpp

80

Rather than an early return, perhaps the code should still be exercised, but the language-specific EXPECTs could be skipped.

[aganea]

In D88975#2317494, @amccarth wrote:

Maybe dotest.py could change its own Windows locale setting and the processes it spawns would inherit that. I don't know if that would work, but I don't see a good alternative.

One problem is that some users might not have the English language pack installed. I only have fr-CA and en-CA installed but not en-US. There's absolutely no guarantee that there will be a en-XX package available. I'll do some more testing and upload a new diff, but I took a defensive approach, like for StatusTest.cpp, where the two tests below in TestTargetCommand.py would only run if en-US is the current locale. Maybe afterwards, in a subsequent patch, we could add more code to temporarily switch to en-US locale if it's available?

[labath]

I really want to avoid having language specific strings in the tests. Skipping tests which depend on these is a reasonable approach (as you've discovered, there shouldn't be that many of them -- it's really just the tests which check that we actually can retrieve the error from the OS), and it could be later improved by switching to an english locale on a best-effort basis (i.e., if the locale is available).

[aganea]

In D88975#2317566, @labath wrote:

I really want to avoid having language specific strings in the tests.

We all agree on that. I'll update the patch once it passes all tests on my machines.

[aganea]

As discussed above:

• Remove locale-specific messages.
• Skip relevant locale-sensitive TestTargetCommand tests.
• Only skip EXPECT calls in Utility/StatusTest.

[aganea]

In D88975#2317494, @amccarth wrote:

It's interesting that I haven't encountered some of these errors. There are five _other_ lldb tests that do fail for me. I have a fix in the works for some of those.

Please see my (remaining) failing tests (with VS2019 16.7.5):

failing_lldb_vs2019.txt18 KBDownload

[labath]

This looks fine. Thanks.

lldb/unittests/Utility/StatusTest.cpp

76

llvm::array_lengthof(name);

77

reinterpret_cast?

[aganea]

Thank you all for your time!

[aganea]

I've commited one more fix here: https://reviews.llvm.org/rG97e7fbb343e2 - This didn't occur locally on my machine, but only on another machine. But in essence, the issue was the same, an accentuated Windows-1252 codepage character that was being decoded as UTF-8 by the Python runtime.

lldb/packages/Python/lldbsuite/test/decorators.py

 from __future__ import absolute_import
 
 # System modules
 from distutils.version import LooseVersion
 from functools import wraps
+import ctypes
+import locale
 import os
 import platform
 import re
 import sys
 import tempfile
 import subprocess
 
 # Third-party modules
def skipIfLinux(func):
     """Decorate the item to skip tests that should be skipped on Linux."""
     return skipIfPlatform(["linux"])(func)
 
 
 def skipIfWindows(func):
     """Decorate the item to skip tests that should be skipped on Windows."""
     return skipIfPlatform(["windows"])(func)
 
+def skipIfWindowsAndNonEnglish(func):
+    """Decorate the item to skip tests that should be skipped on non-English locales on Windows."""
+    def is_Windows_NonEnglish(self):
+        if lldbplatformutil.getPlatform() != "windows":
+            return None
+        kernel = ctypes.windll.kernel32
+        if locale.windows_locale[ kernel.GetUserDefaultUILanguage() ] == "en_US":
+            return None
+        return "skipping non-English Windows locale"
+    return skipTestIfFn(is_Windows_NonEnglish)(func)
+
 def skipUnlessWindows(func):
     """Decorate the item to skip tests that should be skipped on any non-Windows platform."""
     return skipUnlessPlatform(["windows"])(func)
 
 
 def skipUnlessDarwin(func):
     """Decorate the item to skip tests that should be skipped on any non Darwin platform."""
     return skipUnlessPlatform(lldbplatformutil.getDarwinOSTriples())(func)

lldb/packages/Python/lldbsuite/test/dotest.py

 from . import test_result
 from ..support import seven
 
 
 def is_exe(fpath):
     """Returns true if fpath is an executable."""
     if fpath == None:
         return False
+    if sys.platform == 'win32':
+        if not fpath.endswith(".exe"):
+            fpath += ".exe"
     return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
 
 
 def which(program):
     """Returns the full path to a program; None otherwise."""
     fpath, _ = os.path.split(program)
     if fpath:
         if is_exe(program):

lldb/packages/Python/lldbsuite/test_event/build_exception.py

 class BuildError(Exception):
 
     def __init__(self, called_process_error):
         super(BuildError, self).__init__("Error when building test subject")
         self.command = called_process_error.lldb_extensions.get(
             "command", "<command unavailable>")
         self.build_error = called_process_error.lldb_extensions["combined_output"]
 
     def __str__(self):
         return self.format_build_error(self.command, self.build_error)
 
     @staticmethod
     def format_build_error(command, command_output):
         return "Error when building test subject.\n\nBuild Command:\n{}\n\nBuild Command Output:\n{}".format(
-            command, command_output.decode("utf-8"))
+            command, command_output.decode("utf-8", errors='ignore'))

lldb/test/API/commands/target/basic/TestTargetCommand.py

def test_target_select_invalid_index(self):
                     substrs=["index 1 is out of range, valid target indexes are 0 - 0"])
 
 
     @no_debug_info_test
     def test_target_create_multiple_args(self):
         self.expect("target create a b", error=True,
                     substrs=["'target create' takes exactly one executable path"])
 
+    @skipIfWindowsAndNonEnglish
     @no_debug_info_test
     def test_target_create_nonexistent_core_file(self):
         self.expect("target create -c doesntexist", error=True,
                     patterns=["Cannot open 'doesntexist'", ": (No such file or directory|The system cannot find the file specified)"])
 
     # Write only files don't seem to be supported on Windows.
     @skipIfWindows
     @skipIfReproducer # Cannot be captured in the VFS.
     @no_debug_info_test
     def test_target_create_unreadable_core_file(self):
         tf = tempfile.NamedTemporaryFile()
         os.chmod(tf.name, stat.S_IWRITE)
         self.expect("target create -c '" + tf.name + "'", error=True,
                     substrs=["Cannot open '", "': Permission denied"])
 
+    @skipIfWindowsAndNonEnglish
     @no_debug_info_test
     def test_target_create_nonexistent_sym_file(self):
         self.expect("target create -s doesntexist doesntexisteither", error=True,
                     patterns=["Cannot open '", ": (No such file or directory|The system cannot find the file specified)"])
 
     @skipIfWindows
     @no_debug_info_test
     def test_target_create_invalid_core_file(self):

lldb/unittests/Utility/StatusTest.cpp

 //===-- StatusTest.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/Status.h"
 #include "gtest/gtest.h"
 
 #ifdef _WIN32
-#include <winerror.h>
+#include <windows.h>
 #endif
 
 using namespace lldb_private;
 using namespace lldb;
 
 TEST(StatusTest, Formatv) {
   EXPECT_EQ("", llvm::formatv("{0}", Status()).str());
   EXPECT_EQ("Hello Status", llvm::formatv("{0}", Status("Hello Status")).str());

 
 #ifdef _WIN32
 TEST(StatusTest, ErrorWin32) {
   auto success = Status(NO_ERROR, ErrorType::eErrorTypeWin32);
   EXPECT_STREQ(NULL, success.AsCString());
   EXPECT_FALSE(success.ToError());
   EXPECT_TRUE(success.Success());
 
+  WCHAR name[128]{};
+  ULONG nameLen = llvm::array_lengthof(name);
+  ULONG langs = 0;
+  GetUserPreferredUILanguages(MUI_LANGUAGE_NAME, &langs,
+                              reinterpret_cast<PZZWSTR>(&name), &nameLen);
+  // Skip the following tests on non-English, non-US, locales because the
+  // formatted messages will be different.
+  bool skip = wcscmp(L"en-US", name) != 0;
+
   auto s = Status(ERROR_ACCESS_DENIED, ErrorType::eErrorTypeWin32);
   EXPECT_TRUE(s.Fail());
+  if (!skip)
     EXPECT_STREQ("Access is denied. ", s.AsCString());
 
   s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
+  if (!skip)
     EXPECT_STREQ("Negotiation timed out ", s.AsCString());
 
   s.SetError(16000, ErrorType::eErrorTypeWin32);
+  if (!skip)
     EXPECT_STREQ("unknown error", s.AsCString());
 }
 #endif