Skip to content

[lldb] Add setting to override ClangImporter driver options #8682

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
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 @@ -174,6 +174,8 @@ class TargetProperties : public Properties {

llvm::StringRef GetSwiftExtraClangFlags() const;

llvm::StringRef GetSwiftClangOverrideOptions() const;

bool GetSwiftReadMetadataFromFileCache() const;

bool GetSwiftUseReflectionSymbols() const;
Expand Down
76 changes: 72 additions & 4 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "Plugins/TypeSystem/Swift/StoringDiagnosticConsumer.h"
#include "Plugins/ExpressionParser/Swift/SwiftPersistentExpressionState.h"

#include "lldb/Utility/Log.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/ASTDemangler.h"
#include "swift/AST/ASTMangler.h"
Expand Down Expand Up @@ -58,6 +59,7 @@

#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
Expand Down Expand Up @@ -1672,9 +1674,72 @@ void RemoveExplicitModules(std::vector<std::string> &args) {

} // namespace

void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string> &ExtraArgs) {
/// LLDB wrapper for `clang::driver::applyOverrideOptions` (which implements
/// CCC_OVERRIDE_OPTIONS behavior).
static void applyOverrideOptions(std::vector<std::string> &args,
llvm::StringRef overrideOpts) {
if (overrideOpts.empty())
return;

// Convert input args to the type required by applyOverrideOptions.
llvm::SmallVector<const char *, 64> raw_args;
// Add placeholder clang executable, which applyOverrideOptions expects to be
// the first argument.
raw_args.push_back("clang");
for (const std::string &arg : args)
raw_args.push_back(arg.data());

Choose a reason for hiding this comment

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

I find this very scary because we don't know if args contents is nullterminated and it's easy to break this contract in the future.
Could we just make this a std::vector<std::string>?

Copy link
Author

Choose a reason for hiding this comment

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

the type of args is std::vector<std::string>.

If you are referring to raw_args and its type llvm::SmallVector<const char *, 64>, note that it's used because that's the signature of clang::driver::applyOverrideOptions.

Choose a reason for hiding this comment

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

Oh, I see!
Is the null-terminator guaranteed, or should this be arg.c_str()?

Copy link
Author

Choose a reason for hiding this comment

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

it's guaranteed to be null terminated:

The returned array is null-terminated, that is, data() and c_str() perform the same function. (since C++11)

https://en.cppreference.com/w/cpp/string/basic_string/data


/// LLVM stream backed by a callback. This is used to redirect
/// applyOverrideOptions logging to LLDB.
struct CallbackStream : public llvm::raw_ostream {
using callback_t = std::function<void(const char *, size_t)>;
callback_t m_callback;
uint64_t m_pos = 0;

CallbackStream(callback_t callback) : m_callback(callback) {}
~CallbackStream() override { flush(); }

void write_impl(const char *Ptr, size_t Size) override {
m_callback(Ptr, Size);
m_pos += Size;
}

uint64_t current_pos() const override { return m_pos; }
};

// Perform the override operations.
llvm::StringSet<> savedStrings;
auto *log = GetLog(LLDBLog::Types);
CallbackStream log_stream{[log](const char *Ptr, size_t Size) {
if (!log)
return;
if (Ptr[Size] == '\n')
// Skip the newline because LLDB logging writes a newline.
Size--;
log->PutString({Ptr, Size});
}};

clang::driver::applyOverrideOptions(raw_args, overrideOpts.data(),
savedStrings, &log_stream);

// Delete the placeholder "clang" executable argument.
raw_args.erase(raw_args.begin());

// Copy `raw_args` into a new args vector.
std::vector<std::string> new_args;
for (const char *arg : raw_args)
new_args.emplace_back(arg);

// Only now that `raw_args` has been copied into `new_args`, it's safe to
// overwrite `args` (which owns the data pointed to by `raw_args`).
args = new_args;
}

void SwiftASTContext::AddExtraClangArgs(
const std::vector<std::string> &ExtraArgs, StringRef overrideOpts) {
swift::ClangImporterOptions &importer_options = GetClangImporterOptions();
AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs);
applyOverrideOptions(importer_options.ExtraArgs, overrideOpts);
if (HasNonexistentExplicitModule(importer_options.ExtraArgs))
RemoveExplicitModules(importer_options.ExtraArgs);
}
Expand Down Expand Up @@ -2105,7 +2170,8 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
// Apply the working directory to all relative paths.
std::vector<std::string> DeserializedArgs = swift_ast_sp->GetClangArguments();
swift_ast_sp->GetClangImporterOptions().ExtraArgs.clear();
swift_ast_sp->AddExtraClangArgs(DeserializedArgs);
StringRef overrideOpts = target ? target->GetSwiftClangOverrideOptions() : "";
swift_ast_sp->AddExtraClangArgs(DeserializedArgs, overrideOpts);
if (target)
swift_ast_sp->AddUserClangArgs(*target);
else
Expand Down Expand Up @@ -2628,7 +2694,8 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
use_all_compiler_flags, module_filter, target, triple,
plugin_search_options, module_search_paths,
framework_search_paths, extra_clang_args);
swift_ast_sp->AddExtraClangArgs(extra_clang_args);
swift_ast_sp->AddExtraClangArgs(extra_clang_args,
target.GetSwiftClangOverrideOptions());
}

for (const FileSpec &path : target.GetSwiftModuleSearchPaths())
Expand Down Expand Up @@ -2940,7 +3007,8 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
use_all_compiler_flags, module_filter, target, triple,
plugin_search_options, module_search_paths,
framework_search_paths, extra_clang_args);
swift_ast_sp->AddExtraClangArgs(extra_clang_args);
swift_ast_sp->AddExtraClangArgs(extra_clang_args,
target.GetSwiftClangOverrideOptions());
}

// Now fold any extra options we were passed. This has to be done
Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ class SwiftASTContext : public TypeSystemSwift {

/// Add a list of Clang arguments to the ClangImporter options and
/// apply the working directory to any relative paths.
void AddExtraClangArgs(const std::vector<std::string> &ExtraArgs);
void AddExtraClangArgs(const std::vector<std::string> &ExtraArgs,
llvm::StringRef overrideOpts = "");
static void AddExtraClangArgs(const std::vector<std::string>& source,
std::vector<std::string>& dest);
static std::string GetPluginServer(llvm::StringRef plugin_library_path);
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5224,6 +5224,11 @@ llvm::StringRef TargetProperties::GetSwiftExtraClangFlags() const {
return GetPropertyAtIndexAs<llvm::StringRef>(idx, "");
}

llvm::StringRef TargetProperties::GetSwiftClangOverrideOptions() const {
const uint32_t idx = ePropertySwiftClangOverrideOptions;
return GetPropertyAtIndexAs<llvm::StringRef>(idx, "");
}

FileSpecList TargetProperties::GetClangModuleSearchPaths() {
const uint32_t idx = ePropertyClangModuleSearchPaths;
return GetPropertyAtIndexAs<FileSpecList>(idx, {});
Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Target/TargetProperties.td
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ let Definition = "target" in {
def SwiftExtraClangFlags: Property<"swift-extra-clang-flags", "String">,
DefaultStringValue<"">,
Desc<"Additional -Xcc flags to be passed to the Swift ClangImporter.">;
def SwiftClangOverrideOptions: Property<"swift-clang-override-options", "String">,
DefaultStringValue<"">,
Desc<"Specify CCC_OVERRIDE_OPTIONS for ClangImporter flags.">;
def SwiftAutoImportFrameworks : Property<"swift-auto-import-frameworks", "Boolean">,
DefaultFalse,
Desc<"Automatically import all frameworks and dynamic libraries that are autolinked by Swift modules in the target.">;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
SWIFT_SOURCES := main.swift
SWIFTFLAGS_EXTRAS = -Xcc -DDELETEME=1

include Makefile.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import lldb
from lldbsuite.test.decorators import *
import lldbsuite.test.lldbtest as lldbtest
import lldbsuite.test.lldbutil as lldbutil


class TestCase(lldbtest.TestBase):
@swiftTest
@skipUnlessFoundation
def test(self):
"""Check that ClangImporter options can be overridden."""
self.build()

log = self.getBuildArtifact("lldb.log")
self.runCmd(f"log enable lldb types -f '{log}'")
self.runCmd("settings set target.swift-clang-override-options x-DDELETEME=1")

lldbutil.run_to_name_breakpoint(self, "main", bkpt_module="a.out")
self.expect("expression 1")

self.filecheck(f"platform shell cat {log}", __file__)
# CHECK: CCC_OVERRIDE_OPTIONS: x-DDELETEME=1
# CHECK: Deleting argument -DDELETEME=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import Foundation

// empty main