Cleanup

DynamoRIO · Dec 15, 2024 · 716a0ea · 716a0ea
1 parent 0e2df67
commit 716a0ea
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 24 deletions.
diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt
@@ -951,8 +951,7 @@ if (BUILD_TESTS)
     add_win32_flags(tool.drcacheoff.burst_aarch64_sys)
   endif ()
 
-  add_executable(tool.drcachesim.decode_cache_test
-                 tests/decode_cache_test.cpp)
+  add_executable(tool.drcachesim.decode_cache_test tests/decode_cache_test.cpp)
   configure_DynamoRIO_standalone(tool.drcachesim.decode_cache_test)
   add_win32_flags(tool.drcachesim.decode_cache_test)
   target_link_libraries(tool.drcachesim.decode_cache_test
@@ -962,8 +961,7 @@ if (BUILD_TESTS)
   set_tests_properties(tool.drcachesim.decode_cache_test PROPERTIES
     TIMEOUT ${test_seconds})
 
-  add_executable(tool.drcacheoff.opcode_mix_test
-                 tests/opcode_mix_test.cpp)
+  add_executable(tool.drcacheoff.opcode_mix_test tests/opcode_mix_test.cpp)
   configure_DynamoRIO_standalone(tool.drcacheoff.opcode_mix_test)
   add_win32_flags(tool.drcacheoff.opcode_mix_test)
   target_link_libraries(tool.drcacheoff.opcode_mix_test
@@ -1020,7 +1018,7 @@ if (BUILD_TESTS)
     configure_DynamoRIO_standalone(tool.drcacheoff.view_test)
     add_win32_flags(tool.drcacheoff.view_test)
     target_link_libraries(tool.drcacheoff.view_test drmemtrace_view drmemtrace_raw2trace
-      drmemtrace_analyzer drmemtrace_decode_cache test_helpers)
+      drmemtrace_analyzer test_helpers)
     use_DynamoRIO_extension(tool.drcacheoff.view_test drreg_static)
     use_DynamoRIO_extension(tool.drcacheoff.view_test drcovlib_static)
     use_DynamoRIO_extension(tool.drcacheoff.view_test drdecode)

diff --git a/clients/drcachesim/tests/decode_cache_test.cpp b/clients/drcachesim/tests/decode_cache_test.cpp
@@ -116,13 +116,13 @@ check_decode_caching(bool persist_instrs, bool use_module_mapper)
             decode_cache.get_decode_info(reinterpret_cast<app_pc>(memrefs[0].instr.addr));
         if (decode_info_nop == nullptr || !decode_info_nop->is_valid() ||
             !instr_is_nop(decode_info_nop->get_decoded_instr())) {
-            return "Unexpected decode info for nop instr";
+            return "Unexpected instr_decode_info_t for nop instr";
         }
         instr_decode_info_t *decode_info_ret =
             decode_cache.get_decode_info(reinterpret_cast<app_pc>(memrefs[1].instr.addr));
         if (decode_info_ret == nullptr || !decode_info_ret->is_valid() ||
             !instr_is_return(decode_info_ret->get_decoded_instr())) {
-            return "Unexpected decode info for ret instr";
+            return "Unexpected instr_decode_info_t for ret instr";
         }
     } else {
         // These are tests to verify the operation of instr_decode_cache_t: that it caches
@@ -136,7 +136,7 @@ check_decode_caching(bool persist_instrs, bool use_module_mapper)
         }
         if (decode_cache.get_decode_info(
                 reinterpret_cast<app_pc>(memrefs[0].instr.addr)) != nullptr) {
-            return "Unexpected decode info for never-seen pc";
+            return "Unexpected test_decode_info_t for never-seen pc";
         }
         std::string err = decode_cache.add_decode_info(memrefs[0].instr);
         if (err != "")
@@ -145,8 +145,7 @@ check_decode_caching(bool persist_instrs, bool use_module_mapper)
             decode_cache.get_decode_info(reinterpret_cast<app_pc>(memrefs[0].instr.addr));
         if (decode_info_nop == nullptr || !decode_info_nop->is_valid() ||
             !decode_info_nop->is_nop) {
-            return "Unexpected decode info for nop instr: " +
-                std::to_string(reinterpret_cast<uint64_t>(decode_info_nop)) + "\n";
+            return "Unexpected test_decode_info_t for nop instr";
         }
         err = decode_cache.add_decode_info(memrefs[1].instr);
         if (err != "")
@@ -155,8 +154,7 @@ check_decode_caching(bool persist_instrs, bool use_module_mapper)
             decode_cache.get_decode_info(reinterpret_cast<app_pc>(memrefs[1].instr.addr));
         if (decode_info_ret == nullptr || !decode_info_ret->is_valid() ||
             !decode_info_ret->is_ret) {
-            return "Unexpected decode info for ret instr: " +
-                std::to_string(reinterpret_cast<uint64_t>(decode_info_ret)) + "\n";
+            return "Unexpected test_decode_info_t for ret instr";
         }
         err = decode_cache.add_decode_info(memrefs[2].instr);
         if (err != "")
@@ -174,7 +172,7 @@ check_decode_caching(bool persist_instrs, bool use_module_mapper)
 }
 
 std::string
-check_missing_module_mapper()
+check_missing_module_mapper_and_no_encoding()
 {
     memref_t instr = gen_instr(TID_A);
     test_decode_cache_t<instr_decode_info_t> decode_cache(
@@ -220,7 +218,7 @@ test_main(int argc, const char *argv[])
         std::cerr << err << "\n";
         exit(1);
     }
-    err = check_missing_module_mapper();
+    err = check_missing_module_mapper_and_no_encoding();
     if (err != "") {
         std::cerr << err << "\n";
         exit(1);

diff --git a/clients/drcachesim/tests/opcode_mix_test.cpp b/clients/drcachesim/tests/opcode_mix_test.cpp
@@ -30,7 +30,7 @@
  * DAMAGE.
  */
 
-/* Tests for opcode_mix_t library. */
+/* Tests for the opcode_mix_t analysis tool. */
 
 #include <iostream>
 #include <vector>
@@ -47,8 +47,7 @@ class test_opcode_mix_t : public opcode_mix_t {
 public:
     // Pass a non-nullptr instrlist_t if the module mapper must be used.
     test_opcode_mix_t(instrlist_t *instrs)
-        : opcode_mix_t(instrs == nullptr ? "" : "use_mod_map", /*verbose=*/
-                       0,
+        : opcode_mix_t(/*module_file_path=*/"some_file", /*verbose=*/0,
                        /*alt_module_dir=*/"")
         , instrs_(instrs)
     {
@@ -68,9 +67,9 @@ class test_opcode_mix_t : public opcode_mix_t {
     {
         shard->decode_cache = std::unique_ptr<decode_cache_t<opcode_data_t>>(
             new test_decode_cache_t<opcode_data_t>(dcontext, false, instrs_));
-        if (!module_file_path.empty()) {
-            std::string err =
-                shard->decode_cache->use_module_mapper(module_file_path, alt_module_dir);
+        if (instrs_ != nullptr) {
+            std::string err = shard->decode_cache->use_module_mapper(
+                /*module_file_path=*/"", /*alt_module_dir=*/"");
             if (err != "")
                 return err;
         }
@@ -127,11 +126,11 @@ check_opcode_mix(bool use_module_mapper)
     if (mix.size() != 2) {
         return "Found incorrect count of opcodes";
     }
-    int nop_count = mix[instr_get_opcode(nop)];
+    int64_t nop_count = mix[instr_get_opcode(nop)];
     if (nop_count != 2) {
         return "Found incorrect nop count";
     }
-    int ret_count = mix[instr_get_opcode(ret)];
+    int64_t ret_count = mix[instr_get_opcode(ret)];
     if (ret_count != 1) {
         return "Found incorrect ret count";
     }

diff --git a/clients/drcachesim/tools/common/decode_cache.h b/clients/drcachesim/tools/common/decode_cache.h
@@ -38,7 +38,6 @@
 #ifndef _DECODE_CACHE_H_
 #define _DECODE_CACHE_H_ 1
 
-#include "dr_api.h"
 #include "memref.h"
 #include "tracer/raw2trace_shared.h"
 
@@ -324,7 +323,7 @@ template <class DecodeInfo> class decode_cache_t : public decode_cache_base_t {
      * marker. The user may call this API also if they deliberately need to use
      * the module mapper instead of the embedded encodings.
      *
-     * It is left up to the user to determine if and when the need to call this API.
+     * It is left up to the user to determine if and when this API is needed.
      * It is important to note that the trace filetype may be obtained using the
      * get_filetype() API on a \p memtrace_stream_t. However, not all instances of
      * \p memtrace_stream_t have the filetype available at init time (before the