Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -519,7 +519,8 @@ // Try to create a configuration from the files. If there is no valid // configuration possible with the files, this just returns an invalid // configuration. - return CppModuleConfiguration(files); + return CppModuleConfiguration(files, + *FileSystem::Instance().GetVirtualFileSystem()); } bool ClangUserExpression::PrepareForParsing( Index: lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h +++ lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h @@ -11,6 +11,7 @@ #include #include +#include namespace lldb_private { @@ -60,13 +61,17 @@ /// /// Currently only looks at the used paths and doesn't actually access the /// files on the disk. - explicit CppModuleConfiguration(const FileSpecList &support_files); + /// \param support_files The list of source files. + /// \param vfs The FileSystem to use when accessing files on disk. + explicit CppModuleConfiguration(const FileSpecList &support_files, + llvm::vfs::FileSystem &vfs); /// Creates an empty and invalid configuration. CppModuleConfiguration() {} /// Returns true iff this is a valid configuration that can be used to /// load and compile modules. - bool hasValidConfig(); + /// \param vfs The FileSystem to use when accessing files on disk. + bool hasValidConfig(llvm::vfs::FileSystem &vfs); /// Returns a list of include directories that should be used when using this /// configuration (e.g. {"/usr/include", "/usr/include/c++/v1"}). Index: lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp @@ -57,20 +57,42 @@ return true; } -bool CppModuleConfiguration::hasValidConfig() { - // We all these include directories to have a valid usable configuration. - return m_c_inc.Valid() && m_std_inc.Valid(); +bool CppModuleConfiguration::hasValidConfig(llvm::vfs::FileSystem &vfs) { + // We need to have a C and C++ include dir for a valid configuration. + if (!m_c_inc.Valid() || !m_std_inc.Valid()) + return false; + + // Do some basic sanity checks on the directories that we don't activate + // the module when it's clear that it's not usable. + const std::vector filesToCheck = { + // * Check that the C library contains at least one random C standard + // library header. + m_c_inc.Get() + "/stdio.h", + // * Without a libc++ modulemap file we can't have a 'std' module that + // could be imported. + m_std_inc.Get() + "/module.modulemap", + // * Check for a random libc++ header (vector in this case) that has to + // exist in a working libc++ setup. + m_std_inc.Get() + "/vector", + }; + + for (llvm::StringRef fileToCheck : filesToCheck) { + if (!vfs.exists(fileToCheck)) + return false; + } + + return true; } CppModuleConfiguration::CppModuleConfiguration( - const FileSpecList &support_files) { + const FileSpecList &support_files, llvm::vfs::FileSystem &vfs) { // Analyze all files we were given to build the configuration. bool error = !llvm::all_of(support_files, std::bind(&CppModuleConfiguration::analyzeFile, this, std::placeholders::_1)); // If we have a valid configuration at this point, set the // include directories and module list that should be used. - if (!error && hasValidConfig()) { + if (!error && hasValidConfig(vfs)) { // Calculate the resource directory for LLDB. llvm::SmallString<256> resource_dir; llvm::sys::path::append(resource_dir, GetClangResourceDir().GetPath(), Index: lldb/test/API/commands/expression/import-std-module/missing-module-sources/Makefile =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import-std-module/missing-module-sources/Makefile @@ -0,0 +1,10 @@ +# We don't have any standard include directories, so we can't +# parse the test_common.h header we usually inject as it includes +# system headers. +NO_TEST_COMMON_H := 1 + +# Take the libc++ from the build directory (which will be later deleted). +CXXFLAGS_EXTRAS = -I $(BUILDDIR)/root/usr/include/c++/v1/ -I $(BUILDDIR)/root/usr/include/ -nostdinc -nostdinc++ +CXX_SOURCES := main.cpp + +include Makefile.rules Index: lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py @@ -0,0 +1,60 @@ +""" +Check that missing module source files are correctly handled by LLDB. +""" + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +import os +import shutil + + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + # We only emulate a fake libc++ in this test and don't use the real libc++, + # but we still add the libc++ category so that this test is only run in + # test configurations where libc++ is actually supposed to be tested. + @add_test_categories(["libc++"]) + @skipIf(compiler=no_match("clang")) + def test(self): + # The path to our temporary target root that contains the temporary + # module sources. + target_sysroot = self.getBuildArtifact("root") + + # Copy the sources to the root. + shutil.copytree(self.getSourcePath("root"), target_sysroot) + # Build the binary with the copied sources. + self.build() + # Delete the copied sources so that they are now unavailable. + shutil.rmtree(target_sysroot) + + # Set the sysroot where our dummy libc++ used to exist. Just to make + # sure we don't find some existing headers on the system that could + # XPASS this test. + self.runCmd("platform select --sysroot '" + target_sysroot + "' host") + + lldbutil.run_to_source_breakpoint(self, + "// Set break point at this line.", + lldb.SBFileSpec("main.cpp")) + + # Import the std C++ module and run an expression. + # As we deleted the sources, LLDB should refuse the load the module + # and just print the normal error we get from the expression. + self.runCmd("settings set target.import-std-module true") + self.expect("expr v.unknown_identifier", error=True, + substrs=["no member named 'unknown_identifier'"]) + # Check that there is no confusing error about failing to build the + # module. + self.expect("expr v.unknown_identifier", error=True, matching=False, + substrs=["could not build module 'std'"]) + + # Test the fallback mode. It should also just print the normal + # error but not mention a failed module build. + self.runCmd("settings set target.import-std-module fallback") + + self.expect("expr v.unknown_identifier", error=True, + substrs=["no member named 'unknown_identifier'"]) + self.expect("expr v.unknown_identifier", error=True, matching=False, + substrs=["could not build module 'std'"]) Index: lldb/test/API/commands/expression/import-std-module/missing-module-sources/main.cpp =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import-std-module/missing-module-sources/main.cpp @@ -0,0 +1,8 @@ +#include + +int main(int argc, char **argv) { + // Makes sure we have the mock libc headers in the debug information. + libc_struct s; + std::vector v; + return 0; // Set break point at this line. +} Index: lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/module.modulemap =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/module.modulemap @@ -0,0 +1,3 @@ +module std { + module "vector" { header "vector" export * } +} Index: lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/vector =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/vector @@ -0,0 +1,9 @@ +#include "libc_header.h" + +namespace std { + inline namespace __1 { + template + struct vector { + }; + } +} Index: lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/libc_header.h =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/libc_header.h @@ -0,0 +1 @@ +struct libc_struct {}; Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp =================================================================== --- lldb/unittests/Expression/CppModuleConfigurationTest.cpp +++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp @@ -11,6 +11,7 @@ #include "TestingSupport/SubsystemRAII.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" +#include "llvm/Support/SmallVectorMemoryBuffer.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -20,6 +21,21 @@ namespace { struct CppModuleConfigurationTest : public testing::Test { SubsystemRAII subsystems; + llvm::vfs::InMemoryFileSystem fs; + llvm::MemoryBufferRef EmptyBuffer; + + CppModuleConfigurationTest() : EmptyBuffer("", "") {} + + /// Utility function turning a list of paths into a FileSpecList. + FileSpecList makeFiles(llvm::ArrayRef paths) { + FileSpecList result; + for (const std::string &path : paths) { + result.Append(FileSpec(path, FileSpec::Style::posix)); + if (!fs.addFileNoOwn(path, static_cast(0), EmptyBuffer)) + llvm_unreachable("Invalid test configuration?"); + } + return result; + } }; } // namespace @@ -31,20 +47,18 @@ return std::string(resource_dir); } -/// Utility function turningn a list of paths into a FileSpecList. -static FileSpecList makeFiles(llvm::ArrayRef paths) { - FileSpecList result; - for (const std::string &path : paths) - result.Append(FileSpec(path, FileSpec::Style::posix)); - return result; -} TEST_F(CppModuleConfigurationTest, Linux) { // Test the average Linux configuration. - std::string libcpp = "/usr/include/c++/v1"; + std::string usr = "/usr/include"; - CppModuleConfiguration config( - makeFiles({usr + "/bits/types.h", libcpp + "/vector"})); + std::string libcpp = "/usr/include/c++/v1"; + std::vector files = {// C library + usr + "/stdio.h", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre(libcpp, ResourceInc(), usr)); @@ -52,10 +66,15 @@ TEST_F(CppModuleConfigurationTest, Sysroot) { // Test that having a sysroot for the whole system works fine. + std::string libcpp = "/home/user/sysroot/usr/include/c++/v1"; std::string usr = "/home/user/sysroot/usr/include"; - CppModuleConfiguration config( - makeFiles({usr + "/bits/types.h", libcpp + "/vector"})); + std::vector files = {// C library + usr + "/stdio.h", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre(libcpp, ResourceInc(), usr)); @@ -63,10 +82,15 @@ TEST_F(CppModuleConfigurationTest, LinuxLocalLibCpp) { // Test that a locally build libc++ is detected. - std::string libcpp = "/home/user/llvm-build/include/c++/v1"; + std::string usr = "/usr/include"; - CppModuleConfiguration config( - makeFiles({usr + "/bits/types.h", libcpp + "/vector"})); + std::string libcpp = "/home/user/llvm-build/include/c++/v1"; + std::vector files = {// C library + usr + "/stdio.h", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre(libcpp, ResourceInc(), usr)); @@ -74,10 +98,17 @@ TEST_F(CppModuleConfigurationTest, UnrelatedLibrary) { // Test that having an unrelated library in /usr/include doesn't break. - std::string libcpp = "/home/user/llvm-build/include/c++/v1"; + std::string usr = "/usr/include"; - CppModuleConfiguration config(makeFiles( - {usr + "/bits/types.h", libcpp + "/vector", usr + "/boost/vector"})); + std::string libcpp = "/home/user/llvm-build/include/c++/v1"; + std::vector files = {// C library + usr + "/stdio.h", + // unrelated library + usr + "/boost/vector", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre(libcpp, ResourceInc(), usr)); @@ -85,12 +116,19 @@ TEST_F(CppModuleConfigurationTest, Xcode) { // Test detection of libc++ coming from Xcode with generic platform names. + std::string p = "/Applications/Xcode.app/Contents/Developer/"; std::string libcpp = p + "Toolchains/B.xctoolchain/usr/include/c++/v1"; std::string usr = p + "Platforms/A.platform/Developer/SDKs/OSVers.sdk/usr/include"; - CppModuleConfiguration config( - makeFiles({libcpp + "/unordered_map", usr + "/stdio.h"})); + std::vector files = { + // C library + usr + "/stdio.h", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap", + }; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre(libcpp, ResourceInc(), usr)); @@ -98,8 +136,14 @@ TEST_F(CppModuleConfigurationTest, LibCppV2) { // Test that a "v2" of libc++ is still correctly detected. - CppModuleConfiguration config( - makeFiles({"/usr/include/bits/types.h", "/usr/include/c++/v2/vector"})); + + std::string libcpp = "/usr/include/c++/v2"; + std::vector files = {// C library + "/usr/include/stdio.h", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre("/usr/include/c++/v2", ResourceInc(), @@ -109,8 +153,15 @@ TEST_F(CppModuleConfigurationTest, UnknownLibCppFile) { // Test that having some unknown file in the libc++ path doesn't break // anything. - CppModuleConfiguration config(makeFiles( - {"/usr/include/bits/types.h", "/usr/include/c++/v1/non_existing_file"})); + + std::string libcpp = "/usr/include/c++/v1"; + std::vector files = {// C library + "/usr/include/stdio.h", + // C++ library + libcpp + "/non_existing_file", + libcpp + "/module.modulemap", + libcpp + "/vector"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std")); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre("/usr/include/c++/v1", ResourceInc(), @@ -119,22 +170,40 @@ TEST_F(CppModuleConfigurationTest, MissingUsrInclude) { // Test that we don't load 'std' if we can't find the C standard library. - CppModuleConfiguration config(makeFiles({"/usr/include/c++/v1/vector"})); + + std::string libcpp = "/usr/include/c++/v1"; + std::vector files = {// C++ library + libcpp + "/vector", + libcpp + "/module.modulemap"}; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre()); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre()); } TEST_F(CppModuleConfigurationTest, MissingLibCpp) { // Test that we don't load 'std' if we don't have a libc++. - CppModuleConfiguration config(makeFiles({"/usr/include/bits/types.h"})); + + std::string usr = "/usr/include"; + std::vector files = { + // C library + usr + "/stdio.h", + }; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre()); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre()); } TEST_F(CppModuleConfigurationTest, IgnoreLibStdCpp) { // Test that we don't do anything bad when we encounter libstdc++ paths. - CppModuleConfiguration config(makeFiles( - {"/usr/include/bits/types.h", "/usr/include/c++/8.0.1/vector"})); + + std::string usr = "/usr/include"; + std::vector files = { + // C library + usr + "/stdio.h", + // C++ library + usr + "/c++/8.0.1/vector", + }; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre()); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre()); } @@ -142,9 +211,20 @@ TEST_F(CppModuleConfigurationTest, AmbiguousCLib) { // Test that we don't do anything when we are not sure where the // right C standard library is. - CppModuleConfiguration config( - makeFiles({"/usr/include/bits/types.h", "/usr/include/c++/v1/vector", - "/sysroot/usr/include/bits/types.h"})); + + std::string usr1 = "/usr/include"; + std::string usr2 = "/usr/include/other/path"; + std::string libcpp = usr1 + "c++/v1"; + std::vector files = { + // First C library + usr1 + "/stdio.h", + // Second C library + usr2 + "/stdio.h", + // C++ library + libcpp + "/vector", + libcpp + "/module.modulemap", + }; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre()); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre()); } @@ -152,9 +232,21 @@ TEST_F(CppModuleConfigurationTest, AmbiguousLibCpp) { // Test that we don't do anything when we are not sure where the // right libc++ is. - CppModuleConfiguration config( - makeFiles({"/usr/include/bits/types.h", "/usr/include/c++/v1/vector", - "/usr/include/c++/v2/vector"})); + + std::string usr = "/usr/include"; + std::string libcpp1 = usr + "c++/v1"; + std::string libcpp2 = usr + "c++/v2"; + std::vector files = { + // C library + usr + "/stdio.h", + // First C++ library + libcpp1 + "/vector", + libcpp1 + "/module.modulemap", + // Second C++ library + libcpp2 + "/vector", + libcpp2 + "/module.modulemap", + }; + CppModuleConfiguration config(makeFiles(files), fs); EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre()); EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre()); }