Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add -fcf-protection. #4437

Merged
merged 7 commits into from
Jul 21, 2023
Merged

Add -fcf-protection. #4437

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f3eff0c
Add -fcf-protection.
JohanEngelen Jul 11, 2023
e452d59
fix changelog
JohanEngelen Jul 11, 2023
f756c1c
fixup
JohanEngelen Jul 11, 2023
c015a00
Replace versions with getTargetInfo("CET")
JohanEngelen Jul 12, 2023
f511013
fixup changelog
JohanEngelen Jul 12, 2023
c0a3904
error on non-x86 archs
JohanEngelen Jul 12, 2023
81dd27d
fixup
JohanEngelen Jul 15, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- C files are now automatically preprocessed using the external C compiler (configurable via `-gcc` or the `CC` environment variable, and `-Xcc` for extra flags). Extra preprocessor flags (e.g., include dirs and manual defines) can be added via new command-line option `-P`. (#4417)
- Windows: If `clang-cl.exe` is on `PATH`, it is preferred over Microsoft's `cl.exe` by default (e.g., to avoid printing the C source file name to stderr during preprocessing).
- Less pedantic checks for conflicting C(++) function declarations when compiling multiple modules to a single object file ('Error: Function type does not match previously declared function with the same mangled name'). The error now only appears if an object file actually references multiple conflicting functions. (#4420)
- New command-line option `--fcf-protection`. (#4437)
JohanEngelen marked this conversation as resolved.
Show resolved Hide resolved

#### Platform support

Expand Down
13 changes: 13 additions & 0 deletions driver/cl_options_instrumentation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ llvm::StringRef getXRayInstructionThresholdString() {
return thresholdString;
}

cl::opt<CFProtectionType> fCFProtection(
"fcf-protection",
cl::desc("Instrument control-flow architecture protection"), cl::ZeroOrMore,
cl::ValueOptional,
cl::values(clEnumValN(CFProtectionType::None, "none", ""),
clEnumValN(CFProtectionType::Branch, "branch", ""),
clEnumValN(CFProtectionType::Return, "return", ""),
clEnumValN(CFProtectionType::Full, "full", ""),
clEnumValN(CFProtectionType::Full, "",
"") // default to "full" if no argument specified
),
cl::init(CFProtectionType::None));

void initializeInstrumentationOptionsFromCmdline(const llvm::Triple &triple) {
if (ASTPGOInstrGenFile.getNumOccurrences() > 0) {
pgoMode = PGO_ASTBasedInstr;
Expand Down
3 changes: 3 additions & 0 deletions driver/cl_options_instrumentation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ extern cl::opt<bool> instrumentFunctions;
extern cl::opt<bool> fXRayInstrument;
llvm::StringRef getXRayInstructionThresholdString();

enum class CFProtectionType { None, Branch, Return, Full };
extern cl::opt<CFProtectionType> fCFProtection;

/// This initializes the instrumentation options, and checks the validity of the
/// commandline flags. targetTriple should be initialized before calling this.
/// It should be called only once.
Expand Down
14 changes: 14 additions & 0 deletions driver/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,20 @@ void registerPredefinedVersions() {
VersionCondition::addPredefinedGlobalIdent("LDC_ThreadSanitizer");
}

switch (opts::fCFProtection) {
case opts::CFProtectionType::Branch:
VersionCondition::addPredefinedGlobalIdent("__CET_1__");
Copy link
Contributor

Choose a reason for hiding this comment

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

if you wanted to add a enum instead of a bunch of version there is __traits(getInfo, "string_key")

Copy link
Member

Choose a reason for hiding this comment

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

__traits(getTargetInfo, "key"), might be a good idea. clang's __CET__ can be used as bitmask, whereas checking for branch-protection with the current versions would probably be something ugly like

version (__CET_1__) version = BranchCFProtection;
version (__CET_3__) version = BranchCFProtection;
version (BranchCFProtection) …

Copy link
Contributor

Choose a reason for hiding this comment

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

getTargetInfo, that was what I meant to say. Also it is not immediately obvious what __CET_n__ refer to. I agree that adding VersionCondition::addPredefinedGlobalIdent("LDC_BranchCFProtection"); is much better.
(as an aside, does GCC/GDC have something similar? if so we should try to have a common interface "D_BranchControlFlowProtection"? @ibuclaw).

Copy link

Choose a reason for hiding this comment

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

as an aside, does GCC/GDC have something similar?

GDC supports CET but I don't think it sets any predefined version

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the help here :)
__traits(getTargetInfo, "__CET__") for similarity with C ? Or without the underscores?
No version needed then, static if (__traits(getTargetInfo, "key") > 0) would suffice. I'm thinking that __CET__ is very rarely needed; the only usecase I can think of is writing assembly (I see in cet.h the bitmasking usecase the Martin mentioned).

Copy link
Contributor

Choose a reason for hiding this comment

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

as an aside, does GCC/GDC have something similar?

GDC supports CET but I don't think it sets any predefined version

Correct. The druntime library is compiled with -fversion=CET because I've not ported fibers to work with both branch protection or shadow stack.

I gather otherwise the end user shouldn't really care about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the help here :)
__traits(getTargetInfo, "__CET__") for similarity with C ? Or without the underscores?
No version needed then, static if (__traits(getTargetInfo, "key") > 0) would suffice.

BTW, I went with -fversion because the code that really needs to be aware of it in druntime (asm Fibers) is a bunch of nested version conditions and declarations.

break;
case opts::CFProtectionType::Return:
VersionCondition::addPredefinedGlobalIdent("__CET_2__");
break;
case opts::CFProtectionType::Full:
VersionCondition::addPredefinedGlobalIdent("__CET_3__");
break;
case opts::CFProtectionType::None:
break;
}

#if LDC_LLVM_VER >= 1400
// A version identifier for whether opaque pointers are enabled or not. (needed e.g. for intrinsic mangling)
if (!getGlobalContext().supportsTypedPointers()) {
Expand Down
21 changes: 21 additions & 0 deletions gen/modules.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,27 @@ void registerModuleInfo(Module *m) {
emitModuleRefToSection(mangle, moduleInfoSym);
}
}

void addModuleFlags(llvm::Module &m) {
#if LDC_LLVM_VER >= 1500
const auto ModuleMinFlag = llvm::Module::Min;
#else
const auto ModuleMinFlag = llvm::Module::Warning; // Fallback value
#endif

if (opts::fCFProtection == opts::CFProtectionType::Return ||
opts::fCFProtection == opts::CFProtectionType::Full) {
m.addModuleFlag(ModuleMinFlag, "cf-protection-return", 1);
}

if (opts::fCFProtection == opts::CFProtectionType::Branch ||
opts::fCFProtection == opts::CFProtectionType::Full) {
m.addModuleFlag(ModuleMinFlag, "cf-protection-branch", 1);
}
}

} // anonymous namespace

void codegenModule(IRState *irs, Module *m) {
TimeTraceScope timeScope("Generate IR", m->toChars(), m->loc);

Expand Down Expand Up @@ -444,6 +463,8 @@ void codegenModule(IRState *irs, Module *m) {
addCoverageAnalysisInitializer(m);
}

addModuleFlags(irs->module);

gIR = nullptr;
irs->dmodule = nullptr;
}
28 changes: 28 additions & 0 deletions tests/codegen/fcf_protection.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Test -fcf-protection

// REQUIRES: target_X86

// RUN: %ldc -mtriple=x86_64-linux-gnu -output-ll -of=%t.ll %s && FileCheck %s --check-prefix=NOTHING < %t.ll

// RUN: %ldc -mtriple=x86_64-linux-gnu -output-ll -of=%t_branch.ll %s --fcf-protection=branch -d-version=BRANCH && FileCheck %s --check-prefix=BRANCH < %t_branch.ll
// RUN: %ldc -mtriple=x86_64-linux-gnu -output-ll -of=%t_return.ll %s --fcf-protection=return -d-version=RETURN && FileCheck %s --check-prefix=RETURN < %t_return.ll
// RUN: %ldc -mtriple=x86_64-linux-gnu -output-ll -of=%t_full.ll %s --fcf-protection=full -d-version=FULL && FileCheck %s --check-prefix=FULL < %t_full.ll
// RUN: %ldc -mtriple=x86_64-linux-gnu -output-ll -of=%t_noarg.ll %s --fcf-protection -d-version=FULL && FileCheck %s --check-prefix=FULL < %t_noarg.ll

// NOTHING-NOT: cf-prot
// BRANCH-DAG: "cf-protection-branch", i32 1
// RETURN-DAG: "cf-protection-return", i32 1
// FULL-DAG: "cf-protection-branch", i32 1
// FULL-DAG: "cf-protection-return", i32 1

void foo() {}

version(BRANCH) {
version(__CET_1__) {} else { static assert(false); };
}
version(RETURN) {
version(__CET_2__) {} else { static assert(false); };
}
version(FULL) {
version(__CET_3__) {} else { static assert(false); };
}