From 12b539ff75081643feed4b2b4a1ffec63fbb2634 Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Mon, 8 Jul 2013 08:56:16 +0200 Subject: [PATCH] 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 | 19 -------- src/core/sal/CPU.cc | 1 + src/core/sal/CPU.hpp | 2 +- src/core/sal/Register.hpp | 2 +- src/core/util/Database.cc | 2 +- src/experiments/generic-tracing/experiment.cc | 2 +- .../instantiate-experiment-indirect.ah.in | 4 -- src/experiments/instantiate-experiment.ah.in | 4 -- src/experiments/nanojpeg/campaign.cc | 22 ++++----- tools/import-trace/Importer.cc | 2 +- tools/import-trace/MemoryImporter.cc | 1 - tools/import-trace/RandomJumpImporter.cc | 21 +++++---- tools/import-trace/RandomJumpImporter.hpp | 1 + tools/import-trace/RegisterImporter.cc | 26 ++++------- tools/import-trace/main.cc | 46 +++++++++---------- 15 files changed, 60 insertions(+), 95 deletions(-) delete mode 100755 cmake/config_failbochs.sh.in diff --git a/cmake/config_failbochs.sh.in b/cmake/config_failbochs.sh.in deleted file mode 100755 index 7cbf7693..00000000 --- a/cmake/config_failbochs.sh.in +++ /dev/null @@ -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 - diff --git a/src/core/sal/CPU.cc b/src/core/sal/CPU.cc index 2315fa92..815c9f54 100644 --- a/src/core/sal/CPU.cc +++ b/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]; } diff --git a/src/core/sal/CPU.hpp b/src/core/sal/CPU.hpp index 65ff9d3e..23a020f5 100644 --- a/src/core/sal/CPU.hpp +++ b/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. */ diff --git a/src/core/sal/Register.hpp b/src/core/sal/Register.hpp index 66f5ec9b..379220aa 100644 --- a/src/core/sal/Register.hpp +++ b/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 * ^ diff --git a/src/core/util/Database.cc b/src/core/util/Database.cc index 19094b48..b723be0c 100644 --- a/src/core/util/Database.cc +++ b/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() { diff --git a/src/experiments/generic-tracing/experiment.cc b/src/experiments/generic-tracing/experiment.cc index f7a114c9..0d57f049 100644 --- a/src/experiments/generic-tracing/experiment.cc +++ b/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
, 0x
:0x
, 0x
:"); diff --git a/src/experiments/instantiate-experiment-indirect.ah.in b/src/experiments/instantiate-experiment-indirect.ah.in index b3a72752..e40f15dc 100644 --- a/src/experiments/instantiate-experiment-indirect.ah.in +++ b/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@(); diff --git a/src/experiments/instantiate-experiment.ah.in b/src/experiments/instantiate-experiment.ah.in index 5bf1ed32..d6cb24b1 100644 --- a/src/experiments/instantiate-experiment.ah.in +++ b/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" diff --git a/src/experiments/nanojpeg/campaign.cc b/src/experiments/nanojpeg/campaign.cc index 04e4a08e..d7fc8e91 100644 --- a/src/experiments/nanojpeg/campaign.cc +++ b/src/experiments/nanojpeg/campaign.cc @@ -52,14 +52,14 @@ bool NanoJPEGCampaign::run() // list: latest accesses (instr offset | bit mask) map > > reg_cascade; // open up an equivalence class for all bits in all GPRs - reg_cascade[RID_EAX].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_EBX].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_ECX].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_EDX].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_ESP].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_EBP].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_ESI].push_front(std::pair(0, 0xffffffffffffffffULL)); - reg_cascade[RID_EDI].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CAX].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CBX].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CCX].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CDX].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CSP].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CBP].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CSI].push_front(std::pair(0, 0xffffffffffffffffULL)); + reg_cascade[RID_CDI].push_front(std::pair(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); // } } diff --git a/tools/import-trace/Importer.cc b/tools/import-trace/Importer.cc index 5080bd91..dceabac1 100644 --- a/tools/import-trace/Importer.cc +++ b/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; diff --git a/tools/import-trace/MemoryImporter.cc b/tools/import-trace/MemoryImporter.cc index 9f82cadf..64571529 100644 --- a/tools/import-trace/MemoryImporter.cc +++ b/tools/import-trace/MemoryImporter.cc @@ -54,4 +54,3 @@ bool MemoryImporter::handle_mem_event(simtime_t curtime, instruction_count_t ins } return true; } - diff --git a/tools/import-trace/RandomJumpImporter.cc b/tools/import-trace/RandomJumpImporter.cc index d2f52c72..0a7656ef 100644 --- a/tools/import-trace/RandomJumpImporter.cc +++ b/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 - if (cmd[FROM].count() > 0) { + // Read FROM memory map(s) + 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); } } @@ -91,12 +92,12 @@ bool RandomJumpImporter::handle_ip_event(fail::simtime_t curtime, instruction_co // skip events that are outside the memory map. -m instruction map if (m_mm && !m_mm->isMatching(ev.ip())) { - return true; + return true; } // skip events that are outside the --jump-from memory map. - if (!m_mm_from->isMatching(ev.ip())) { - return true; + 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; diff --git a/tools/import-trace/RandomJumpImporter.hpp b/tools/import-trace/RandomJumpImporter.hpp index f79337d8..4c9b1304 100644 --- a/tools/import-trace/RandomJumpImporter.hpp +++ b/tools/import-trace/RandomJumpImporter.hpp @@ -16,6 +16,7 @@ class RandomJumpImporter : public Importer { fail::MemoryMap *m_mm_from, *m_mm_to; std::vector 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 diff --git a/tools/import-trace/RegisterImporter.cc b/tools/import-trace/RegisterImporter.cc index d0304f89..e7b1249f 100644 --- a/tools/import-trace/RegisterImporter.cc +++ b/tools/import-trace/RegisterImporter.cc @@ -17,12 +17,12 @@ static Logger LOG("RegisterImporter"); 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 = cmd.addOption("", "no-gp", Arg::None, + "--no-gp \tRegisterImporter: do not inject general purpose registers"); FLAGS = cmd.addOption("", "flags", Arg::None, - "--flags: RegisterImporter: trace flags register\n"); - IP = cmd.addOption("", "ip", Arg::None, - "--ip: RegisterImporter: trace instruction pointer\n"); + "--flags \tRegisterImporter: inject flags register"); + IP = cmd.addOption("", "ip", Arg::None, + "--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; } - - // Read FROM memory file - if (cmd[NO_GP].count() > 0) { - do_gp = false; - } - if (cmd[FLAGS].count() > 0) { - do_flags = true; - } - if (cmd[IP].count() > 0) { - do_ip = true; - } + do_gp = !cmd[NO_GP]; + do_flags = cmd[FLAGS]; + do_ip = cmd[IP]; /* Disassemble the binary if necessary */ llvm::InitializeAllTargetInfos(); @@ -125,7 +117,7 @@ bool RegisterImporter::handle_ip_event(fail::simtime_t curtime, instruction_coun const LLVMDisassembler::Instr &opcode = instr_map.at(ev.ip()); //const MCRegisterInfo ®_info = disas->getRegisterInfo(); - fail::LLVMtoFailTranslator & ltof = disas->getTranslator() ; + fail::LLVMtoFailTranslator & ltof = disas->getTranslator() ; for (std::vector::const_iterator it = opcode.reg_uses.begin(); it != opcode.reg_uses.end(); ++it) { diff --git a/tools/import-trace/main.cc b/tools/import-trace/main.cc index ddc577a8..f287dace 100644 --- a/tools/import-trace/main.cc +++ b/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 variant; + std::string trace_file, variant, benchmark; 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(true); - } - if (cmd[EXTENDED_TRACE].count() > 0) { - importer->set_extended_trace(true); - } + importer->set_sanitychecks(cmd[ENABLE_SANITYCHECKS]); + importer->set_extended_trace(cmd[EXTENDED_TRACE]); 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); }