Skip to content

Rtsan/blocking 2 llvm pass #35

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

Closed
wants to merge 2 commits into from
Closed

Rtsan/blocking 2 llvm pass #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef470bb
[LLVM][rtsan] Add LLVM attribute sanitize_realtime_unsafe
davidtrevelyan Aug 30, 2024
eebda69
Implement sanitize_realtime_unsafe Pass
davidtrevelyan Aug 30, 2024
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
5 changes: 5 additions & 0 deletions llvm/docs/LangRef.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,6 +2320,11 @@ example:
(realtime safety analysis - no allocations, syscalls or exceptions) are enabled
for this function.
This attribute is incompatible with the ``nosanitize_realtime`` attribute.
``sanitize_realtime_unsafe``
This attribute indicates that RealtimeSanitizer should error immediately
if the attributed function is called during invocation of a function
attributed with ``sanitize_realtime``.
This attribute is incompatible with the ``sanitize_realtime`` attribute.
``speculative_load_hardening``
This attribute indicates that
`Speculative Load Hardening <https://llvm.org/docs/SpeculativeLoadHardening.html>`_
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/Bitcode/LLVMBitCodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ enum AttributeKindCodes {
ATTR_KIND_HYBRID_PATCHABLE = 95,
ATTR_KIND_SANITIZE_REALTIME = 96,
ATTR_KIND_NO_SANITIZE_REALTIME = 97,
ATTR_KIND_SANITIZE_REALTIME_UNSAFE = 98,
};

enum ComdatSelectionKindCodes {
Expand Down
5 changes: 5 additions & 0 deletions llvm/include/llvm/IR/Attributes.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ def SanitizeNumericalStability : EnumAttr<"sanitize_numerical_stability", [FnAtt
/// RealtimeSanitizer is on.
def SanitizeRealtime : EnumAttr<"sanitize_realtime", [FnAttr]>;

/// RealtimeSanitizer should error if an realtime_unsafe function is called
/// during a sanitize_realtime function.
def SanitizeRealtimeUnsafe : EnumAttr<"sanitize_realtime_unsafe", [FnAttr]>;

/// Speculative Load Hardening is enabled.
///
/// Note that this uses the default compatibility (always compatible during
Expand Down Expand Up @@ -392,6 +396,7 @@ def : CompatRule<"isEqual<SanitizeHWAddressAttr>">;
def : CompatRule<"isEqual<SanitizeMemTagAttr>">;
def : CompatRule<"isEqual<SanitizeNumericalStabilityAttr>">;
def : CompatRule<"isEqual<SanitizeRealtimeAttr>">;
def : CompatRule<"isEqual<SanitizeRealtimeUnsafeAttr>">;
def : CompatRule<"isEqual<SafeStackAttr>">;
def : CompatRule<"isEqual<ShadowCallStackAttr>">;
def : CompatRule<"isEqual<UseSampleProfileAttr>">;
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2145,6 +2145,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
return Attribute::SanitizeNumericalStability;
case bitc::ATTR_KIND_SANITIZE_REALTIME:
return Attribute::SanitizeRealtime;
case bitc::ATTR_KIND_SANITIZE_REALTIME_UNSAFE:
return Attribute::SanitizeRealtimeUnsafe;
case bitc::ATTR_KIND_SPECULATIVE_LOAD_HARDENING:
return Attribute::SpeculativeLoadHardening;
case bitc::ATTR_KIND_SWIFT_ERROR:
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
return bitc::ATTR_KIND_SANITIZE_NUMERICAL_STABILITY;
case Attribute::SanitizeRealtime:
return bitc::ATTR_KIND_SANITIZE_REALTIME;
case Attribute::SanitizeRealtimeUnsafe:
return bitc::ATTR_KIND_SANITIZE_REALTIME_UNSAFE;
case Attribute::SpeculativeLoadHardening:
return bitc::ATTR_KIND_SPECULATIVE_LOAD_HARDENING;
case Attribute::SwiftError:
Expand Down
28 changes: 25 additions & 3 deletions llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ static void insertCallAtAllFunctionExitPoints(Function &Fn,
insertCallBeforeInstruction(Fn, I, InsertFnName);
}

static PreservedAnalyses rtsanPreservedAnalyses() {
PreservedAnalyses PA;
PA.preserveSet<CFGAnalyses>();
return PA;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would get rid of this function and leave these explicit. I think the loop changing we will do will not preserve CFG, so I think it is OK to just do this explicitly in our main block

Copy link
Author

Choose a reason for hiding this comment

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

Hmm yeah - part of my refactoring in the main pass function was intended to have a sequence of:

if (condition) {
    return transformXYZ(...);
}
else if (condition) {
    return transformABC(...);
}

... to express the intent that these transformations are mutually exclusive and return early. But now I'm thinking about it, it doesn't really need to be that way - and it's a bit early days to start those sorts of refactors. So yeah, I think I'll revert this as you suggest

Copy link
Author

Choose a reason for hiding this comment

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

On second thoughts, we still need it in two places. I've updated the function name to be more explicit, though - you'll see this in my latest PR. When we no longer need it as part of the sanitize_realtime pass, we can inline it then. I think it's better to keep it in this case, don't you?

}

static void transformRealtimeUnsafeFunction(Function &F) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit (take it or leave it):

Transform is a little vague, could be more descriptive as to the actual transformation taking place

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, agreed

IRBuilder<> Builder(&F.front().front());
Value *NameArg = Builder.CreateGlobalString(F.getName());

FunctionType *FuncType =
FunctionType::get(Type::getVoidTy(F.getContext()),
{PointerType::getUnqual(F.getContext())}, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should call reviewer eyes to this code when we post this so we make sure it is correct (not saying it isn't just that I have no clue about this code)


FunctionCallee Func = F.getParent()->getOrInsertFunction(
"__rtsan_expect_not_realtime", FuncType);

Builder.CreateCall(Func, {NameArg});
}

RealtimeSanitizerPass::RealtimeSanitizerPass(
const RealtimeSanitizerOptions &Options) {}

Expand All @@ -53,10 +73,12 @@ PreservedAnalyses RealtimeSanitizerPass::run(Function &F,
if (F.hasFnAttribute(Attribute::SanitizeRealtime)) {
insertCallAtFunctionEntryPoint(F, "__rtsan_realtime_enter");
insertCallAtAllFunctionExitPoints(F, "__rtsan_realtime_exit");
return rtsanPreservedAnalyses();
}

PreservedAnalyses PA;
PA.preserveSet<CFGAnalyses>();
return PA;
if (F.hasFnAttribute(Attribute::SanitizeRealtimeUnsafe)) {
transformRealtimeUnsafeFunction(F);
return rtsanPreservedAnalyses();
}

return PreservedAnalyses::all();
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Utils/CodeExtractor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,7 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs,
case Attribute::SanitizeHWAddress:
case Attribute::SanitizeMemTag:
case Attribute::SanitizeRealtime:
case Attribute::SanitizeRealtimeUnsafe:
case Attribute::SpeculativeLoadHardening:
case Attribute::StackProtect:
case Attribute::StackProtectReq:
Expand Down
6 changes: 6 additions & 0 deletions llvm/test/Bitcode/attributes.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,11 @@ define void @f93() nosanitize_realtime
ret void;
}

; CHECK: define void @f94() #55
define void @f94() sanitize_realtime_unsafe {
ret void;
}

; CHECK: define void @f87() [[FNRETTHUNKEXTERN:#[0-9]+]]
define void @f87() fn_ret_thunk_extern { ret void }

Expand Down Expand Up @@ -613,6 +618,7 @@ define void @initializes(ptr initializes((-4, 0), (4, 8)) %a) {
; CHECK: attributes #52 = { nosanitize_bounds }
; CHECK: attributes #53 = { sanitize_realtime }
; CHECK: attributes #54 = { nosanitize_realtime }
; CHECK: attributes #55 = { sanitize_realtime_unsafe }
; CHECK: attributes [[FNRETTHUNKEXTERN]] = { fn_ret_thunk_extern }
; CHECK: attributes [[SKIPPROFILE]] = { skipprofile }
; CHECK: attributes [[OPTDEBUG]] = { optdebug }
Expand Down
12 changes: 8 additions & 4 deletions llvm/test/Bitcode/compatibility.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,7 @@ exit:
; CHECK: select <2 x i1> <i1 true, i1 false>, <2 x i8> <i8 2, i8 3>, <2 x i8> <i8 3, i8 2>

call void @f.nobuiltin() builtin
; CHECK: call void @f.nobuiltin() #54
; CHECK: call void @f.nobuiltin() #55

call fastcc noalias ptr @f.noalias() noinline
; CHECK: call fastcc noalias ptr @f.noalias() #12
Expand Down Expand Up @@ -1992,8 +1992,11 @@ declare void @f.sanitize_numerical_stability() sanitize_numerical_stability
declare void @f.sanitize_realtime() sanitize_realtime
; CHECK: declare void @f.sanitize_realtime() #52

declare void @f.sanitize_realtime_unsafe() sanitize_realtime_unsafe
; CHECK: declare void @f.sanitize_realtime_unsafe() #53

declare void @f.nosanitize_realtime() nosanitize_realtime
; CHECK: declare void @f.nosanitize_realtime() #53
; CHECK: declare void @f.nosanitize_realtime() #54

; CHECK: declare nofpclass(snan) float @nofpclass_snan(float nofpclass(snan))
declare nofpclass(snan) float @nofpclass_snan(float nofpclass(snan))
Expand Down Expand Up @@ -2118,8 +2121,9 @@ define float @nofpclass_callsites(float %arg) {
; CHECK: attributes #50 = { allockind("alloc,uninitialized") }
; CHECK: attributes #51 = { sanitize_numerical_stability }
; CHECK: attributes #52 = { sanitize_realtime }
; CHECK: attributes #53 = { nosanitize_realtime }
; CHECK: attributes #54 = { builtin }
; CHECK: attributes #53 = { sanitize_realtime_unsafe }
; CHECK: attributes #54 = { nosanitize_realtime }
; CHECK: attributes #55 = { builtin }

;; Metadata

Expand Down
16 changes: 16 additions & 0 deletions llvm/test/Instrumentation/RealtimeSanitizer/rtsan_unsafe.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
; RUN: opt < %s -passes=rtsan -S | FileCheck %s

define void @blocking_function() #0 {
ret void
}

define noundef i32 @main() #2 {
call void @blocking_function() #4
ret i32 0
}

attributes #0 = { mustprogress noinline sanitize_realtime_unsafe optnone ssp uwtable(sync) }

; RealtimeSanitizer pass should insert __rtsan_expect_not_realtime at function entrypoint
; CHECK-LABEL: @blocking_function()
; CHECK-NEXT: call{{.*}}@__rtsan_expect_not_realtime({{ptr .*}})
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice one!