Add argument to expectedFailureAll decorator to xfail for environment variables, and xfail tests for hard float abi and -mthumb on android arm

[chying]

Summary

xfail TestCallUserAnonTypedef.test for -mthumb, TestReturnValue.test_with_python for hard fp, TestLldbGdbServer.software_breakpoint_set_and_remove_work for hard fp.

[zturner]

This seems very strange to me. What if someone specifies those flags on the command line in the makefile instead of via some environment variables?

It also seems like a very specialized argument that we might use once or twice ever.

+pavel and tamas in case they have some suggestions.

[chaoren]

In D18481#383694, @zturner wrote:

This seems very strange to me. What if someone specifies those flags on the command line in the makefile instead of via some environment variables?

I don't think there's anything we can do about that unless we parse the Makefiles from within the test suite. These kinds of flags are bad to hard code into the Makefiles anyway, unless there's a way to mark a test as thumb specific or hard float specific in the test suite, in which case we'd be using *those* indicators instead to filter for xfail. For now I think this is a pretty reasonable way to discriminate on the validity of failures based on compile flags.

It also seems like a very specialized argument that we might use once or twice ever.

This applies to arbitrary compile flags. Though you probably won't ever need it for x86/x86_64, I can see it being useful for ARM and MIPS due to the sheer number of variations.

[zturner]

Couldn't we have a way to say in the Makefile "never use these flags", then the test suite could check the environment and remove them if they are present. This woudl allow the test to run.

architecture, compiler, etc are things we don't really have control over. If a flag or an environment variable is causing a test to fail, it seems like the right thing to do is not to use that flag or environment variable.

[zturner]

I'll still wait and see what Pavel and/or Tamas say, but if we are going to go this route, I would rather the argument be called cflags and not env_flags. Xfailing a test based on an arbitrary environment variable just seems like something we shouldn't be doing. I'd rather prime the environment however we need in order to get the test to run, than disable it and lose test coverage.

[chying]

In D18481#383718, @zturner wrote:

Couldn't we have a way to say in the Makefile "never use these flags", then the test suite could check the environment and remove them if they are present. This woudl allow the test to run.

architecture, compiler, etc are things we don't really have control over. If a flag or an environment variable is causing a test to fail, it seems like the right thing to do is not to use that flag or environment variable.

I don't find a way to pass compiler flags to test suite by current dotest options (please let me know if there is), so they're passed by CFLAGS_EXTRAS variable, which is checked and applied in the main Makefile. Alternative would be adding new dotest options to take compiler flags, then we add xfail based on compiler flag, please let me know how you think about this approach.

In D18481#383720, @zturner wrote:

I'll still wait and see what Pavel and/or Tamas say, but if we are going to go this route, I would rather the argument be called cflags and not env_flags. Xfailing a test based on an arbitrary environment variable just seems like something we shouldn't be doing. I'd rather prime the environment however we need in order to get the test to run, than disable it and lose test coverage.

Yes, let's get more opinions on this.

[tberghammer]

You can pass extra compiler flags to the test suit with the -E option (haven't tested it). I would suggest to use that option and then xfail based on it (xfail if extra flags contain -foo).

In long term I would prefer to not add a new option to expectedFailureAll but to implement a generic functional style solution where we can just pass in arbitrary number of lambda functions to it so the number of arguments aren't increasing constantly but it is something what is completely unrelated to this patch.

[labath]

So, the way I would achieve this is to have a way to specify "i want to run the tests with hard-float", or "I want to use thumb instruction set" when running dotest.py and then based on this information, set the correct compiler flags and xfail/skip tests. When we do it this way, we could actually fix some of the tests you are trying to skip here. E.g. the only reason TestLldbGdbServer is failing for thumb is because the test does not know the instruction set and so it cannot put the right kind of a breakpoint. As for the dotest flags, we could either invent some new flags (--isa and --abi, perhaps?) or shove this information into --arch somehow (--arch=arm-softfp, --arch=thumb-hardfp ?).

[tberghammer]

I would prefer a more general solution then the one Pavel proposed because currently we are adding 2 more options (abi and isa) but in the future we might need a lot of other stuff as well and adding a new argument for each one will be problematic (e.g. omit frame pointer, tune for processor core, specify fpu instruction set, etc...). I think if we have the option to specify a list of additional command line flags then tests can change behavior based on that list what will result in a similar behavior then Pavel's suggestion

packages/Python/lldbsuite/test/decorators.py

def skipTestIfFn(expected_fn, bugnumber=None):
     # or with arguments (e.g. @expectedFailureWindows(compilers=['gcc'])).  When called
     # the first way, the first argument will be the actual function because decorators are
     # weird like that.  So this is basically a check that says "how was the decorator used"
     if six.callable(bugnumber):
         return skipTestIfFn_impl(bugnumber)
     else:
         return skipTestIfFn_impl
 
+def _check_expected_env_flag(env_flags):
+    for env, value in env_flags.iteritems():
+        env_val = os.environ.get(env)
+        if env_val is None or value not in env_val.split():
+            return False
+    return True
+
 def _decorateTest(mode,
                  bugnumber=None, oslist=None, hostoslist=None,
                  compiler=None, compiler_version=None,
                  archs=None, triple=None,
                  debug_info=None,
                  swig_version=None, py_version=None,
-                 remote=None):
+                 remote=None, env_flags=None):
     def fn(self):
         skip_for_os = _match_decorator_property(lldbplatform.translate(oslist), self.getPlatform())
         skip_for_hostos = _match_decorator_property(lldbplatform.translate(hostoslist), lldbplatformutil.getHostPlatform())
         skip_for_compiler = _match_decorator_property(compiler, self.getCompiler()) and self.expectedCompilerVersion(compiler_version)
         skip_for_arch = _match_decorator_property(archs, self.getArchitecture())
         skip_for_debug_info = _match_decorator_property(debug_info, self.debug_info)
         skip_for_triple = _match_decorator_property(triple, lldb.DBG.GetSelectedPlatform().GetTriple())
         skip_for_remote = _match_decorator_property(remote, lldb.remote_platform is not None)
 
         skip_for_swig_version = (swig_version is None) or (not hasattr(lldb, 'swig_version')) or (_check_expected_version(swig_version[0], swig_version[1], lldb.swig_version))
         skip_for_py_version = (py_version is None) or _check_expected_version(py_version[0], py_version[1], sys.version_info)
 
+        skip_for_env_flags = (env_flags is None) or _check_expected_env_flag(env_flags)
+
         # For the test to be skipped, all specified (e.g. not None) parameters must be True.
         # An unspecified parameter means "any", so those are marked skip by default.  And we skip
         # the final test if all conditions are True.
         conditions = [(oslist, skip_for_os, "target o/s"),
                       (hostoslist, skip_for_hostos, "host o/s"),
                       (compiler, skip_for_compiler, "compiler or version"),
                       (archs, skip_for_arch, "architecture"),
                       (debug_info, skip_for_debug_info, "debug info format"),
                       (triple, skip_for_triple, "target triple"),
                       (swig_version, skip_for_swig_version, "swig version"),
                       (py_version, skip_for_py_version, "python version"),
-                      (remote, skip_for_remote, "platform locality (remote/local)")]
+                      (remote, skip_for_remote, "platform locality (remote/local)"),
+                      (env_flags, skip_for_env_flags, "environment flags")]
         reasons = []
         final_skip_result = True
         for this_condition in conditions:
             final_skip_result = final_skip_result and this_condition[1]
             if this_condition[0] is not None and this_condition[1]:
                 reasons.append(this_condition[2])
         reason_str = None
         if final_skip_result:

 # @expectedFailureAll(compiler='gcc'), xfail for gcc on all platform/architecture
 # @expectedFailureAll(bugnumber, ["linux"], "gcc", ['>=', '4.9'], ['i386']), xfail for gcc>=4.9 on linux with i386
 def expectedFailureAll(bugnumber=None,
                        oslist=None, hostoslist=None,
                        compiler=None, compiler_version=None,
                        archs=None, triple=None,
                        debug_info=None,
                        swig_version=None, py_version=None,
-                       remote=None):
+                       remote=None, env_flags=None):
     return _decorateTest(DecorateMode.Xfail,
                         bugnumber=bugnumber,
                         oslist=oslist, hostoslist=hostoslist,
                         compiler=compiler, compiler_version=compiler_version,
                         archs=archs, triple=triple,
                         debug_info=debug_info,
                         swig_version=swig_version, py_version=py_version,
-                        remote=remote)
+                        remote=remote, env_flags=env_flags)
 
 
 # provide a function to skip on defined oslist, compiler version, and archs
 # if none is specified for any argument, that argument won't be checked and thus means for all
 # for example,
 # @skipIf, skip for all platform/compiler/arch,
 # @skipIf(compiler='gcc'), skip for gcc on all platform/architecture
 # @skipIf(bugnumber, ["linux"], "gcc", ['>=', '4.9'], ['i386']), skip for gcc>=4.9 on linux with i386

packages/Python/lldbsuite/test/expression_command/anonymous-struct/TestCallUserAnonTypedef.py

 class TestExprLookupAnonStructTypedef(TestBase):
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         TestBase.setUp(self)
         # Find the breakpoint
         self.line = line_number('main.cpp', '// lldb testsuite break')
 
+    @expectedFailureAll(bugnumber="llvm.org/pr27074", compiler="gcc", archs=["arm"], env_flags={"CFLAGS_EXTRAS": "-mfloat-abi=hard"})
     def test(self):
         """Test typedeffed untagged struct arguments for function call expressions"""
         self.build()
 
         self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
         lldbutil.run_break_set_by_file_and_line(
             self,
             "main.cpp",
             self.line,
             num_expected_locations=-1,
             loc_exact=True
         )
 
         self.runCmd("run", RUN_SUCCEEDED)
         self.expect("expr multiply(&s)", substrs=['$0 = 1'])

packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py

 class ReturnValueTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @expectedFailureAll(oslist=["macosx","freebsd"], archs=["i386"])
     @expectedFailureAll(oslist=["linux"], compiler="clang", compiler_version=["<=", "3.6"], archs=["i386"])
     @expectedFailureAll(bugnumber="llvm.org/pr25785", hostoslist=["windows"], compiler="gcc", archs=["i386"], triple='.*-android')
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
+    @expectedFailureAll(bugnumber="llvm.org/pr27073", compiler="gcc", archs=["arm"], env_flags={"CFLAGS_EXTRAS": "-mfloat-abi=hard"})
     @add_test_categories(['pyapi'])
     def test_with_python(self):
         """Test getting return values from stepping out."""
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
         error = lldb.SBError()
 
         self.target = self.dbg.CreateTarget(exe)

packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

 
     @llgs_test
     def test_qMemoryRegionInfo_reports_heap_address_as_readable_writeable_llgs(self):
         self.init_llgs_test()
         self.build()
         self.set_inferior_startup_launch()
         self.qMemoryRegionInfo_reports_heap_address_as_readable_writeable()
 
+    @expectedFailureAll(bugnumber="llvm.org/pr27075", compiler="gcc", archs=["arm"], env_flags={"CFLAGS_EXTRAS": "-mthumb"})
     def software_breakpoint_set_and_remove_work(self):
         # Start up the inferior.
         procs = self.prep_debug_monitor_and_inferior(
             inferior_args=["get-code-address-hex:hello", "sleep:1", "call-function:hello"])
 
         # Run the process
         self.add_register_info_collection_packets()
         self.add_process_info_collection_packets()