Skip to content
This repository was archived by the owner on Jan 16, 2026. It is now read-only.

Fix store log and more #21

Open
AlonB644 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix store log and more #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ REPORT_WITH_OFFSET ?= 0
ENABLE_COVERAGE ?= 1
COVERAGE_MEMORY_ERRORS ?= 1
COVERAGE_CONTROL_FLOW_ERRORS ?= 1
SEED_NON_SPECULATIVE_ERRORS ?= 1
Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Sep 29, 2020

Choose a reason for hiding this comment

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

Our implicit assumption is that the program we fuzz does not include any non-speculative bugs. It doesn't make much sense to fuzz for both speculative and conventional memory violations simultaneously: They are way too different and, I believe, tackling both would make the code base too complicated.

Anyway, what was the reason you decided to add it? Did you detect any bugs in your benchmarks?

Copy link
Author

@AlonB644 AlonB644 Oct 10, 2020

Choose a reason for hiding this comment

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

Seeding on unexpected error gives you the best-case scenario.
If the program has bugs so you can run the native version on the input and verify it.
If the fuzzing is buggy then you saved the right input to debug it.

My reason was the latter. For traceability of bugs during development.

Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Oct 11, 2020

Choose a reason for hiding this comment

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

Got it. In this case, could you rename this variable to something more descriptive? Something like STORE_INPUT_ON_SPECFUZZ_FAILURE

ENABLE_SANITY_CHECKS ?= 1
ENABLE_STATS ?= 0
ENABLE_SEQUENTIAL_SIMULATION ?= 0
Expand Down
6 changes: 6 additions & 0 deletions install/patches/honggfuzz/instrument.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ void specfuzz_cov_vuln(uintptr_t pc) {
}
}

// Adds current input to corpus
void specfuzz_seed_input() {
// TODO: less lazy implementation
ATOMIC_PRE_INC_RELAXED(feedback->pidFeedbackPc[my_thread_no]);
}

__attribute__((preserve_most))
void specfuzz_cov_trace_pc(uintptr_t pc) {
// quick path - check the cache
Expand Down
146 changes: 138 additions & 8 deletions src/SpecFuzzPass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,24 +633,150 @@ auto X86SpecFuzzPass::visitWrite(MachineInstr &MI, MachineBasicBlock &Parent) ->
.add(MI.getOperand(MemRefBegin + X86::AddrSegmentReg));

restoreRegister(Parent, MI, Loc, X86::RSP, "checkpoint_sp");


MachineMemOperand *MMO = *MI.memoperands_begin();
uint64_t width = MMO->getSize();

LLVM_DEBUG(dbgs() << "Store's width: " << width << "\n");

BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm((width > 8)? 8 : width);

// PUSH %TmpReg
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64r), TmpReg);

// PUSH (%TmpReg)
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64rmm), TmpReg)
.addImm(1).addReg(0)
.addImm(0).addReg(0);

Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Sep 30, 2020

Choose a reason for hiding this comment

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

That's SIMD support, isn't it? Could you add a comment with an explanation of what's happening here? Also, it looks like it could be moved into a separate function.


switch (width) {
case 1:
preserveRegister(Parent, MI, Loc, X86::R14, "tmp_gpr2");

// Immediate is arbitrary
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(0);

BuildMI(Parent, MI, Loc, TII->get(X86::MOV8rm), X86::R14B)
.addReg(TmpReg).addImm(1)
.addReg(0).addImm(0)
.addReg(0);

BuildMI(Parent, MI, Loc, TII->get(X86::MOV8mr))
.addReg(X86::RSP).addImm(1)
.addReg(0).addImm(0)
.addReg(0)
.addReg(X86::R14B);

restoreRegister(Parent, MI, Loc, X86::R14, "tmp_gpr2");

break;

case 2:
preserveRegister(Parent, MI, Loc, X86::R14, "tmp_gpr2");

// Immediate is arbitrary
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(0);

BuildMI(Parent, MI, Loc, TII->get(X86::MOV16rm), X86::R14W)
.addReg(TmpReg).addImm(1)
.addReg(0).addImm(0)
.addReg(0);

BuildMI(Parent, MI, Loc, TII->get(X86::MOV16mr))
.addReg(X86::RSP).addImm(1)
.addReg(0).addImm(0)
.addReg(0)
.addReg(X86::R14W);

restoreRegister(Parent, MI, Loc, X86::R14, "tmp_gpr2");

break;

case 4:
preserveRegister(Parent, MI, Loc, X86::R14, "tmp_gpr2");

// Immediate is arbitrary
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(0);

BuildMI(Parent, MI, Loc, TII->get(X86::MOV32rm), X86::R14D)
.addReg(TmpReg).addImm(1)
.addReg(0).addImm(0)
.addReg(0);

BuildMI(Parent, MI, Loc, TII->get(X86::MOV32mr))
.addReg(X86::RSP).addImm(1)
.addReg(0).addImm(0)
.addReg(0)
.addReg(X86::R14D);

restoreRegister(Parent, MI, Loc, X86::R14, "tmp_gpr2");

break;

case 8:
case 16:
case 32:
// PUSH (%TmpReg)
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64rmm), TmpReg)
.addImm(1).addReg(0)
.addImm(0).addReg(0);

if (width == 8) break;

BuildMI(Parent, MI, Loc, TII->get(X86::LEA64r), TmpReg)
.addReg(TmpReg).addImm(1)
.addReg(0).addImm(8)
.addReg(0);

BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(8);

// PUSH %TmpReg
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64r), TmpReg);

// PUSH (%TmpReg)
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64rmm), TmpReg)
.addImm(1).addReg(0)
.addImm(0).addReg(0);

if (width == 16) { LLVM_DEBUG(dbgs() << " The store is 128-bit wide\n"); break; }

BuildMI(Parent, MI, Loc, TII->get(X86::LEA64r), TmpReg)
.addReg(TmpReg).addImm(1)
.addReg(0).addImm(8)
.addReg(0);

BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(8);

// PUSH %TmpReg
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64r), TmpReg);

// PUSH (%TmpReg)
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64rmm), TmpReg)
.addImm(1).addReg(0)
.addImm(0).addReg(0);

LLVM_DEBUG(dbgs() << " The store is 256-bit wide\n");
break;

default:
llvm_unreachable("Unknown width");
break;
}

// SSE stores are 128-bit wide
if (Desc.TSFlags >> X86II::SSEDomainShift & 3) { // NOLINT
/*if (Desc.TSFlags >> X86II::SSEDomainShift & 3) { // NOLINT
LLVM_DEBUG(dbgs() << " The store is 128-bit wide\n");

// LEAQ 8(%TmpReg), %TmpReg
BuildMI(Parent, MI, Loc, TII->get(X86::LEA64r), TmpReg)
.addReg(TmpReg).addImm(1)
.addReg(0).addImm(8)
.addReg(0);

BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(8);

// PUSH %TmpReg
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64r), TmpReg);
Expand All @@ -659,7 +785,7 @@ auto X86SpecFuzzPass::visitWrite(MachineInstr &MI, MachineBasicBlock &Parent) ->
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64rmm), TmpReg)
.addImm(1).addReg(0)
.addImm(0).addReg(0);
}
}*/

preserveRegister(Parent, MI, Loc, X86::RSP, "checkpoint_sp");
restoreRegister(Parent, MI, Loc, TmpReg, "tmp_gpr1");
Expand All @@ -685,6 +811,9 @@ auto X86SpecFuzzPass::visitPush(MachineInstr &MI, MachineBasicBlock &Parent) ->

restoreRegister(Parent, MI, Loc, X86::RSP, "checkpoint_sp");

BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64i8))
.addImm(8);

// PUSH %TmpReg
BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64r), TmpReg);

Expand Down Expand Up @@ -1042,6 +1171,7 @@ auto X86SpecFuzzPass::getCallTargetType(MachineInstr &MI) -> X86SpecFuzzPass::Ca
"__asan_set_shadow_f8",

"__asan_frame_malloc_0",
"__asan_stack_malloc_0",
"__asan_stack_malloc_1",
"__asan_stack_malloc_2",
"__asan_stack_malloc_3",
Expand Down
22 changes: 22 additions & 0 deletions src/specfuzz_init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,48 @@ void specfuzz_handler(int signo, siginfo_t *siginfo, void *ucontext) {
#if ENABLE_SANITY_CHECKS == 1
if (inside_handler != 0) {
fprintf(stderr, "\n[SF] Error: Fault inside the signal handler\n");
#if SEED_NON_SPECULATIVE_ERRORS == 1
Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Sep 30, 2020

Choose a reason for hiding this comment

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

If this case is reached, then it's definitely a bug in SpecFuzz. It must crash at this point with an error. There's no reason to continue fuzzing when a simulation is buggy.

Copy link
Author

@AlonB644 AlonB644 Oct 10, 2020

Choose a reason for hiding this comment

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

So you stop fuzzing and the latest file in corpus can be used for debugging

specfuzz_seed_input();
#endif
abort();
}
inside_handler = 1;

if (nesting_level <= 0x0) {
fprintf(stderr, "[SF] Error: Signal handler called outside speculation\n");
#if SEED_NON_SPECULATIVE_ERRORS == 1
specfuzz_seed_input();
#endif
abort();
}

if (checkpoint_sp > &checkpoint_stack || checkpoint_sp < &checkpoint_stack_bottom) {
fprintf(stderr, "[SF] Error: checkpoint_sp is corrupted\n");
#if SEED_NON_SPECULATIVE_ERRORS == 1
Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Sep 30, 2020

Choose a reason for hiding this comment

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

See my previous comment.

specfuzz_seed_input();
#endif
abort();
}

if ((uint64_t *) uc_gregs[REG_RSP] <= &specfuzz_rtl_frame
&& (uint64_t *) uc_gregs[REG_RSP] >= &specfuzz_rtl_frame_bottom) {
fprintf(stderr, "[SF] Error: a signal caught within the SpecFuzz runtime\n");
#if SEED_NON_SPECULATIVE_ERRORS == 1
Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Sep 30, 2020

Choose a reason for hiding this comment

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

See my previous comment.

specfuzz_seed_input();
#endif
abort();
}

if (in_rlbk) {
fprintf(stderr, "[SF] Error: a signal caught within SpecFuzz's rollback\n");
#if SEED_NON_SPECULATIVE_ERRORS == 1
Copy link
Owner

@OleksiiOleksenko OleksiiOleksenko Sep 30, 2020

Choose a reason for hiding this comment

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

See my previous comment.

specfuzz_seed_input();
#endif
abort();
}
#endif



if (siginfo->si_signo == SIGFPE) {
STAT_INCREMENT(stat_signal_misc);
Expand Down
Loading