Commit d3cc5fc9 authored by Jason Hiser's avatar Jason Hiser 🚜

fixing up code review findings

parent 922963a4
Pipeline #2581 passed with stages
in 6 minutes and 53 seconds
......@@ -40,10 +40,11 @@ using namespace InitStack;
InitStack_t::InitStack_t(FileIR_t *p_variantIR, const string& p_functionsFilename, int p_initValue, bool p_verbose)
:
Transform(p_variantIR), // initialize the Transform class so things like insertAssembly and getFileIR() can be used
m_initValue(p_initValue), // member variable inits
m_initValue(p_initValue), // member variable inits, these will vary depending on your transform's objectives
m_verbose(p_verbose),
m_numTransformed(0)
{
// check whether to read in a list of functions to transform
if (p_functionsFilename=="")
{
cout << "Auto-initialize all functions" << endl;
......
......@@ -27,7 +27,7 @@
//
// Put the transform in its own namespace
// just to keep the header files easy to read.
// This is not an IRDB transform requirement.
// This is not an IRDB transform requirement, just good coding practice.
//
namespace InitStack
{
......@@ -54,7 +54,8 @@ namespace InitStack
// return value: true -> success, false -> fail
bool execute();
private: // methods
private:
// methods
// read in the given file full of function names to transform (called from constructor)
// input: the filename and FileIR to transform
......@@ -66,7 +67,7 @@ namespace InitStack
// output: the transformed fileIR
void initStack(Function_t* f);
private: // data
// data
set<Function_t*> m_funcsToInit; // the functions whose stacks this object should initialize
int m_initValue; // the value with which to init the stack.
bool m_verbose; // do verbose logging
......
/* fix copyright headers in all files */
/* fix variable name schema (camelcase? underscores? p_'s m_'s */
......@@ -31,6 +30,10 @@ using namespace std;
using namespace IRDB_SDK;
using namespace InitStack;
//
// Print usage info
//
void usage(char* p_name)
{
cerr << "Usage: " << p_name << " <variant_id>\n";
......@@ -41,8 +44,17 @@ void usage(char* p_name)
}
//
// The entry point for a stand-alone executable transform.
// Note: Thanos-enabled transforms are easier to write, faster to execute, and generally preferred,
// but stand-alone transforms may be useful if the transform has issues with memory leaks and/or memory errors.
// Memory issues in a stand alone transform cannot affect correctness of other transforms.
//
int main(int argc, char **argv)
{
//
// Sanity check that the command line has at least a variant ID, otherwise we won't know what variant to operate on.
//
if(argc < 2)
{
usage(argv[0]);
......@@ -58,7 +70,7 @@ int main(int argc, char **argv)
auto funcs_filename = string();
auto init_value = 0;
// parse some options for the transform
// declare some options for the transform
const char* short_opts="f:i:v?h";
struct option long_options[] = {
{"functions", required_argument, 0, 'f'},
......@@ -69,6 +81,8 @@ int main(int argc, char **argv)
{0,0,0,0}
};
// parse the options in a standard getopts_long loop
while(true)
{
int index = 0;
......@@ -98,15 +112,15 @@ int main(int argc, char **argv)
}
}
// setup the interface to the sql server
// stand alone transforms must setup the interface to the sql server
auto pqxx_interface=pqxxDB_t::factory();
BaseObj_t::setInterface(pqxx_interface.get());
// create and read a variant ID from the database
// stand alone transforms must create and read a variant ID from the database
auto pidp=VariantID_t::factory(variantID);
assert(pidp->isRegistered()==true);
// create and read the main file's IR from the datatbase
// stand alone transforms must create and read the main file's IR from the datatbase
auto this_file = pidp->getMainFile();
auto url = this_file->getURL();
......@@ -117,6 +131,7 @@ int main(int argc, char **argv)
try
{
// Create and download the file's IR.
// Note: this is done differently than with thanos-enabled plugins
auto firp = FileIR_t::factory(pidp.get(), this_file);
// sanity
......@@ -133,6 +148,8 @@ int main(int argc, char **argv)
if (success)
{
cout << "Writing changes for " << url << endl;
// Stand alone trnasforms must manually write the IR back to the IRDB and commit the transactions
firp->writeToDB();
pqxx_interface->commit();
}
......
......@@ -32,6 +32,13 @@ KillDeads::KillDeads(FileIR_t *p_variantIR)
}
//
// A method to actually transform the file IR by "killing" (setting) every
// register that is marked dead at every program point.
//
// input: a file IR
// output: a transformed file IR with every dead register having its value changed
//
bool KillDeads::execute()
{
// init statistics variables
......@@ -40,7 +47,7 @@ bool KillDeads::execute()
// seed RNG and choose a random kill value.
srand(time(0));
int kill_val=rand();
auto kill_val=rand();
// declare and construct a deep analysis engine
auto de=DeepAnalysis_t::factory(getFileIR());
......@@ -52,7 +59,7 @@ bool KillDeads::execute()
auto &reg_map=*reg_map_ptr;
// log
cout<<"Using random kill value: "<<dec<<kill_val<<endl;
cout << "Using random kill value: " << dec << kill_val << endl;
// the insertAssembly family modifies what getInstructions(), and the range-based for loop
// may go crazy. Copy first so we only check the original instructions
......@@ -67,13 +74,13 @@ bool KillDeads::execute()
// each register that's dead
for(auto reg : regset)
{
// if it's the flags, kill the flags with a cmp
// if it's the flags, kill the flags with a cmp instruction
if (reg==rn_EFLAGS)
{
// for flags, do a random compare to change them
char buf[100];
sprintf(buf," cmp rax, %d", kill_val);
cout<<"Inserting '"<<buf<<"' before '"<<insn->getDisassembly()<<"'"<<endl;
cout << "Inserting '" << buf << "' before '" << insn->getDisassembly() << "'" << endl;
insertAssemblyBefore(insn, buf);
killed_flags++;
}
......@@ -81,10 +88,10 @@ bool KillDeads::execute()
// check if it's an integer register
if(is64bitRegister(reg) || is32bitRegister(reg) || is16bitRegister(reg) || is8bitRegister(reg))
{
// other integer registers can be killed with a mov
// integer registers can be killed with a mov instruction
char buf[100];
sprintf(buf," mov %s, %d", registerToString(reg).c_str(), kill_val);
cout<<"Inserting '"<<buf<<"' before '"<<insn->getDisassembly()<<"'"<<endl;
cout << "Inserting '" << buf << "' before '" << insn->getDisassembly() << "'" << endl;
insertAssemblyBefore(insn, buf);
killed_regs++;
}
......@@ -101,8 +108,8 @@ bool KillDeads::execute()
//
// Output stats to log using #ATTRIBUTE convention
//
cout<<"#ATTRIBUTE killed_flags="<<dec<<killed_flags<<endl;
cout<<"#ATTRIBUTE killed_regs="<<dec<<killed_regs<<endl;
cout << "#ATTRIBUTE killed_flags=" << dec << killed_flags << endl;
cout << "#ATTRIBUTE killed_regs=" << dec << killed_regs << endl;
// success!
return true;
......
......@@ -25,6 +25,14 @@
#include <irdb-transform>
#include <irdb-deep>
//
// A simple class that overwrites any dead regiters.
// This is useful for demonstrating how to obtain a list of dead registers.
// Also useful for testing if dead registers are correct.
//
// Note: This elides creating a new namespace or "using" the IRDB_SDK namespace,
// so references to the IRDB_SDK classes must be explicitly scoped.
//
class KillDeads : public IRDB_SDK::Transform
{
public:
......@@ -34,4 +42,5 @@ class KillDeads : public IRDB_SDK::Transform
private:
// no class member data or methods yet.
};
#endif
......@@ -30,6 +30,7 @@ using namespace IRDB_SDK;
//
// kill_deads is a Thanos-enabled transform. Thanos-enabled transforms must implement the TransfromStep_t abstract class.
// See the IRDB SDK for additional details.
//
// For convenience, since this class is simple and shouldn't be used elsewhere, we just implement the class in the .cpp file
//
......@@ -41,13 +42,7 @@ class KillDeadsDriver_t : public IRDB_SDK::TransformStep_t
//
int parseArgs(const vector<string> step_args) override
{
// must get at least the variant ID to work on as the first parameter.
if(step_args.size() != 0)
{
usage(program_name);
return 2; // error
}
// no arguments to parse.
return 0; // success (bash-style 0=success, 1=warnings, 2=errors)
}
......@@ -56,15 +51,17 @@ class KillDeadsDriver_t : public IRDB_SDK::TransformStep_t
//
int executeStep() override
{
auto url=getMainFile()->getURL() ;
// record the URL from the main file for log output later
auto url=getMainFile()->getURL();
// try to load and transform the file's IR.
try
{
// load the fileIR (or, get the handle to an already loaded IR)
auto firp = getMainFileIR();
// declare and execute a transform
KillDeads kill_deads(firp);
auto success=kill_deads.execute();
// create a transform object and execute a transform
auto success = KillDeads(firp).execute();
// check for success
if (success)
......@@ -103,8 +100,8 @@ class KillDeadsDriver_t : public IRDB_SDK::TransformStep_t
int variantID = BaseObj_t::NOT_IN_DATABASE;
//
// optional: print arguments for this class
// very simple args: only the variant ID
// optional: print using info for this transform.
// This transform takes no parameters.
//
void usage(const string& p_name)
{
......
......@@ -35,17 +35,34 @@ using namespace IRDB_SDK;
#define ALLOF(a) begin(a), end(a)
//
// A thanos-enabled driver to "stamp" (xor) return addresses on the stack
//
class StackStampDriver_t : public IRDB_SDK::TransformStep_t
{
public:
//
// required overrride: how to parse your arguments
//
int parseArgs(const vector<string> step_args) override
{
/* convert to argv format for esay parsing wth getopts */
//
// convert to argc/argv format for parsing wth getopts
//
// Notes:
// step_args does not include the program name, so we add one in the argv initialization for getopts
// C++ containers are a bit weird about having constants, so we cast the const-ness away with const-cast
//
auto argv = vector<char*>({const_cast<char*>("libstack_stamp.so")});
transform(ALLOF(step_args), back_inserter(argv), [](const string &s) -> char* { return const_cast<char*>(s.c_str()); } );
const auto argc=argv.size();
//
// See the RNG and use it to start with a randomized stamp value, before we parse to see if a
// particular stack value is requested.
//
srand(getpid()+time(NULL));
stamp_value=rand();
......@@ -62,10 +79,8 @@ class StackStampDriver_t : public IRDB_SDK::TransformStep_t
// Parse options for the transform
while(true)
{
auto index = 0;
auto c = getopt_long(argc, &argv[0], short_opts, long_options, &index);
if(c == -1)
break;
auto c = getopt_long(argc, &argv[0], short_opts, long_options, nullptr);
if(c == -1) break;
switch(c)
{
case 's':
......@@ -85,43 +100,65 @@ class StackStampDriver_t : public IRDB_SDK::TransformStep_t
}
}
// log our stamp value
cout<<"Stamp value is set to:"<<hex<<stamp_value<<endl;
return 0;
}
//
// required override: how to transform the IR
//
int executeStep() override
{
// get the file's URL for later logging
auto url=getMainFile()->getURL();
// try to load and transform the file IR.
try
{
// Ask thanos for the IR
auto firp=getMainFileIR();
Stamper::StackStamp_t ss(firp, stamp_value, verbose);
const auto success=ss.execute();
// execute a transform.
const auto success= Stamper::StackStamp_t(firp, stamp_value, verbose).execute();
// return success status
return success ? 0 : 2; // bash-style, 0=success, 1=warnings, 2=errors
}
catch (DatabaseError_t dberr)
{
cerr << programName << ": Unexpected database error: " << dberr << "file url: " << url << endl;
// log any database errors
cerr << program_name << ": Unexpected database error: " << dberr << "file url: " << url << endl;
return 2; // error
}
catch (...)
{
cerr << programName << ": Unexpected error file url: " << url << endl;
// log any other errors
cerr << program_name << ": Unexpected error file url: " << url << endl;
return 2; // error
}
}
//
// required override: what is this step's name?
//
string getStepName(void) const override
{
return string("stack_stamp");
return program_name;
}
private:
const string programName = string("libstack_stamp.so");
bool verbose = false;
Stamper::StampValue_t stamp_value=-1;
// data
const string program_name = string("stack_stamp"); // this programs nam
bool verbose = false; // use verbose mode?
Stamper::StampValue_t stamp_value=-1; // how should we stamp?
// methods
//
// optional print usage for this program
//
void usage(const string& name)
{
cerr<<"Usage: "<<name<<endl;
......@@ -134,6 +171,10 @@ class StackStampDriver_t : public IRDB_SDK::TransformStep_t
};
//
// Required interface: libstack_stamp.so needs to have this factory function so thanos can create the transform step object
//
extern "C"
shared_ptr<TransformStep_t> getTransformStep(void)
{
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment