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.

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

rewriter: support the 6 x86 param regs for post condition functions #471

Merged
merged 4 commits into from
Dec 18, 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
1 change: 1 addition & 0 deletions tests/post_condition/Output/dav1d.out
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
dav1d_get_picture post condition ran
negative stride
5 changes: 4 additions & 1 deletion tests/post_condition/dav1d.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ int dav1d_get_picture(Dav1dContext *const c, Dav1dPicture *const out) {
return 0;
}

void dav1d_get_picture_post_condition() {
void dav1d_get_picture_post_condition(Dav1dContext *const c, Dav1dPicture *const out) {
cr_log_info("dav1d_get_picture post condition ran");
if (out->stride[0] < 0) {
cr_log_info("negative stride");
}
}
42 changes: 32 additions & 10 deletions tools/rewriter/GenCallAsm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ static AsmWriter get_asmwriter(bool as_macro) {
return {.ss = {}, .terminator = terminator};
}

static void emit_prologue(AsmWriter &aw, uint32_t caller_pkey, uint32_t target_pkey, Arch arch) {
static void emit_prologue(AsmWriter &aw, uint32_t caller_pkey, uint32_t target_pkey, Arch arch, bool save_param_regs) {
if (arch == Arch::X86) {
// Save the old frame pointer and set the frame pointer for the call gate
add_asm_line(aw, "pushq %rbp");
Expand All @@ -464,6 +464,15 @@ static void emit_prologue(AsmWriter &aw, uint32_t caller_pkey, uint32_t target_p
for (auto &r : x86_preserved_registers) {
add_asm_line(aw, "pushq %"s + r);
}
if (save_param_regs) {
// Push first 6 registers for args onto the stack, too,
// so that we can store them for the post condition function.
add_comment_line(aw, "Save param regs for post condition call");
for (auto &r : x86_int_param_reg_order) {
add_asm_line(aw, "pushq %"s + r);
}
}
// TODO this for arm, too
kkysen marked this conversation as resolved.
Show resolved Hide resolved
} else if (arch == Arch::Aarch64) {
// Frame pointer and link register need to be saved first, to make backtraces work
add_asm_line(aw, "stp x29, x30, [sp, #-16]!");
Expand All @@ -485,6 +494,10 @@ static void emit_prologue(AsmWriter &aw, uint32_t caller_pkey, uint32_t target_p
// TODO(performance): We could store by pairs (STP)
add_asm_line(aw, "str "s + aarch64_preserved_registers[i] + ", [sp, #" + std::to_string(i * 8) + "]");
}

if (save_param_regs) {
llvm::report_fatal_error("--enable-dav1d_get_picture-post-condition is not yet supported on aarch64");
}
}
}

Expand Down Expand Up @@ -824,6 +837,13 @@ static void emit_set_return_pkru(AsmWriter &aw, uint32_t caller_pkey, Arch arch)

static void emit_post_condition_fn_call(AsmWriter &aw, Arch arch, std::string_view target_post_condition_name) {
llvm::errs() << "emitting post condition call to " << target_post_condition_name << "\n";
if (arch == Arch::X86) {
add_comment_line(aw, "Restore param regs for post condition call");
for (auto it = x86_int_param_reg_order.rbegin(); it != x86_int_param_reg_order.rend(); ++it) {
auto &r = *it;
add_asm_line(aw, "popq %"s + r);
}
}
add_comment_line(aw, "Align stack");
add_asm_line(aw, "subq $8, %rsp");
add_comment_line(aw, "Call post condition function");
Expand Down Expand Up @@ -998,6 +1018,14 @@ std::string emit_asm_wrapper(AbiSignature &sig,
stack_alignment = (compartment_stack_space + 8) % 16;
}

// For now, we hardcode the existence and name of the post-condition function.
// The name is `${target_name}_post_condition`,
// and we only do this for `dav1d_get_picture`.
std::optional<std::string> target_post_condition_name = std::nullopt;
if (enable_dav1d_get_picture_post_condition && target_name && *target_name == "dav1d_get_picture") {
target_post_condition_name = *target_name + "_post_condition";
}

add_comment_line(aw, "Wrapper for "s + sig_string(sig, target_name) + ":");
add_asm_line(aw, ".text");
if (kind != WrapperKind::PointerToStatic) {
Expand All @@ -1009,7 +1037,9 @@ std::string emit_asm_wrapper(AbiSignature &sig,
add_asm_line(aw, ".type "s + wrapper_name + ", @function");
add_asm_line(aw, wrapper_name + ":");

emit_prologue(aw, caller_pkey, target_pkey, arch);
// Note that when there's a post condition call,
// the prologue needs to undo the `pushq`s added in `emit_prologue` to save the register params.
emit_prologue(aw, caller_pkey, target_pkey, arch, target_post_condition_name.has_value());
kkysen marked this conversation as resolved.
Show resolved Hide resolved

if (arch == Arch::X86) {
add_raw_line(aw, llvm::formatv("ASSERT_PKRU({0:x8}) \"\\n\"", ~((0b11 << (2 * caller_pkey)) | 0b11)));
Expand Down Expand Up @@ -1049,14 +1079,6 @@ std::string emit_asm_wrapper(AbiSignature &sig,

// Call the post-condition function, if one was specified.
// The call happens in the caller's compartment.
// For now, we hardcode the existence and name of the post-condition function.
// The name is `${target_name}_post_condition`,
// and we only do this for `dav1d_get_picture`.
std::optional<std::string> target_post_condition_name = std::nullopt;
if (enable_dav1d_get_picture_post_condition && target_name && *target_name == "dav1d_get_picture") {
target_post_condition_name = *target_name + "_post_condition";
}

if (target_post_condition_name) {
emit_post_condition_fn_call(aw, arch, *target_post_condition_name);
}
Expand Down
Loading