[lldb] Use just-built libcxx for tests when available

[fdeazeve]

Summary

This commit improves upon cc0b5ebf7fc8, which added support for
specifying which libcxx to use when testing LLDB. That patch honored
requests by tests that had USE_LIBCPP=1 defined in their makefiles.
Now, we also use a non-default libcxx if all conditions below are true:

1. The test is not explicitly requesting the use of libstdcpp

(USE_LIBSTDCPP=1).

2. The test is not explicitly requesting the use of the system's

library (USE_SYSTEM_STDLIB=1).

3. A path to libcxx was either provided by the user through CMake flags

or libcxx was built together with LLDB.

Condition (2) is new and introduced in this patch in order to support
tests that are either:

• Cross-platform (such as API/macosx/macCatalyst and

API/tools/lldb-server). The just-built libcxx is usually not built for
platforms other than the host's.

• Cross-language (such as API/lang/objc/exceptions). In this case, the

Objective C runtime throws an exceptions that always goes through the
system's libcxx, instead of the just built libcxx. Fixing this would
require either changing the install-name of the just built libcxx in Mac
systems, or tuning the DYLD_LIBRARY_PATH variable at runtime.

Some other tests exposes limitations of LLDB when running with a debug
standard library. TestDbgInfoContentForwardLists had an assertion
removed, as it was checking for buggy LLDB behavior (which now
crashes). TestFixIts had a variable renamed, as the old name clashes
with a standard library name when debug info is present. This is a known
issue: https://github.com/llvm/llvm-project/issues/34391.

For TestSBModule, the way the "main" module is found was changed to
look for the "a.out" module, instead of relying on the index being 0. In
some systems, the index 0 is dyld when a custom standard library is
used.

[fdeazeve]

Improved error message

[fdeazeve]

This patch should finally enable us to fix the LLDB bots using older versions of Clang: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-matrix/

[JDevlieghere]

LGTM

[labath]

Cross-platform (such as API/macosx/macCatalyst and API/tools/lldb-server).

I find this statement fairly surprising. One should be able to run ~all tests in a "cross-platform" manner (i.e. where the test architecture is different than lldb (host) arch). What makes these two tests special? Couldn't we detect that we're in the cross-platform scenario (in python or cmake) and automatically switch to the system c++ library for all tests?

[fdeazeve]

In D132940#3761038, @labath wrote:

Cross-platform (such as API/macosx/macCatalyst and API/tools/lldb-server).

What makes these two tests special?

These two tests are _always_ building binaries for Mac Catalyst (API/macosx/macCatalyst), and watchOS/iOS Simulator (API/tools/lldb-server/TestAppleSimulatorOSType.py). Most builds of libcxx won't have support for those, so we end up with link errors like:

building for watchOS Simulator, but linking in dylib built for macOS, file
'[...] build/./lib/libc++.dylib' for architecture arm64

[labath]

In D132940#3761082, @fdeazeve wrote:

In D132940#3761038, @labath wrote:

Cross-platform (such as API/macosx/macCatalyst and API/tools/lldb-server).

What makes these two tests special?

These two tests are _always_ building binaries for Mac Catalyst (API/macosx/macCatalyst), and watchOS/iOS Simulator (API/tools/lldb-server/TestAppleSimulatorOSType.py).

Ok, that makes more sense. I was confused because the Makefile in lldb/test/API/tools/lldb-server is used by (a lot) more tests than just TestAppleSimulatorOSType. In that case, I would say that this argument should be passed directly from python, as a part of the self.build command (the test already passes a bunch of arguments there anyway).

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

390–395

Instead of three distinct variables, it might be nicer to just have one argument (STDLIB_KIND ?) which can take three different values...

[fdeazeve]

is used by (a lot) more tests than just TestAppleSimulatorOSType. In that case, I would say that this argument should be passed directly from python, as a part of the self.build command (the test already passes a bunch of arguments there anyway).

Good catch! Will change it in the next revision

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

390–395

I think this could be an improvement, but since it would involve changing _all_ makefiles that define these variables, IMO it would be better to do it in a separate NFC change, if that's ok

[fdeazeve]

Addressed review comments

[labath]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

390–395

Ok, I'm going to hold you to that. :)

[fdeazeve]

Added test changes for tests known to be faulty in the presence of a standard
library containing debug symbols, so that developers using debug builds don't
see any failures.

[fdeazeve]

I had add extra checks to some tests which are known to fail with debug builds of the standard library, disabling them while a fix is not proposed.
Didn't want devs using debug configs to suddenly start seeing test failures

[labath]

lldb/test/API/commands/expression/fixits/TestFixIts.py

54

What kind of error are you getting here? Are you sure this is not some form of https://github.com/llvm/llvm-project/issues/34391, which could be worked around by renaming some variables in the expression?

lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py

31

And here it might be better to just remove the check, as it's checking for buggy behavior anyway.

[fdeazeve]

Addressed review comments

lldb/test/API/commands/expression/fixits/TestFixIts.py

54

The error is indeed about a name clash with X, where the expression evaluator also sees a definition of a type X inside a function in libcxxabi/src/private_typeinfo.cpp.

It seems like the link you described has a variant of this problem, so I'll make a comment there and rename the X in this test to something else. Thanks for the link!

lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py

31

Done!

[JDevlieghere]

LGTM

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

396

s/library/standard library/

[aprantl]

Could we land this today so we can get the bots green again? Please still make sure to respond to any post-commit feedback from @labath in case there's anything hasn't been addressed.

[fdeazeve]

Updated wording of error message

[fdeazeve]

It seems that a linux bot is failing after this: https://lab.llvm.org/buildbot/#/builders/68/builds/39086

This job does _not_ build the libcxx projects, and yet the LIBCPP_INCLUDE_DIR variable is defined. I'm investigating

[fdeazeve]

I've reverted this while I find an Ubuntu machine to investigate

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

ifneq (,$(findstring clang,$(CC)))
     # generated by the clang compiler.
     LDFLAGS += -fuse-ld=lld
   endif
 endif
 
 #----------------------------------------------------------------------
 # C++ standard library options
 #----------------------------------------------------------------------
+ifneq ($(and $(USE_LIBSTDCPP), $(USE_LIBCPP)),)
+  $(error Libcxx and Libstdc++ cannot be used together)
+endif
+
+ifeq (1, $(USE_SYSTEM_STDLIB))
+  ifneq ($(or $(USE_LIBSTDCPP), $(USE_LIBCPP)),)
+    $(error Cannot use system's standard library and a custom standard library together)
+  endif
+endif
+
 ifeq (1,$(USE_LIBSTDCPP))
   # Clang requires an extra flag: -stdlib=libstdc++
   ifneq (,$(findstring clang,$(CC)))
     CXXFLAGS += -stdlib=libstdc++
     LDFLAGS += -stdlib=libstdc++
   endif
 endif
 
else
     ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
         ifneq (,$(LLVM_LIBS_DIR))
         LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
         endif
     endif
   endif
 endif
 
+# If no explicit request was made, but we have paths to a custom libcxx, use
+# them.
+ifeq ($(or $(USE_LIBSTDCPP), $(USE_LIBCPP), $(USE_SYSTEM_STDLIB)),)
+  ifneq ($(and $(LIBCPP_INCLUDE_DIR), $(LIBCPP_LIBRARY_DIR)),)
+    CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(LIBCPP_INCLUDE_DIR)
+    LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++
+  endif
+endif
+
 #----------------------------------------------------------------------
 # Additional system libraries
 #----------------------------------------------------------------------
 ifeq (1,$(USE_LIBDL))
   ifeq (,$(filter $(OS), NetBSD Windows_NT))
     LDFLAGS += -ldl
   endif
 endif

lldb/test/API/commands/expression/fixits/TestFixIts.py

def test_with_target(self):
         # Try with one error:
         value = frame.EvaluateExpression("my_pointer.first", options)
         self.assertTrue(value.IsValid())
         self.assertSuccess(value.GetError())
         self.assertEquals(value.GetValueAsUnsigned(), 10)
 
         # Try with one error in a top-level expression.
         # The Fix-It changes "ptr.m" to "ptr->m".
-        expr = "struct X { int m; }; X x; X *ptr = &x; int m = ptr.m;"
+        expr = "struct MyTy { int m; }; MyTy x; MyTy *ptr = &x; int m = ptr.m;"
         value = frame.EvaluateExpression(expr, top_level_options)
         # A successfully parsed top-level expression will yield an error
         # that there is 'no value'. If a parsing error would have happened we
         # would get a different error kind, so let's check the error kind here.
         self.assertEquals(value.GetError().GetCString(), "error: No value")
 
         # Try with two errors:
         two_error_expression = "my_pointer.second->a"
         value = frame.EvaluateExpression(two_error_expression, options)
         self.assertTrue(value.IsValid())
         self.assertSuccess(value.GetError())
         self.assertEquals(value.GetValueAsUnsigned(), 20)
 
         # Try a Fix-It that is stored in the 'note:' diagnostic of an error.

lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py

def test(self):
         list_type = "std::forward_list<Foo>"
         value_type = "value_type"
 
         # FIXME: This has three elements in it but the formatter seems to
         # calculate the wrong size and contents.
         self.expect_expr("a", result_type=list_type, result_summary="size=1")
         self.expect_expr("std::distance(a.begin(), a.end())", result_value="3")
         self.expect_expr("a.front().a", result_type="int", result_value="3")
         self.expect_expr("a.begin()->a", result_type="int", result_value="3")
-
-        # FIXME: The value here isn't actually empty.
-        self.expect_expr("a.front()",
-                         result_type=value_type,
-                         result_children=[ValueCheck()])

lldb/test/API/lang/objc/exceptions/Makefile

 OBJCXX_SOURCES := main.mm
 
 CFLAGS_EXTRAS := -w
 
-
+USE_SYSTEM_STDLIB := 1
 
 LD_EXTRAS := -framework Foundation
 include Makefile.rules

lldb/test/API/macosx/macCatalyst/Makefile

 C_SOURCES := main.c
 
 override TRIPLE := $(ARCH)-apple-ios13.1-macabi
 CFLAGS_EXTRAS := -target $(TRIPLE)
 
+USE_SYSTEM_STDLIB := 1
+
 # FIXME: rdar://problem/54986190
 # There is a Clang driver change missing on llvm.org.
 override CC=xcrun clang
 
 include Makefile.rules

lldb/test/API/python_api/sbmodule/TestSBModule.py

def test_module_is_file_backed(self):
         os.unlink(exe)
 
         target = self.dbg.CreateTarget('')
         self.assertEqual(self.dbg.GetNumTargets(), 1)
         error = lldb.SBError()
         process = target.AttachToProcessWithID(self.dbg.GetListener(),
                                                self.background_pid, error)
         self.assertTrue(error.Success() and process,  PROCESS_IS_VALID)
-        main_module = target.GetModuleAtIndex(0)
-        self.assertEqual(main_module.GetFileSpec().GetFilename(), "a.out")
+        main_module = target.FindModule(lldb.SBFileSpec("a.out"))
+        self.assertTrue(main_module is not None)
         self.assertFalse(main_module.IsFileBacked(),
                          "The module should not be backed by a file on disk.")
 
         error = process.Destroy()
         self.assertSuccess(error, "couldn't destroy process %s" % background_process.pid)
 

lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py

def check_simulator_ostype(self, sdk, platform_name, arch=platform.machine()):
         version_min = '-m{}-simulator-version-min={}'.format(platform_name, vers)
         self.build(
             dictionary={
                 'EXE': exe_name,
                 'CC': clang,
                 'SDKROOT': sdkroot.strip(),
                 'ARCH': arch,
                 'ARCH_CFLAGS': '-target {} {}'.format(triple, version_min),
+                'USE_SYSTEM_STDLIB': 1,
             })
         exe_path = os.path.realpath(self.getBuildArtifact(exe_name))
         cmd = [
             'xcrun', 'simctl', 'spawn', '-s', deviceUDID, exe_path,
             'print-pid', 'sleep:10'
         ]
         self.trace(' '.join(cmd))
         sim_launcher = subprocess.Popen(cmd, stderr=subprocess.PIPE)