From bd5802e5d7846c4ae03c4ae0a22e8ceec1cfdcf6 Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Fri, 27 Feb 2015 11:42:37 +0100 Subject: [PATCH 1/3] core/sal: allow repeating BochsController::save BochsController::save() now can in principle be called multiple times in a row. Not that this would really make sense, but the results are consistent now. Change-Id: Ib4c6eb571a364b0f7ea6142c8cfec004a12f98b3 --- src/core/sal/bochs/SaveState.ah | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/core/sal/bochs/SaveState.ah b/src/core/sal/bochs/SaveState.ah index f0f11699..9a479d58 100644 --- a/src/core/sal/bochs/SaveState.ah +++ b/src/core/sal/bochs/SaveState.ah @@ -19,13 +19,14 @@ aspect SaveState { advice execution (cpuLoop()) : order ("SaveState", "Breakpoints"); advice execution (cpuLoop()) : after () { - if (!fail::save_bochs_request) - return; - assert(fail::sr_path.size() > 0 && "FATAL ERROR: tried to save state without valid path"); - SIM->save_state(fail::sr_path.c_str()); - std::cout << "[FAIL] Save finished" << std::endl; - // TODO: Log-Level? - fail::simulator.saveDone(); + // loop allows to call save() multiple times in a row + while (fail::save_bochs_request) { + assert(fail::sr_path.size() > 0 && "FATAL ERROR: tried to save state without valid path"); + SIM->save_state(fail::sr_path.c_str()); + std::cout << "[FAIL] Save finished" << std::endl; + // TODO: Log-Level? + fail::simulator.saveDone(); + } } }; From 91a9c6f688994daddea14309515ca55931f29cf3 Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Fri, 27 Feb 2015 19:34:38 +0100 Subject: [PATCH 2/3] core/sal: restore() more reliable for bochs BochsController::restore() now recreates a state more expectable from the experiment. The state is now the same that save() leaves behind in its most prominent use case after hitting a breakpoint. This change breaks backwards compatibility with some experiments, see below! Right after a breakpoint on a specific address fired and BochsController::save() was called, another breakpoint on that specific address would not fire again (unless that instruction is executed again later on). Up to this change, the situation after calling BochsController::restore() was different: A breakpoint on that specific address would fire twice. This difference led to the problem that running the tracing plugin after save() would work fine (recording the current instruction once, since 3dc752c "tracing: fix loss of first dynamic instruction"), but running it after restore() would record the current instruction *twice*. This change aligns restore()'s behavior to that of save(). The implications for existing experiments, traces and results are: - Existing result data should be not affected at all, as trace.time1/time2 were correct before this change. Nevertheless, the assumption time2-time1 >= instr2-instr1 does not hold for equivalence classes including the first instruction, if the latter was faultily recorded twice (see below). - Existing traces that were recorded after a restore() (with a tracing plugin including the aforementioned commit 3dc752c) contain the first instruction twice. An affected trace can be corrected with this command line: dump-trace old.tc | tail -n +2 | convert-trace -f dump -t new.tc - For experiments that record traces after a restore() (such as ecos_kernel_test), nothing changes, as both the tracing and the fast-forwarding before the fault injection now see one instruction event less. - Experiments that record traces after a save(), especially those that rely on the generic-tracing experiment for tracing, now see one instruction event less, before they need to inject their fault. These experiments need to be adjusted, for example dciao-kernelstructs now should use bp.setCounter(injection_instr) instead of bp.setCounter(injection_instr+1). Change-Id: I913bed9f1cad91ed3025f610024d62cfc2b9b11b --- src/core/sal/bochs/BochsController.cc | 3 ++- src/core/sal/bochs/FailBochsGlobals.hpp | 1 + src/core/sal/bochs/RestoreState.ah | 22 +++++++++++++++++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/core/sal/bochs/BochsController.cc b/src/core/sal/bochs/BochsController.cc index f527c1e1..7b8aa140 100644 --- a/src/core/sal/bochs/BochsController.cc +++ b/src/core/sal/bochs/BochsController.cc @@ -9,6 +9,7 @@ namespace fail { #ifdef DANCEOS_RESTORE bx_bool restore_bochs_request = false; +bx_bool restore_bochs_finished = false; bx_bool save_bochs_request = false; std::string sr_path = ""; #endif @@ -106,6 +107,7 @@ void BochsController::restore(const std::string& path) { clearListeners(); restore_bochs_request = true; + restore_bochs_finished = false; BX_CPU(0)->async_event |= 1; sr_path = path; m_CurrFlow = m_Flows.getCurrent(); @@ -114,7 +116,6 @@ void BochsController::restore(const std::string& path) void BochsController::restoreDone() { - restore_bochs_request = false; m_Flows.toggle(m_CurrFlow); } diff --git a/src/core/sal/bochs/FailBochsGlobals.hpp b/src/core/sal/bochs/FailBochsGlobals.hpp index 8ee30bbf..50346dea 100644 --- a/src/core/sal/bochs/FailBochsGlobals.hpp +++ b/src/core/sal/bochs/FailBochsGlobals.hpp @@ -9,6 +9,7 @@ namespace fail { #ifdef DANCEOS_RESTORE extern bx_bool restore_bochs_request; +extern bx_bool restore_bochs_finished; extern bx_bool save_bochs_request; extern std::string sr_path; #endif diff --git a/src/core/sal/bochs/RestoreState.ah b/src/core/sal/bochs/RestoreState.ah index 9c9001f6..9ad278c2 100644 --- a/src/core/sal/bochs/RestoreState.ah +++ b/src/core/sal/bochs/RestoreState.ah @@ -13,10 +13,30 @@ #include "../SALInst.hpp" aspect RestoreState { - pointcut restoreState() = "void bx_sr_after_restore_state()"; + pointcut restoreState() = "void bx_sr_after_restore_state()"; + pointcut cpuLoop() = "void defineCPULoopJoinPoint(...)"; advice execution (restoreState()) : after () { + // did the experiment trigger this restore? + if (fail::restore_bochs_request) { + fail::restore_bochs_request = false; + fail::restore_bochs_finished = true; + } + } + + // Make sure the "RestoreState" aspect comes *after* the breakpoint stuff + // to create a simulator / experiment state very similar to when the state + // was saved. In an "after" advice this means it must get a *higher* + // precedence, therefore it's first in the order list. + advice execution (cpuLoop()) : order ("RestoreState", "Breakpoints"); + + advice execution (cpuLoop()) : after () + { + if (!fail::restore_bochs_finished) { + return; + } + fail::restore_bochs_finished = false; std::cout << "[FAIL] Restore finished" << std::endl; // TODO: Log-Level? fail::simulator.restoreDone(); From 193e5b757e8d158f93f59cef1e5385975c449948 Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Wed, 18 Mar 2015 18:16:47 +0100 Subject: [PATCH 3/3] adapt experiments to new restore() behavior This change adapts several experiments, including the DatabaseExperiment framework, to the restore() behavior update from the previous change. Existing traces should continue to be usable. This is not tested yet, mainly because I don't have access to most of the experiment targets / guest systems necessary for testing. Please test your own experiments if possible, or at least leave me a note that you couldn't test it! Especially the cored-voter/experiment.cc update may be broken, but maybe the "FISHY" +2 in there was not OK in the first place. Change-Id: I0c5daeabc8fe6ce0c3ce3e7e13d02195f41340ad --- src/core/efw/DatabaseExperiment.cc | 39 ++++++++++--------- src/experiments/cored-tester/experiment.cc | 26 +++++++------ src/experiments/cored-voter/experiment.cc | 4 +- .../dciao-kernelstructs/experiment.cc | 2 +- src/experiments/fiascoFail/experiment.cc | 2 +- src/experiments/kesogc/experiment.cc | 3 +- src/experiments/kesorefs/experiment.cc | 2 +- src/experiments/vezs-example/experiment.cc | 3 +- 8 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/core/efw/DatabaseExperiment.cc b/src/core/efw/DatabaseExperiment.cc index 82fccf24..93736d1d 100644 --- a/src/core/efw/DatabaseExperiment.cc +++ b/src/core/efw/DatabaseExperiment.cc @@ -109,28 +109,29 @@ bool DatabaseExperiment::run() simulator.clearListeners(); - // Generate an experiment listener, that matches on any IP - // event. It is used to forward to the injection - // point. The +1 is needed, since even for the zeroth - // dynamic instruction we need at least one breakpoint - // event. - BPSingleListener bp; - bp.setWatchInstructionPointer(ANY_ADDR); - bp.setCounter(injection_instr + 1); - simulator.addListener(&bp); - if (!this->cb_before_fast_forward()) { continue; } - fail::BaseListener * listener; - while (true) { - listener = simulator.resume(); - if (listener == &bp) { - break; - } else { - bool should_continue = this->cb_during_fast_forward(listener); - if (!should_continue) - break; // Stop fast forwarding + + // Do we need to fast-forward at all? + if (injection_instr > 0) { + // Create a listener that matches any IP event. It is used to + // forward to the injection point. + BPSingleListener bp; + bp.setWatchInstructionPointer(ANY_ADDR); + bp.setCounter(injection_instr); + simulator.addListener(&bp); + + fail::BaseListener * listener; + while (true) { + listener = simulator.resume(); + if (listener == &bp) { + break; + } else { + bool should_continue = this->cb_during_fast_forward(listener); + if (!should_continue) + break; // Stop fast forwarding + } } } if (!this->cb_after_fast_forward(listener)) { diff --git a/src/experiments/cored-tester/experiment.cc b/src/experiments/cored-tester/experiment.cc index 0e8a486d..a5cc698a 100644 --- a/src/experiments/cored-tester/experiment.cc +++ b/src/experiments/cored-tester/experiment.cc @@ -321,18 +321,22 @@ bool CoredTester::run() { simulator.addListener(&l_fail_trace); BPSingleListener bp; - bp.setWatchInstructionPointer(ANY_ADDR); - // TODO: why does this need a +1? - bp.setCounter(injection_instr+1); - simulator.addListener(&bp); - - fail::BaseListener * listener = simulator.resume(); + fail::BaseListener *listener = 0; bool ok = true; - while ( ok && (listener == &l_fail_trace) ) { - // m_log << "CP IP 0x" << std::hex << simulator.getCPU(0).getInstructionPointer() << std::endl; - ok = cpoint.check(s_fail_trace, l_fail_trace.getTriggerInstructionPointer()) == Checkpoint::IDENTICAL; - if(ok) listener = simulator.addListenerAndResume(&l_fail_trace); + // do we have to fast-forward at all? + if (injection_instr > 0) { + bp.setWatchInstructionPointer(ANY_ADDR); + bp.setCounter(injection_instr); + simulator.addListener(&bp); + + listener = simulator.resume(); + + while ( ok && (listener == &l_fail_trace) ) { + // m_log << "CP IP 0x" << std::hex << simulator.getCPU(0).getInstructionPointer() << std::endl; + ok = cpoint.check(s_fail_trace, l_fail_trace.getTriggerInstructionPointer()) == Checkpoint::IDENTICAL; + if(ok) listener = simulator.addListenerAndResume(&l_fail_trace); + } } unsigned experiment_number = cpoint.getCount(); @@ -347,7 +351,7 @@ bool CoredTester::run() { break; } - if (listener != &bp) { + if (listener != &bp && injection_instr > 0) { result = param.msg.add_result(); handleEvent(*result, result->NOINJECTION, "WTF"); diff --git a/src/experiments/cored-voter/experiment.cc b/src/experiments/cored-voter/experiment.cc index a09af3ad..75c9d16b 100644 --- a/src/experiments/cored-voter/experiment.cc +++ b/src/experiments/cored-voter/experiment.cc @@ -349,11 +349,11 @@ bool CoredVoter::run() { // Fast forward to injection address m_log << "Trying to inject @ instr #" << dec << injection_instr << endl; - if ((injection_instr + 2) > 0) { + if (injection_instr > 0) { simulator.clearListeners(); BPSingleListener bp; bp.setWatchInstructionPointer(ANY_ADDR); - bp.setCounter(injection_instr + 2); // FIXME: FISHY! + bp.setCounter(injection_instr); simulator.addListener(&bp); fail::BaseListener * listener = simulator.resume(); diff --git a/src/experiments/dciao-kernelstructs/experiment.cc b/src/experiments/dciao-kernelstructs/experiment.cc index f3bb3da2..1b360e9f 100644 --- a/src/experiments/dciao-kernelstructs/experiment.cc +++ b/src/experiments/dciao-kernelstructs/experiment.cc @@ -197,7 +197,7 @@ bool DCIAOKernelStructs::run() { simulator.addListener(&l_time_marker_print); bp.setWatchInstructionPointer(ANY_ADDR); - bp.setCounter(injection_instr+1); + bp.setCounter(injection_instr); simulator.addListener(&bp); // Add vport listener diff --git a/src/experiments/fiascoFail/experiment.cc b/src/experiments/fiascoFail/experiment.cc index b9cac1c4..16237200 100644 --- a/src/experiments/fiascoFail/experiment.cc +++ b/src/experiments/fiascoFail/experiment.cc @@ -253,7 +253,7 @@ bool FiascoFailExperiment::faultInjection() if(instr_offset > 0) { bp.setWatchInstructionPointer(ANY_ADDR); // Create new Breakpoint... - bp.setCounter(instr_offset + 1); // ...to break when the IP for the fault injection is reached... + bp.setCounter(instr_offset); // ...to break when the IP for the fault injection is reached... simulator.addListener(&bp); // ...and add it to the actual listeners BaseListener *go = waitIOOrOther(true); // Resume simulation and log VGA-Output diff --git a/src/experiments/kesogc/experiment.cc b/src/experiments/kesogc/experiment.cc index 19f60219..30d0eacb 100644 --- a/src/experiments/kesogc/experiment.cc +++ b/src/experiments/kesogc/experiment.cc @@ -187,8 +187,7 @@ bool KESOgc::run() BPSingleListener bp; bp.setWatchInstructionPointer(ANY_ADDR); - // Fix offset by 1 error - bp.setCounter(injection_instr + 1); + bp.setCounter(injection_instr); simulator.addListener(&bp); diff --git a/src/experiments/kesorefs/experiment.cc b/src/experiments/kesorefs/experiment.cc index 0e1e9a84..ef39088f 100644 --- a/src/experiments/kesorefs/experiment.cc +++ b/src/experiments/kesorefs/experiment.cc @@ -161,7 +161,7 @@ bool KESOrefs::run() BPSingleListener bp; bp.setWatchInstructionPointer(ANY_ADDR); - bp.setCounter(injection_instr + 1); + bp.setCounter(injection_instr); simulator.addListener(&bp); bool inject = true; diff --git a/src/experiments/vezs-example/experiment.cc b/src/experiments/vezs-example/experiment.cc index d5267793..b0759f85 100644 --- a/src/experiments/vezs-example/experiment.cc +++ b/src/experiments/vezs-example/experiment.cc @@ -160,8 +160,7 @@ bool VEZSExperiment::run() BPSingleListener bp; bp.setWatchInstructionPointer(ANY_ADDR); - // Fix offset by 1 error - bp.setCounter(injection_instr + 1); + bp.setCounter(injection_instr); simulator.addListener(&bp);