From 13cd951a87d52eaa2c34f8dbf6bf21e289f703a4 Mon Sep 17 00:00:00 2001
From: unzner <unzner@8c4709b5-6ec9-48aa-a5cd-a96041d1645a>
Date: Sat, 9 Jun 2012 17:31:32 +0000
Subject: [PATCH] Bugfix in BufferCache, added some security checks, plus minor
stuff in doc/howtobuild.txt
git-svn-id: https://www4.informatik.uni-erlangen.de/i4svn/danceos/trunk/devel/fail@1323 8c4709b5-6ec9-48aa-a5cd-a96041d1645a
---
doc/howto-build.txt | 8 ++--
src/core/sal/BufferCache.cc | 59 +++++++++++++++++++--------
src/core/sal/BufferCache.hpp | 47 +++++++++++----------
src/core/sal/EventList.cc | 15 +++++--
src/core/sal/EventList.hpp | 1 +
src/core/sal/bochs/BochsController.cc | 17 +++-----
src/experiments/l4-sys/experiment.cc | 53 +++++++++++++++++-------
7 files changed, 127 insertions(+), 73 deletions(-)
diff --git a/doc/howto-build.txt b/doc/howto-build.txt
index 4216def3..da5f50a3 100644
--- a/doc/howto-build.txt
+++ b/doc/howto-build.txt
@@ -97,7 +97,7 @@ Build Bochs
===========
For the first time
------------------
-cd ../bochs
+cd ../simulators/bochs
> Sufficient:
./configure --prefix=$(echo ~/localroot/usr) --enable-{cpu-level=6,ne2000,trace-cache,gdb-stub} --disable-docbook
> More optimised:
@@ -136,17 +136,17 @@ Debug build
-----------
> Configure Bochs to use debugging-related compiler flags:
cd ../bochs
-CXXFLAGS="-g -O0" ./configure --prefix=... ... (see above)
+CFLAGS="-g -O0" CXXFLAGS="-g -O0" ./configure --prefix=... ... (see above)
Profiling-based optimization build
----------------------------------
> FIXME: ag++ needs to be run with --keep_woven
> Configure Bochs to use compiler flags to enable profiling:
cd ../bochs
-CXXFLAGS="-fprofile-generate" LDFLAGS="-fprofile-generate" ./configure --prefix=... ... (see above)
+CFLAGS="-fprofile-generate" CXXFLAGS="-fprofile-generate" LDFLAGS="-fprofile-generate" ./configure --prefix=... ... (see above)
> Build Bochs normally, and run it.
> Configure Bochs to use compiler flags to enable optimizations based on profiling results:
-CXXFLAGS="-fprofile-use -Wcoverage-mismatch" LDFLAGS="-fprofile-use" ./configure --prefix=... ... (see above)
+CFLAGS="-fprofile-use -Wcoverage-mismatch" CXXFLAGS="-fprofile-use -Wcoverage-mismatch" LDFLAGS="-fprofile-use" ./configure --prefix=... ... (see above)
Benchmarking
------------
diff --git a/src/core/sal/BufferCache.cc b/src/core/sal/BufferCache.cc
index cc876f77..8987ef49 100644
--- a/src/core/sal/BufferCache.cc
+++ b/src/core/sal/BufferCache.cc
@@ -1,27 +1,28 @@
+#include <algorithm>
+#include <vector>
#include "BufferCache.hpp"
#include "Event.hpp"
+#include "EventList.hpp"
namespace fail {
template<class T>
-int BufferCache<T>::add(T val)
+void BufferCache<T>::add(T val)
{
- size_t new_size = getCount() + 1;
- size_t new_last_index = getCount();
+ int new_size = getCount() + 1;
+ int new_last_index = getCount();
int res = reallocate_buffer(new_size);
- if (res == 0) {
- set(new_last_index, val);
- }
+ assert (res == 0 && "FATAL ERROR: Could not add event to cache");
- return res;
+ set(new_last_index, val);
}
template<class T>
-int BufferCache<T>::remove(T val)
+void BufferCache<T>::remove(T val)
{
bool do_remove = false;
- for (size_t i = 0; i < getCount(); i++) {
+ for (int i = 0; i < getCount(); i++) {
if (get(i) == val) {
do_remove = true;
}
@@ -32,13 +33,11 @@ int BufferCache<T>::remove(T val)
}
}
- int res = 0;
if (do_remove) {
- size_t new_size = getCount() - 1;
- res = reallocate_buffer(new_size);
+ int new_size = getCount() - 1;
+ int res = reallocate_buffer(new_size);
+ assert (res == 0 && "FATAL ERROR: Could not remove event from cache");
}
-
- return res;
}
template<class T>
@@ -52,19 +51,25 @@ void BufferCache<T>::clear()
template<class T>
int BufferCache<T>::erase(int idx)
{
- for (size_t i = idx; i < getCount() - 1; i++) {
+ if(idx < 0 || idx >= getCount())
+ return -2;
+
+ for (int i = idx; i < getCount() - 1; i++) {
set(i, get(i + 1));
}
- size_t new_size = getCount() - 1;
+ int new_size = getCount() - 1;
if (reallocate_buffer(new_size) != 0)
return -1;
return idx;
}
template<class T>
-int BufferCache<T>::reallocate_buffer(size_t new_size)
+int BufferCache<T>::reallocate_buffer(int new_size)
{
+ if (new_size < 0)
+ return 20;
+
if (new_size == 0) {
clear();
return 0;
@@ -77,6 +82,26 @@ int BufferCache<T>::reallocate_buffer(size_t new_size)
return 0;
}
+template<class T>
+int BufferCache<T>::makeActive(EventList &ev_list, int idx)
+{
+ assert(idx < getCount() &&
+ "FATAL ERROR: Index larger than cache!");
+ T ev = get(idx);
+ assert(ev && "FATAL ERROR: Object pointer cannot be NULL!");
+ ev->decreaseCounter();
+ if (ev->getCounter() > 0) {
+ return ++idx;
+ }
+ ev->resetCounter();
+ // Note: This is the one and only situation in which remove() should NOT
+ // store the removed item in the delete-list.
+ EventList::iterator it = std::find(ev_list.begin(), ev_list.end(), static_cast<BaseEvent*>(ev));
+ ev_list.m_remove(it, true); // remove event from buffer-list
+ ev_list.m_FireList.push_back(ev);
+ return erase(idx);
+}
+
// Declare whatever instances of the template you are going to use here:
template class BufferCache<BPEvent*>;
diff --git a/src/core/sal/BufferCache.hpp b/src/core/sal/BufferCache.hpp
index c08c4efa..5e413686 100644
--- a/src/core/sal/BufferCache.hpp
+++ b/src/core/sal/BufferCache.hpp
@@ -3,11 +3,10 @@
#include <stdlib.h>
-// FIXME: (Maybe) This should be located in utils, because
-// it's "Fail*-independend"...?
-
namespace fail {
+class EventList;
+
/**
* \class BufferCache
*
@@ -15,11 +14,8 @@ namespace fail {
*
* This class is intended to serve as a kind of cache for the entirely STL-based,
* untyped and therefore quite slow event handling mechanism of Fail*.
- * To keep the code easily readable, the buffer management methods
- * are less performant than the could be (remove() and erase() have linear complexity).
- *
- * FIXME: This desription sounds like a contradiction...
- * (-> "quite slow event handling" vs. "are less performant than the could be")
+ * To keep the code easily readable, some buffer management methods
+ * perform suboptimally (remove() and erase() have linear complexity).
*
* FIXME: Why not using std::vector? ("A simple dynamic array")
*/
@@ -28,21 +24,21 @@ class BufferCache {
private:
// TODO: comments ("//!<") needed!
T *m_Buffer;
- size_t m_BufferCount;
-protected:
+ int m_BufferCount;
/**
- * Changes the current length of the array. Should be inlined.
+ * Changes m_BufferCount. Should be inlined.
* @param new_count the new array length
*/
- inline void setCount(size_t new_count) { m_BufferCount = new_count; }
+ inline void setCount(int new_count) { if(new_count >= 0) m_BufferCount = new_count; }
+protected:
/**
* Reallocates the buffer. This implementation is extremely primitive,
* but since the amount of entries is small,
* this will not be significant, hopefully. Should be inlined.
* @param new_size the new number of elements in the array
- * @return 0 if successful, an error code otherwise (ATM only 10 if malloc() fails)
+ * @return 0 if successful, an error code otherwise (10 if realloc() fails, 20 for an invalid new size)
*/
- inline int reallocate_buffer(size_t new_size);
+ inline int reallocate_buffer(int new_size);
public:
BufferCache()
: m_Buffer(NULL), m_BufferCount(0) {}
@@ -50,19 +46,17 @@ public:
/**
* Add an element to the array. The object pointed to remains untouched.
* @param val the element to add
- * @return 0 if successful, an error code otherwise (ATM only 10 if malloc() fails)
*/
- int add(T val);
+ void add(T val);
/**
* Remove an element from the array. The object pointed to remains untouched.
* @param val the element to remove
- * @return 0 if successful, an error code otherwise (ATM only 10 if malloc() fails)
*/
- int remove(T val);
+ void remove(T val);
/**
* Remove an element at a specific position. The object pointed to remains untouched.
* @param val the element to remove
- * @return a pointer to the given element's successor if successful, -1 otherwise
+ * @return a pointer to the given element's successor if successful, a negative value otherwise
*/
int erase(int i);
/**
@@ -74,18 +68,27 @@ public:
* @param idx the position to retrieve the element from
* @return the element at the given position
*/
- inline T get(size_t idx) { return m_Buffer[idx]; }
+ inline T get(int idx) { return (idx >= 0 && idx < getCount() ? m_Buffer[idx] : NULL); }
/**
* Set an element at a given position. Should be inlined.
* @param idx the position to change an element at
* @param val the new value of the given element
*/
- inline void set(size_t idx, T val) { m_Buffer[idx] = val; }
+ inline void set(int idx, T val) { if(idx >= 0 && idx < getCount()) m_Buffer[idx] = val; }
/**
* Retrieves the current length of the array. Should be inlined.
* @return the array length
*/
- inline size_t getCount() { return m_BufferCount; }
+ inline int getCount() { return m_BufferCount; }
+ /**
+ * Acts as a replacement for EventList::makeActive, manipulating
+ * the buffer cache exclusively. EventList::fireActiveEvents needs
+ * to be called to fire the active events (see there).
+ * This method is declared as a friend method in EventList.
+ * @param idx the index of the event to trigger
+ * @returns an updated index which can be used to update a loop counter
+ */
+ int makeActive(EventList &ev_list, int idx);
};
} // end-of-namespace: fail
diff --git a/src/core/sal/EventList.cc b/src/core/sal/EventList.cc
index 0d27f7da..571a262c 100644
--- a/src/core/sal/EventList.cc
+++ b/src/core/sal/EventList.cc
@@ -63,16 +63,23 @@ EventList::iterator EventList::m_remove(iterator it, bool skip_deletelist)
if (!skip_deletelist) {
// If skip_deletelist = true, m_remove was called from makeActive. Accordingly, we
// are not going to delete an event, instead we are "moving" an event object (= *it)
- // from the buffer list to the fire-list. Therefor we only need to call the simulator's
+ // from the buffer list to the fire-list. Therefore we only need to call the simulator's
// event handler (m_onEventDeletion), if m_remove is called with the primary intention
// to *delete* (not "move") an event.
simulator.onEventDeletion(*it);
m_DeleteList.push_back(*it);
+
+ // Cached events have their own BufferCache<T>::makeActive() implementation, which
+ // calls this method and afterwards erase() in the cache class. This is why, when
+ // called from any kind of makeActive() method, it is unnecessary to call
+ // BufferCache<T>::remove() from m_remove().
+
+ // NOTE: in case the semantics of skip_deletelist change, please adapt the following lines
+ BPEvent *bp_ev;
+ if((bp_ev = dynamic_cast<BPEvent*>(*it)) != NULL)
+ m_Bp_cache.remove(bp_ev);
}
- BPEvent *bp_ev;
- if((bp_ev = dynamic_cast<BPEvent*>(*it)) != NULL)
- m_Bp_cache.remove(bp_ev);
return (m_BufferList.erase(it));
}
diff --git a/src/core/sal/EventList.hpp b/src/core/sal/EventList.hpp
index 828dc7d8..8e3c8d0f 100644
--- a/src/core/sal/EventList.hpp
+++ b/src/core/sal/EventList.hpp
@@ -54,6 +54,7 @@ class EventList
deletelist_t m_DeleteList; //!< the deleted events (used temporarily)
BaseEvent* m_pFired; //!< the recently fired Event-object
BufferCache<BPEvent*> m_Bp_cache;
+ friend int BufferCache<BPEvent*>::makeActive(EventList &ev_list, int idx);
public:
/**
* The iterator of this class used to loop through the list of
diff --git a/src/core/sal/bochs/BochsController.cc b/src/core/sal/bochs/BochsController.cc
index fa1f6b82..e012c45a 100644
--- a/src/core/sal/bochs/BochsController.cc
+++ b/src/core/sal/bochs/BochsController.cc
@@ -119,24 +119,17 @@ void BochsController::onInstrPtrChanged(address_t instrPtr, address_t address_sp
#endif
//this code is highly optimised for the average case, so it me appear a bit ugly
bool do_fire = false;
- unsigned i = 0;
+ int i = 0;
BufferCache<BPEvent*> *buffer_cache = m_EvList.getBPBuffer();
while(i < buffer_cache->getCount()) {
BPEvent *pEvBreakpt = buffer_cache->get(i);
if(pEvBreakpt->isMatching(instrPtr, address_space)) {
pEvBreakpt->setTriggerInstructionPointer(instrPtr);
- //transition to STL: find the element we are working on in the Event List
- EventList::iterator it = std::find(m_EvList.begin(), m_EvList.end(), pEvBreakpt);
- it = m_EvList.makeActive(it);
- //find out how much elements need to be skipped to get in sync again
- //(should be one or none, the loop is just to make sure)
- for(unsigned j = i; j < buffer_cache->getCount(); j++) {
- if(buffer_cache->get(j) == (*it)) {
- i = j;
- break;
- }
- }
+ i = buffer_cache->makeActive(m_EvList, i);
+ assert(i >= 0 &&
+ "FATAL ERROR: Could not erase BPEvent from cache");
+
// we now know we need to fire the active events - usually we do not have to
do_fire = true;
// "i" has already been set to the next element (by calling
diff --git a/src/experiments/l4-sys/experiment.cc b/src/experiments/l4-sys/experiment.cc
index a0c68713..89e31df8 100644
--- a/src/experiments/l4-sys/experiment.cc
+++ b/src/experiments/l4-sys/experiment.cc
@@ -32,12 +32,29 @@ using namespace fail;
save, and restore. Enable these in the configuration.
#endif
+typedef struct __trace_instr_type {
+ address_t trigger_addr;
+ unsigned bp_counter;
+} trace_instr;
+
+ostream& operator<<(ostream& out, const trace_instr &val) {
+ out << val.trigger_addr << "," << val.bp_counter;
+ return out;
+}
+istream& operator>>(istream& in, trace_instr &val) {
+ in >> val.trigger_addr;
+ //skip the comma
+ in.ignore(1);
+ in >> val.bp_counter;
+ return in;
+}
+
char const * const state_folder = "l4sys.state";
char const * const instr_list_fn = "ip.list";
char const * const golden_run_fn = "golden.out";
address_t const aspace = 0x01e00000;
string output;
-vector<address_t> instr_list;
+vector<trace_instr> instr_list;
string golden_run;
//the program needs to run 5 times without a fault
const unsigned times_run = 5;
@@ -85,8 +102,6 @@ bool L4SysExperiment::run()
log << "startup" << endl;
-#ifdef PREPARE_EXPERIMENT
-
struct stat teststruct;
// STEP 1: run until interesting function starts, and save state
if (stat(state_folder, &teststruct) == -1) {
@@ -115,14 +130,25 @@ bool L4SysExperiment::run()
ofstream instr_list_file(instr_list_fn);
instr_list_file << hex;
bp.setWatchInstructionPointer(ANY_ADDR);
+
+ map<address_t, unsigned> times_called_map;
while (bp.getTriggerInstructionPointer() != L4SYS_FUNC_EXIT) {
simulator.addEventAndWait(&bp);
//short sanity check
- address_t curr_instr = bp.getTriggerInstructionPointer();
+ address_t curr_addr = bp.getTriggerInstructionPointer();
assert(
- curr_instr == simulator.getRegisterManager().getInstructionPointer());
- instr_list.push_back(curr_instr);
- instr_list_file << curr_instr << endl;
+ curr_addr == simulator.getRegisterManager().getInstructionPointer());
+
+ unsigned times_called = times_called_map[curr_addr];
+ times_called++;
+ times_called_map[curr_addr] = times_called;
+
+ trace_instr new_instr;
+ new_instr.trigger_addr = curr_addr;
+ new_instr.bp_counter = times_called;
+ instr_list.push_back(new_instr);
+
+ instr_list_file << new_instr << endl;
}
log << "saving instructions triggered during normal execution" << endl;
instr_list_file.close();
@@ -130,7 +156,7 @@ bool L4SysExperiment::run()
ifstream instr_list_file(instr_list_fn);
instr_list_file >> hex;
while (!instr_list_file.eof()) {
- address_t curr_instr;
+ trace_instr curr_instr;
instr_list_file >> curr_instr;
instr_list.push_back(curr_instr);
}
@@ -186,10 +212,8 @@ bool L4SysExperiment::run()
output.reserve(flen);
}
-#endif
-
// STEP 4: The actual experiment.
- for (int i = 0; i < 1/*L4SYS_NUMINSTR*/; i++) {
+ for (int i = 0; i < L4SYS_NUMINSTR; i++) {
log << "restoring state" << endl;
simulator.restore(state_folder);
@@ -206,7 +230,8 @@ bool L4SysExperiment::run()
log << "job " << id << " instr " << instr_offset << " bit "
<< bit_offset << endl;
- bp.setWatchInstructionPointer(instr_list[instr_offset]);
+ bp.setWatchInstructionPointer(instr_list[instr_offset].trigger_addr);
+ bp.setCounter(instr_list[instr_offset].bp_counter);
simulator.addEvent(&bp);
//and log the output
waitIOOrOther(true);
@@ -227,10 +252,10 @@ bool L4SysExperiment::run()
<< endl;
// sanity check (only works if we're working with an instruction trace)
- if (injection_ip != instr_list[instr_offset]) {
+ if (injection_ip != instr_list[instr_offset].trigger_addr) {
stringstream ss;
ss << "SANITY CHECK FAILED: " << injection_ip << " != "
- << instr_list[instr_offset] << endl;
+ << instr_list[instr_offset].trigger_addr << endl;
log << ss.str();
param.msg.set_resulttype(param.msg.UNKNOWN);
param.msg.set_resultdata(injection_ip);
--
GitLab