Skip to content

Turn off PCM validation by default. #9051

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
merged 1 commit into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions lldb/include/lldb/Target/Target.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ class TargetProperties : public Properties {

bool GetSwiftAllowExplicitModules() const;

AutoBool GetSwiftPCMValidation() const;

Args GetSwiftPluginServerForPath() const;

bool GetSwiftAutoImportFrameworks() const;
Expand Down
34 changes: 34 additions & 0 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
#include "clang/Basic/TargetOptions.h"
#include "clang/Driver/Driver.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"

#include "clang/Lex/PreprocessorOptions.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
Expand Down Expand Up @@ -9210,6 +9212,38 @@ bool SwiftASTContext::GetCompileUnitImportsImpl(
if (cu_imports.size() == 0)
return true;

// Set PCM validation. This is not a great place to do this, but it
// needs to happen after ClangImporter was created and
// m_has_explicit_modules has been initialized.
{
// Read the setting.
AutoBool validate_pcm_setting = AutoBool::Auto;
TargetSP target_sp = GetTargetWP().lock();
if (target_sp)
validate_pcm_setting = target_sp->GetSwiftPCMValidation();

// If the setting is explicit, honor it.
bool validate_pcm = validate_pcm_setting != AutoBool::False;
if (validate_pcm_setting == AutoBool::Auto) {
// Disable validation for explicit modules.
validate_pcm = m_has_explicit_modules ? false : true;
// Enable validation in asserts builds.
#ifndef NDEBUG
validate_pcm = true;
#endif
}

auto &pp_opts = m_clangimporter->getClangPreprocessor()
.getPreprocessorOpts();
pp_opts.DisablePCHOrModuleValidation =
validate_pcm ? clang::DisableValidationForModuleKind::None
: clang::DisableValidationForModuleKind::All;
pp_opts.ModulesCheckRelocated = validate_pcm;

LOG_PRINTF(GetLog(LLDBLog::Types), "PCM validation is %s",
validate_pcm ? "disabled" : "enabled");
}

LOG_PRINTF(GetLog(LLDBLog::Types), "Importing dependencies of current CU");

// Turn off implicit clang modules while importing CU dependencies.
Expand Down
20 changes: 20 additions & 0 deletions lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4688,6 +4688,13 @@ static constexpr OptionEnumValueElement g_memory_module_load_level_values[] = {
},
};

static constexpr OptionEnumValueElement g_swift_pcm_validation_values[] = {
{llvm::to_underlying(AutoBool::Auto), "auto",
"Turned on when explicit modules are enabled"},
{llvm::to_underlying(AutoBool::True), "true", "Enable validation."},
{llvm::to_underlying(AutoBool::False), "false", "Disable validation."},
};


#define LLDB_PROPERTIES_target
#include "TargetProperties.inc"
Expand Down Expand Up @@ -4927,6 +4934,19 @@ bool TargetProperties::GetSwiftAllowExplicitModules() const {
return true;
}

AutoBool TargetProperties::GetSwiftPCMValidation() const {
const Property *exp_property =
m_collection_sp->GetPropertyAtIndex(ePropertyExperimental);
OptionValueProperties *exp_values =
exp_property->GetValue()->GetAsProperties();
if (exp_values)
return exp_values
->GetPropertyAtIndexAs<AutoBool>(ePropertySwiftPCMValidation)
.value_or(AutoBool::Auto);

return AutoBool::Auto;
}

Args TargetProperties::GetSwiftPluginServerForPath() const {
const uint32_t idx = ePropertySwiftPluginServerForPath;

Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Target/TargetProperties.td
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ let Definition = "target_experimental" in {
def SwiftAllowExplicitModules: Property<"swift-allow-explicit-modules", "Boolean">,
DefaultTrue,
Desc<"Allows explicit module flags to be passed through to ClangImporter.">;
def SwiftPCMValidation: Property<"swift-pcm-validation", "Enum">,
Global,
DefaultEnumValue<"llvm::to_underlying(AutoBool::Auto)">,
EnumValues<"OptionEnumValues(g_swift_pcm_validation_values)">,
Desc<"Enable validation when loading Clang PCM files (-fvalidate-pch, -fmodules-check-relocated).">;

Choose a reason for hiding this comment

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

This is from memory, but I believe pch validation gates the module relocation checks. In other words, disabling pch validation should make it unnecessary to explicitly disable module-check-relocated.

}

let Definition = "target" in {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ def test(self):
# CHECK-NOT: -fno-implicit-modules
# CHECK-NOT: -fno-implicit-module-maps
# CHECK: -DMARKER2
# CHECK: PCM validation is