misc cleanups

This change touches several subsystems, tools and experiments
(sal, util, cmake, import-trace, generic-tracing, nanojpeg), and
changes details not worth separate commits.

Change-Id: Icd1d664d1be5cfc2212dbf77801c271183214d08

cmake/config_failbochs.sh.in

@@ -1,19 +0,0 @@
-#!/bin/bash
-
-SOURCE_DIR=@CMAKE_SOURCE_DIR@
-BINARY_DIR=@CMAKE_BINARY_DIR@
-PREFIX_DIR=@BOCHS_PREFIX_DIR@
-
-if [ ! -d "$SOURCE_DIR" ]; then
-	echo Source directory does not exists! $SOURCE_DIR
-	exit 2
-fi
-
-if [ ! -d "$PREFIX_DIR" ]; then
-	echo Prefix directory does not exists! $BINARY_DIR 
-	exit 2
-fi
-
-
-./configure CXX="ag++ -p $SOURCE_DIR -I$SOURCE_DIR/src -I"$BINARY_DIR"/src --real-instances --Xcompiler" LIBTOOL="/bin/sh ./libtool --tag=CXX" --prefix=$PREFIX_DIR --enable-{a20-pin,x86-64,cpu-level=6,ne2000,acpi,pci,usb,repeat-speedups,trace-cache,fast-function-calls,host-specific-asms,disasm,all-optimizations,readline,clgd54xx,fpu,vmx=2,monitor-mwait,cdrom,sb16=linux,gdb-stub} --with-all-libs
-

src/core/sal/CPU.cc

@@ -22,6 +22,7 @@ void CPUArchitecture::m_addRegister(Register* reg, RegisterType type)
 Register* CPUArchitecture::getRegister(size_t i) const
 {
 	assert(i < m_Registers.size() && "FATAL ERROR: Invalid index provided!");
+	assert(m_Registers[i]->getId() == i && "FATAL ERROR: Register index mismatch");
 	return m_Registers[i];
 }
 

src/core/sal/CPU.hpp

@@ -13,7 +13,7 @@ namespace fail {
 /**
  * \class CPUArchitecture
  * This is the base class for CPU architectures that can be used to merge information and
- * functionallity that every backend with the same target architecture will share. The classes
+ * functionality that every backend with the same target architecture will share.  The classes
  * directly derived from this are especially meant to be usable in campaigns, so they shouldn't
  * contain any backend specific code.
  */

src/core/sal/Register.hpp

@@ -102,7 +102,7 @@ public:
 	 */
 	iterator begin() { return m_Regs.begin(); }
 	/**
-	 * Returns an iterator to the end of the interal data structure.
+	 * Returns an iterator to the end of the internal data structure.
 	 * \code
 	 * [1|2| ... |n]X
 	 *              ^

src/core/util/Database.cc

@@ -217,7 +217,7 @@ void Database::cmdline_setup() {
 	HOSTNAME	  = cmd.addOption("H", "hostname", Arg::Required,
 								  "-h/--hostname \tMYSQL Hostname (default: taken from ~/.my.cnf)");
 	USERNAME	  = cmd.addOption("u", "username", Arg::Required,
-								  "-u/--username \tMYSQL Username (default: taken from ~/.my.cnf, or your current user)");
+								  "-u/--username \tMYSQL Username (default: taken from ~/.my.cnf, or your current user)\n");
 }
 
 Database * Database::cmdline_connect() {

src/experiments/generic-tracing/experiment.cc

@@ -41,7 +41,7 @@ void  GenericTracing::parseOptions() {
 
 	CommandLine::option_handle FULL_TRACE = cmd.addOption("", "full-trace", Arg::None, "--full-trace \tDo a full trace (more data, default: off)");
 	CommandLine::option_handle MEM_SYMBOL	= cmd.addOption("m", "memory-symbol", Arg::Required,
-															"-m,--memory-symbol \tELF symbol(s) to trace accesses (without specifiying all mem read/writes are traced)");
+															"-m,--memory-symbol \tELF symbol(s) to trace accesses (default: all mem read/writes are traced)");
 	CommandLine::option_handle MEM_REGION	= cmd.addOption("M", "memory-region", Arg::Required,
 															"-M,--memory-region \trestrict memory region which is traced"
 															"	Possible formats: 0x<address>, 0x<address>:0x<address>, 0x<address>:<length>");

src/experiments/instantiate-experiment-indirect.ah.in

@@ -5,10 +5,6 @@
 // includes generated headers (e.g., protobuf message definitions) that are not
 // guaranteed to exist when the aspect is woven.
 
-// FIXME: cmake does not remove these .ah files when the user configures
-// another experiment (or even makes "clean").  Currently, this needs to be
-// worked around by manually removing $BUILDDIR/core/experiments/*/*.ah .
-
 // You need to provide the implementation of this function in your experiment
 // directory:
 void instantiate@EXPERIMENT_TYPE@();

src/experiments/instantiate-experiment.ah.in

@@ -1,10 +1,6 @@
 #ifndef __INSTANTIATE_@EXPERIMENT_TYPE@_AH__
   #define __INSTANTIATE_@EXPERIMENT_TYPE@_AH__
 
-// FIXME: cmake does not remove these .ah files when the user configures
-// another experiment (or even makes "clean").  Currently, this needs to be
-// worked around by manually removing $BUILDDIR/core/experiments/*/*.ah .
-
 // Make sure your experiment declaration is in experiment.hpp:
 #include "../experiments/@EXPERIMENT_NAME@/experiment.hpp"
 #include "sal/SALInst.hpp"

src/experiments/nanojpeg/campaign.cc

@@ -52,14 +52,14 @@ bool NanoJPEGCampaign::run()
 	// list: latest accesses (instr offset | bit mask)
 	map<GPRegisterId, std::list<std::pair<unsigned, uint64_t> > > reg_cascade;
 	// open up an equivalence class for all bits in all GPRs
-	reg_cascade[RID_EAX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CAX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_EBX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CBX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_ECX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CCX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_EDX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CDX].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_ESP].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CSP].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_EBP].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CBP].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_ESI].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CSI].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
-	reg_cascade[RID_EDI].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
+	reg_cascade[RID_CDI].push_front(std::pair<unsigned, uint64_t>(0, 0xffffffffffffffffULL));
 
 	// load trace
 	ifstream tracef(NANOJPEG_TRACE);
@@ -145,7 +145,7 @@ bool NanoJPEGCampaign::run()
 				acc->second &= ~common_mask;
 
 				// new EC with experiments: acc->first -- instr, common_mask
-//				if (reg != RID_EBP && reg != RID_ESI && reg != RID_EDI) {
+//				if (reg != RID_CBP && reg != RID_CSI && reg != RID_CDI) {
 					count_exp += add_experiment_ec(acc->first, instr, absolute_instr, reg, common_mask);
 //				}
 
@@ -189,7 +189,7 @@ bool NanoJPEGCampaign::run()
 				// skip empty EC (because register was read within the same instruction)?
 				if (acc->first <= instr) {
 					// new EC with known result: acc->first -- instr, common_mask
-//					if (reg != RID_EBP && reg != RID_ESI && reg != RID_EDI) {
+//					if (reg != RID_CBP && reg != RID_CSI && reg != RID_CDI) {
 						count_known += add_known_ec(acc->first, instr, absolute_instr, reg, common_mask);
 //					}
 				}
@@ -234,7 +234,7 @@ bool NanoJPEGCampaign::run()
 			// skip empty EC (because register was read within the same instruction)?
 			if (acc->first <= instr) {
 				// new EC with known result: acc->first -- instr, common_mask
-//				if (reg != RID_EBP && reg != RID_ESI && reg != RID_EDI) {
+//				if (reg != RID_CBP && reg != RID_CSI && reg != RID_CDI) {
 					count_exp += add_experiment_ec(acc->first, instr, absolute_instr, reg, common_mask);
 //				}
 			}

tools/import-trace/Importer.cc

@@ -117,7 +117,7 @@ bool Importer::copy_to_database(fail::ProtoIStream &ps) {
 	m_last_ip = ev.ip();      // The last event in the log
 
 	/* Signal that the trace was completely imported */
-	LOG << "trace duration: " << (curtime - m_time_trace_start) << " ticks" << std::endl;
+	LOG << "trace duration: " << std::dec << (curtime - m_time_trace_start) << " ticks" << std::endl;
 	LOG << "Inserted " << m_row_count << " real trace events into the database" << std::endl;
 
 

tools/import-trace/MemoryImporter.cc

@@ -54,4 +54,3 @@ bool MemoryImporter::handle_mem_event(simtime_t curtime, instruction_count_t ins
 	}
 	return true;
 }
-

tools/import-trace/RandomJumpImporter.cc

@@ -18,9 +18,9 @@ bool RandomJumpImporter::cb_commandline_init() {
 	CommandLine &cmd = CommandLine::Inst();
 
 	FROM = cmd.addOption("", "jump-from", Arg::Required,
-						 "--jump-from\t RandomJump: Which addresses should be jumped from\n");
+		"--jump-from \tRandomJump: Which addresses should be jumped from (a memory map; may be used more than once)");
 	TO   = cmd.addOption("", "jump-to", Arg::Required,
-						 "--jump-to\t RandomJump: Where to jump (a memory map>\n");
+		"--jump-to \tRandomJump: Where to jump (a memory map; may be used more than once)");
 	return true;
 }
 
@@ -35,8 +35,8 @@ bool RandomJumpImporter::handle_ip_event(fail::simtime_t curtime, instruction_co
 			return false;
 		}
 
-		// Read FROM memory file
+		// Read FROM memory map(s)
-		if (cmd[FROM].count() > 0) {
+		if (cmd[FROM]) {
 			m_mm_from = new MemoryMap();
 			for (option::Option *o = cmd[FROM]; o; o = o->next()) {
 				if (!m_mm_from->readFromFile(o->arg)) {
@@ -46,7 +46,8 @@ bool RandomJumpImporter::handle_ip_event(fail::simtime_t curtime, instruction_co
 			}
 		}
 
-		if (cmd[TO].count() > 0) {
+		// Read TO memory map(s)
+		if (cmd[TO]) {
 			m_mm_to = new MemoryMap();
 			for (option::Option *o = cmd[TO]; o; o = o->next()) {
 				if (!m_mm_to->readFromFile(o->arg)) {
@@ -81,7 +82,7 @@ bool RandomJumpImporter::handle_ip_event(fail::simtime_t curtime, instruction_co
 		/* Collect all addresses we want to jump to */
 		for (LLVMDisassembler::InstrMap::const_iterator instr = instr_map.begin();
 			 instr != instr_map.end(); ++instr) {
-			if (m_mm_to->isMatching(instr->first)) {
+			if (m_mm_to && m_mm_to->isMatching(instr->first)) {
 				m_jump_to_addresses.push_back(instr->first);
 			}
 		}
@@ -95,7 +96,7 @@ bool RandomJumpImporter::handle_ip_event(fail::simtime_t curtime, instruction_co
 	}
 
 	// skip events that are outside the --jump-from memory map.
-	if (!m_mm_from->isMatching(ev.ip())) {
+	if (m_mm_from && !m_mm_from->isMatching(ev.ip())) {
 		return true;
 	}
 
@@ -120,7 +121,7 @@ bool RandomJumpImporter::handle_ip_event(fail::simtime_t curtime, instruction_co
 		// pass through potentially available extended trace information
 		ev.set_accesstype(ev.READ); // instruction fetch is always a read
 		ev.set_memaddr(to_addr);
-		ev.set_width(4); // FIXME arbitrary?
+		ev.set_width(4); // FIXME arbitrary, use Instr.length instead?
 		if (!add_trace_event(margin, margin, ev)) {
 			LOG << "add_trace_event failed" << std::endl;
 			return false;

tools/import-trace/RandomJumpImporter.hpp

@@ -16,6 +16,7 @@ class RandomJumpImporter : public Importer {
 	fail::MemoryMap *m_mm_from, *m_mm_to;
 	std::vector<fail::guest_address_t> m_jump_to_addresses;
 public:
+	RandomJumpImporter() : m_mm_from(0), m_mm_to(0) {}
 	/**
 	 * Callback function that can be used to add command line options
 	 * to the campaign

tools/import-trace/RegisterImporter.cc

@@ -18,11 +18,11 @@ bool RegisterImporter::cb_commandline_init() {
 	CommandLine &cmd = CommandLine::Inst();
 
 	NO_GP = cmd.addOption("", "no-gp", Arg::None,
-							"--no-gp\t RegisterImporter: do not inject general purpose registers\n");
+		"--no-gp \tRegisterImporter: do not inject general purpose registers");
 	FLAGS = cmd.addOption("", "flags", Arg::None,
-						  "--flags: RegisterImporter: trace flags register\n");
+		"--flags \tRegisterImporter: inject flags register");
 	IP    = cmd.addOption("", "ip", Arg::None,
-						 "--ip: RegisterImporter: trace instruction pointer\n");
+		"--ip \tRegisterImporter: inject instruction pointer");
 
 	return true;
 }
@@ -85,17 +85,9 @@ bool RegisterImporter::handle_ip_event(fail::simtime_t curtime, instruction_coun
 			std::cerr << "Error parsing arguments." << std::endl;
 			return false;
 		}
-
+		do_gp = !cmd[NO_GP];
-		// Read FROM memory file
+		do_flags = cmd[FLAGS];
-		if (cmd[NO_GP].count() > 0) {
+		do_ip = cmd[IP];
-			do_gp = false;
-		}
-		if (cmd[FLAGS].count() > 0) {
-			do_flags = true;
-		}
-		if (cmd[IP].count() > 0) {
-			do_ip = true;
-		}
 
 		/* Disassemble the binary if necessary */
 		llvm::InitializeAllTargetInfos();

tools/import-trace/main.cc

@@ -49,8 +49,7 @@ ProtoIStream openProtoStream(std::string input_file) {
 }
 
 int main(int argc, char *argv[]) {
-	std::string trace_file, username, hostname, database, benchmark;
+	std::string trace_file, variant, benchmark;
-	std::string variant;
 	ElfReader *elf_file = 0;
 	MemoryMap *memorymap = 0;
 
@@ -108,7 +107,7 @@ int main(int argc, char *argv[]) {
 	CommandLine::option_handle ENABLE_SANITYCHECKS =
 		cmd.addOption("", "enable-sanitychecks", Arg::None,
 			"--enable-sanitychecks \tEnable sanity checks "
-			"(in case something looks fishy)"
+			"(in case something looks fishy) "
 			"(default: disabled)");
 
 	if (!cmd.parse()) {
@@ -118,28 +117,28 @@ int main(int argc, char *argv[]) {
 
 	Importer *importer;
 
-	if (cmd[IMPORTER].count() > 0) {
+	if (cmd[IMPORTER]) {
 		std::string imp(cmd[IMPORTER].first()->arg);
 		if (imp == "BasicImporter" || imp == "MemoryImporter" || imp == "memory" || imp == "mem") {
-			LOG << "Using MemoryImporter" << endl;
+			imp = "MemoryImporter";
 			importer = new MemoryImporter();
 #ifdef BUILD_LLVM_DISASSEMBLER
 		} else if (imp == "InstructionImporter" || imp == "code") {
-			LOG << "Using InstructionImporter" << endl;
+			imp = "InstructionImporter";
 			importer = new InstructionImporter();
 
 		} else if (imp == "RegisterImporter" || imp == "regs") {
-			LOG << "Using RegisterImporter" << endl;
+			imp = "RegisterImporter";
 			importer = new RegisterImporter();
 
 		} else if (imp == "RandomJumpImporter") {
-			LOG << "Using RandomJumpImporter" << endl;
 			importer = new RandomJumpImporter();
 #endif
 		} else {
 			LOG << "Unkown import method: " << imp << endl;
 			exit(-1);
 		}
+		LOG << "Using " << imp << endl;
 
 	} else {
 		LOG << "Using MemoryImporter" << endl;
@@ -160,30 +159,33 @@ int main(int argc, char *argv[]) {
 		exit(0);
 	}
 
-	if (cmd[TRACE_FILE].count() > 0)
+	if (cmd[TRACE_FILE]) {
 		trace_file = std::string(cmd[TRACE_FILE].first()->arg);
-	else
+	} else {
 		trace_file = "trace.pb";
+	}
 
 	ProtoIStream ps = openProtoStream(trace_file);
 	Database *db = Database::cmdline_connect();
 
-	if (cmd[VARIANT].count() > 0)
+	if (cmd[VARIANT]) {
 		variant = std::string(cmd[VARIANT].first()->arg);
-	else
+	} else {
 		variant = "none";
+	}
 
-	if (cmd[BENCHMARK].count() > 0)
+	if (cmd[BENCHMARK]) {
 		benchmark = std::string(cmd[BENCHMARK].first()->arg);
-	else
+	} else {
 		benchmark = "none";
+	}
 
-	if (cmd[ELF_FILE].count() > 0) {
+	if (cmd[ELF_FILE]) {
 		elf_file = new ElfReader(cmd[ELF_FILE].first()->arg);
 	}
 	importer->set_elf(elf_file);
 
-	if (cmd[MEMORYMAP].count() > 0) {
+	if (cmd[MEMORYMAP]) {
 		memorymap = new MemoryMap();
 		for (option::Option *o = cmd[MEMORYMAP]; o; o = o->next()) {
 			if (!memorymap->readFromFile(o->arg)) {
@@ -193,7 +195,7 @@ int main(int argc, char *argv[]) {
 	}
 	importer->set_memorymap(memorymap);
 
-	if (cmd[FAULTSPACE_RIGHTMARGIN].count() > 0) {
+	if (cmd[FAULTSPACE_RIGHTMARGIN]) {
 		std::string rightmargin(cmd[FAULTSPACE_RIGHTMARGIN].first()->arg);
 		if (rightmargin == "W") {
 			importer->set_faultspace_rightmargin('W');
@@ -207,12 +209,8 @@ int main(int argc, char *argv[]) {
 		importer->set_faultspace_rightmargin('W');
 	}
 
-	if (cmd[ENABLE_SANITYCHECKS].count() > 0) {
+	importer->set_sanitychecks(cmd[ENABLE_SANITYCHECKS]);
-		importer->set_sanitychecks(true);
+	importer->set_extended_trace(cmd[EXTENDED_TRACE]);
-	}
-	if (cmd[EXTENDED_TRACE].count() > 0) {
-		importer->set_extended_trace(true);
-	}
 
 	if (!importer->init(variant, benchmark, db)) {
 		LOG << "importer->init() failed" << endl;
@@ -228,7 +226,7 @@ int main(int argc, char *argv[]) {
 		exit(-1);
 	}
 
-	if (cmd[NO_DELETE].count() == 0 && !importer->clear_database()) {
+	if (!cmd[NO_DELETE] && !importer->clear_database()) {
 		LOG << "clear_database() failed" << endl;
 		exit(-1);
 	}