Skip to content

[lldb] Fallback to implicit modules when explicit modules are missing #8356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,15 @@
#include "clang/Driver/Driver.h"

#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/MC/TargetRegistry.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/TargetSelect.h"
Expand Down Expand Up @@ -1594,9 +1596,50 @@ void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string> &source,
}
}

namespace {

bool HasNonexistentExplicitModule(const std::vector<std::string> &args) {
for (const auto &arg : args) {
StringRef value = arg;
if (!value.consume_front("-fmodule-file="))
continue;
StringRef path = value;
size_t eq = value.find('=');
// The value that follows is in one of two formats:
// 1. ModuleName=ModulePath
// 2. ModulePath
if (eq != std::string::npos)
// The value appears to be in ModuleName=ModulePath forat.
path = value.drop_front(eq + 1);
// Check both path and value. This is to handle paths containing '='.
if (!llvm::sys::fs::exists(path) && !llvm::sys::fs::exists(value)) {
std::string m_description;
HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", path.data());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately you'll need to either use path.str() or add a HEALTH_LOG macro that takes LLVM formats. There is no guarantee that .data() is nullterminated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The StringRef references a std::string, which I understand would it a guarantee that data is null terminated.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Technically correct :-)
  2. But then it would be safer to replace const auto & with const std::string & then to guard against future modification
  3. It would be even better to add a HEALTH_LOG that uses llvm::format so we can avoid the problem altogether

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do HEALTH_LOG in an isolated change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced auto with std::string in 610d19f

return true;
}
}
return false;
}

void RemoveExplicitModules(std::vector<std::string> &args) {
llvm::erase_if(args, [](const std::string &arg) {
if (arg == "-fno-implicit-modules" || arg == "-fno-implicit-module-maps")
return true;
StringRef s = arg;
if (s.starts_with("-fmodule-file=") || s.starts_with("-fmodule-map-file="))
return true;

return false;
});
}

} // namespace

void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string> &ExtraArgs) {
swift::ClangImporterOptions &importer_options = GetClangImporterOptions();
AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs);
if (HasNonexistentExplicitModule(importer_options.ExtraArgs))
RemoveExplicitModules(importer_options.ExtraArgs);
}

void SwiftASTContext::AddUserClangArgs(TargetProperties &props) {
Expand Down