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

@@ -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
 ------------

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;
+	int new_size = getCount() + 1;
-	size_t new_last_index = getCount();
+	int new_last_index = getCount();
 
 	int res = reallocate_buffer(new_size);
-	if (res == 0) {
+	assert (res == 0 && "FATAL ERROR: Could not add event to cache");
-		set(new_last_index, val);
-	}
 
-	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;
+		int new_size = getCount() - 1;
-		res = reallocate_buffer(new_size);
+		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*>;
 

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
+ * To keep the code easily readable, some buffer management methods
- * are less performant than the could be (remove() and erase() have linear complexity).
+ * perform suboptimally (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")
  * 
  * 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;
+	int m_BufferCount;
-protected:
 	/**
-	 * 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

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));
 }
 

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

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
+			i = buffer_cache->makeActive(m_EvList, i);
-			EventList::iterator it = std::find(m_EvList.begin(), m_EvList.end(), pEvBreakpt);
+			assert(i >= 0 &&
-			it = m_EvList.makeActive(it);
+					   "FATAL ERROR: Could not erase BPEvent from cache");
-			//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;
-				}
-			}
 			// 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

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());
+					curr_addr == simulator.getRegisterManager().getInstructionPointer());
-			instr_list.push_back(curr_instr);
+
-			instr_list_file << curr_instr << endl;
+			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);