i#3106: add analysis_tool_t::initialize() (#3319)

Adds a separate analysis_tool_t::initialize() method to better handle
tool initialization that can fail, separating it out from the
constructor.  This simplifies subclassing as well.

Updates the view_t and opcode_mix_t tools to move their module mapping
and other code into initialize().  Leaves updating the simulator tools
for future work.

Changes raw2trace_directory_t to return error strings instead of
aborting the process, and splits its constructor from new initialize()
and initialize_module_file() methods.

Fixes #3106

api/docs/release.dox

@@ -252,7 +252,8 @@ Further non-compatibility-affecting changes include:
    - Added #module_mapper_t, which factors out the module mapping functionality
      out of #raw2trace_t, replacing the following #raw2trace_t APIs:
      #raw2trace_t::handle_custom_data(), #raw2trace_t::do_module_parsing(),
-     #raw2trace_t::do_module_parsing_and_mapping(), and #raw2trace_t::find_mapped_trace_address().
+     #raw2trace_t::do_module_parsing_and_mapping(), and
+     #raw2trace_t::find_mapped_trace_address().
    - Added #trace_metadata_writer_t, a set of utility functions used by drcachesim's
      #raw2trace_t for writing trace metadata: process/thread ids, timestamps, etc.
    - Added #trace_metadata_reader_t, a set of utilities for checking and validating
@@ -272,6 +273,10 @@ Further non-compatibility-affecting changes include:
    in a hashtable with user data also available.
  - Added cmake function DynamoRIO_get_full_path that shall be used instead of reading
    the LOCATION target property.
+ - Added a drcachesim/drmemtrace analysis tool routine initialize() to help separate
+   initialization that could fail from tool construction.
+ - Split raw2trace_directory_t initialization from its constructors
+   into new initialize() and initialize_module_file() methods.
 
 **************************************************
 <hr>

clients/drcachesim/analysis_tool.h

@@ -66,6 +66,15 @@ class analysis_tool_t {
     analysis_tool_t()
         : success(true){};
     virtual ~analysis_tool_t(){}; /**< Destructor. */
+    /**
+     * Tools are encouraged to perform any initialization that might fail here rather
+     * than in the constructor.  On an error, this will set the success flag, and
+     * get_error_string() provides a descriptive error message.
+     */
+    virtual void
+    initialize()
+    {
+    }
     /** Returns whether the tool was created successfully. */
     virtual bool operator!()
     {

clients/drcachesim/analyzer.cpp

@@ -78,6 +78,8 @@ analyzer_t::analyzer_t(const std::string &trace_file, analysis_tool_t **tools_in
     , tools(tools_in)
 {
     for (int i = 0; i < num_tools; ++i) {
+        if (tools[i] != NULL && !!*tools[i])
+            tools[i]->initialize();
         if (tools[i] == NULL || !*tools[i]) {
             success = false;
             error_string = "Tool is not successfully initialized";

clients/drcachesim/analyzer.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2016-2017 Google, Inc.  All rights reserved.
+ * Copyright (c) 2016-2018 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -78,6 +78,7 @@ class analyzer_t {
      * The analyzer will reference the tools array passed in during its lifetime:
      * it does not make a copy.
      * The user must free them afterward.
+     * The analyzer calls the initialize() function on each tool before use.
      */
     analyzer_t(const std::string &trace_file, analysis_tool_t **tools, int num_tools);
     /** Launches the analysis process. */

clients/drcachesim/analyzer_multi.cpp

@@ -102,7 +102,12 @@ analyzer_multi_t::analyzer_multi_t()
         if (needs_processing) {
             delete existing;
             existing = nullptr;
-            raw2trace_directory_t dir(op_indir.get_value(), "", op_verbose.get_value());
+            raw2trace_directory_t dir(op_verbose.get_value());
+            std::string dir_err = dir.initialize(op_indir.get_value(), "");
+            if (!dir_err.empty()) {
+                success = false;
+                error_string = "Directory setup failed: " + dir_err;
+            }
             raw2trace_t raw2trace(dir.modfile_bytes, dir.in_files, dir.out_files, nullptr,
                                   op_verbose.get_value(), op_jobs.get_value());
             std::string error = raw2trace.do_conversion();
@@ -149,24 +154,30 @@ analyzer_multi_t::create_analysis_tools()
      */
     tools = new analysis_tool_t *[max_num_tools];
     tools[0] = drmemtrace_analysis_tool_create();
-    if (tools[0] != NULL && !*tools[0]) {
+    if (tools[0] == NULL)
+        return false;
+    if (!!*tools[0])
+        tools[0]->initialize();
+    if (!*tools[0]) {
         error_string = tools[0]->get_error_string();
         delete tools[0];
         tools[0] = NULL;
-    }
-    if (tools[0] == NULL)
         return false;
+    }
     num_tools = 1;
 #ifdef DEBUG
     if (op_test_mode.get_value()) {
         tools[1] = new trace_invariants_t(op_offline.get_value(), op_verbose.get_value());
-        if (tools[1] != NULL && !*tools[1]) {
+        if (tools[1] == NULL)
+            return false;
+        if (!!*tools[1])
+            tools[1]->initialize();
+        if (!*tools[1]) {
             error_string = tools[1]->get_error_string();
             delete tools[1];
             tools[1] = NULL;
-        }
-        if (tools[1] == NULL)
             return false;
+        }
         num_tools = 2;
     }
 #endif

clients/drcachesim/tests/burst_replace.cpp

@@ -185,7 +185,9 @@ post_process()
         /* First, test just the module parsing w/o writing a final trace, in a separate
          * scope to delete the drmodtrack state afterward.
          */
-        raw2trace_directory_t dir(raw_dir, "");
+        raw2trace_directory_t dir;
+        std::string dir_err = dir.initialize(raw_dir, "");
+        assert(dir_err.empty());
         std::unique_ptr<module_mapper_t> module_mapper = module_mapper_t::create(
             dir.modfile_bytes, parse_cb, process_cb, MAGIC_VALUE, free_cb);
         assert(module_mapper->get_last_error().empty());
@@ -200,7 +202,9 @@ post_process()
     /* Now write a final trace to a location that the drcachesim -indir step
      * run by the outer test harness will find (TRACE_FILENAME).
      */
-    raw2trace_directory_t dir(raw_dir, "");
+    raw2trace_directory_t dir;
+    std::string dir_err = dir.initialize(raw_dir, "");
+    assert(dir_err.empty());
     raw2trace_t raw2trace(dir.modfile_bytes, dir.in_files, dir.out_files, dr_context, 0);
     std::string error =
         raw2trace.handle_custom_data(parse_cb, process_cb, MAGIC_VALUE, free_cb);

clients/drcachesim/tests/raw2trace_io.cpp

@@ -262,7 +262,10 @@ main(int argc, const char *argv[])
     /* Open input/output files outside of traced region. And explicitly don't destroy dir,
      * so they never get closed.
      */
-    raw2trace_directory_t *dir = new raw2trace_directory_t(op_indir.get_value(), "");
+    raw2trace_directory_t *dir = new raw2trace_directory_t;
+    std::string dir_err = dir->initialize(op_indir.get_value(), "");
+    if (!dir_err.empty())
+        std::cerr << "Directory setup failed: " << dir_err;
 
     bool test1_ret = test_raw2trace(dir);
     bool test2_ret = test_module_mapper(dir);

clients/drcachesim/tools/opcode_mix.cpp

@@ -52,22 +52,33 @@ opcode_mix_tool_create(const std::string &module_file_path, unsigned int verbose
     return new opcode_mix_t(module_file_path, verbose);
 }
 
-opcode_mix_t::opcode_mix_t(const std::string &module_file_path, unsigned int verbose)
+opcode_mix_t::opcode_mix_t(const std::string &module_file_path_in, unsigned int verbose)
     : dcontext(nullptr)
-    , directory(module_file_path)
+    , module_file_path(module_file_path_in)
     , knob_verbose(verbose)
     , instr_count(0)
+{
+}
+
+void
+opcode_mix_t::initialize()
 {
     if (module_file_path.empty()) {
         error_string = "Module file path is missing";
         success = false;
         return;
     }
     dcontext = dr_standalone_init();
+    std::string error = directory.initialize_module_file(module_file_path);
+    if (!error.empty()) {
+        error_string = "Failed to initialize directory: " + error;
+        success = false;
+        return;
+    }
     module_mapper = module_mapper_t::create(directory.modfile_bytes, nullptr, nullptr,
-                                            nullptr, nullptr, verbose);
+                                            nullptr, nullptr, knob_verbose);
     module_mapper->get_loaded_modules();
-    std::string error = module_mapper->get_last_error();
+    error = module_mapper->get_last_error();
     if (!error.empty()) {
         error_string = "Failed to load binaries: " + error;
         success = false;

clients/drcachesim/tools/opcode_mix.h

@@ -46,13 +46,16 @@ class opcode_mix_t : public analysis_tool_t {
     virtual ~opcode_mix_t()
     {
     }
-    virtual bool
-    process_memref(const memref_t &memref);
-    virtual bool
-    print_results();
+    void
+    initialize() override;
+    bool
+    process_memref(const memref_t &memref) override;
+    bool
+    print_results() override;
 
 protected:
     void *dcontext;
+    std::string module_file_path;
     std::unique_ptr<module_mapper_t> module_mapper;
     // We reference directory.modfile_bytes throughout operation, so its lifetime
     // must match ours.

clients/drcachesim/tools/view.cpp

@@ -52,38 +52,49 @@ view_tool_create(const std::string &module_file_path, uint64_t skip_refs,
     return new view_t(module_file_path, skip_refs, sim_refs, syntax, verbose);
 }
 
-view_t::view_t(const std::string &module_file_path, uint64_t skip_refs, uint64_t sim_refs,
-               const std::string &syntax, unsigned int verbose)
+view_t::view_t(const std::string &module_file_path_in, uint64_t skip_refs,
+               uint64_t sim_refs, const std::string &syntax, unsigned int verbose)
     : dcontext(nullptr)
-    , directory(module_file_path)
+    , module_file_path(module_file_path_in)
     , knob_verbose(verbose)
     , instr_count(0)
     , knob_skip_refs(skip_refs)
     , knob_sim_refs(sim_refs)
+    , knob_syntax(syntax)
     , num_disasm_instrs(0)
+{
+}
+
+void
+view_t::initialize()
 {
     if (module_file_path.empty()) {
         error_string = "Module file path is missing";
         success = false;
         return;
     }
     dcontext = dr_standalone_init();
+    std::string error = directory.initialize_module_file(module_file_path);
+    if (!error.empty()) {
+        error_string = "Failed to initialize directory: " + error;
+        success = false;
+        return;
+    }
     module_mapper = module_mapper_t::create(directory.modfile_bytes, nullptr, nullptr,
-                                            nullptr, nullptr, verbose);
+                                            nullptr, nullptr, knob_verbose);
     module_mapper->get_loaded_modules();
-    std::string error = module_mapper->get_last_error();
+    error = module_mapper->get_last_error();
     if (!error.empty()) {
         error_string = "Failed to load binaries: " + error;
         success = false;
         return;
     }
-
     dr_disasm_flags_t flags = DR_DISASM_ATT;
-    if (syntax == "intel") {
+    if (knob_syntax == "intel") {
         flags = DR_DISASM_INTEL;
-    } else if (syntax == "dr") {
+    } else if (knob_syntax == "dr") {
         flags = DR_DISASM_DR;
-    } else if (syntax == "arm") {
+    } else if (knob_syntax == "arm") {
         flags = DR_DISASM_ARM;
     }
     disassemble_set_syntax(flags);

clients/drcachesim/tools/view.h

@@ -47,20 +47,24 @@ class view_t : public analysis_tool_t {
     virtual ~view_t()
     {
     }
-    virtual bool
-    process_memref(const memref_t &memref);
-    virtual bool
-    print_results();
+    void
+    initialize() override;
+    bool
+    process_memref(const memref_t &memref) override;
+    bool
+    print_results() override;
 
 protected:
     void *dcontext;
+    std::string module_file_path;
     std::unique_ptr<module_mapper_t> module_mapper;
     raw2trace_directory_t directory;
     unsigned int knob_verbose;
     uint64_t instr_count;
     static const std::string TOOL_NAME;
     uint64_t knob_skip_refs;
     uint64_t knob_sim_refs;
+    std::string knob_syntax;
     uint64_t num_disasm_instrs;
     std::unordered_map<app_pc, std::string> disasm_cache;
 };