Fix store log and more #21
Conversation
| ENABLE_COVERAGE ?= 1 | ||
| COVERAGE_MEMORY_ERRORS ?= 1 | ||
| COVERAGE_CONTROL_FLOW_ERRORS ?= 1 | ||
| SEED_NON_SPECULATIVE_ERRORS ?= 1 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Got it. In this case, could you rename this variable to something more descriptive? Something like STORE_INPUT_ON_SPECFUZZ_FAILURE
| #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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
So you stop fuzzing and the latest file in corpus can be used for debugging
|
|
||
| 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 |
There was a problem hiding this comment.
See my previous comment.
| 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 |
There was a problem hiding this comment.
See my previous comment.
|
|
||
| if (in_rlbk) { | ||
| fprintf(stderr, "[SF] Error: a signal caught within SpecFuzz's rollback\n"); | ||
| #if SEED_NON_SPECULATIVE_ERRORS == 1 |
There was a problem hiding this comment.
See my previous comment.
| cmp %rbx, (%rcx) | ||
| jne .L1 | ||
| addq $16, %rsp | ||
| movq 16(%rsp), %rdx |
There was a problem hiding this comment.
Could you add a comment to explain what's going on here? It's very hard to read raw assembly without explanations.
| popq %rbx // value | ||
| popq %rcx // address | ||
| movq %rbx, (%rcx) | ||
| popq %rdx // size |
There was a problem hiding this comment.
Same as above - explanation needed.
| BuildMI(Parent, MI, Loc, TII->get(X86::PUSH64rmm), TmpReg) | ||
| .addImm(1).addReg(0) | ||
| .addImm(0).addReg(0); | ||
|
|
There was a problem hiding this comment.
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.
2 participants
Store-Log with cases of store width
Raise error on rollback's failure
Seed non-speculative errors into corpus (error traceability)