i#2062 memtrace nonmod part 3: Kernel interruption PC (#6001)

Changes the kernel interruption PC for 64-bit from a modidx+modoffs
scheme that was trying to avoid a 2nd record but which failed to handle
a non-module PC to use the absolute PC (a recently added assert fires in
this case; previously we could assert or crash or continue with a bogus
value in raw2trace depending on the uninitialized value of modidx).

Bumps the raw offline version number. Updates raw2trace to handle both
the old version as modix+modoffs plus the new absolute PC version. Adds
new unit tests for both.

Adds a SIGILL to the burst_gencode trace which triggers the new tracer
assert and passes with this fix.

To build the test, adds 0-valued entries to operand enums:
DR_EXTEND_DEFAULT and DR_OPND_DEFAULT, to avoid C++ compiler warnings in
INSTR_CREATE_dc_ivac(). #6000 covers using those in all the AArch64
creation macros.

Issue: #2062

clients/drcachesim/common/trace_entry.h

@@ -248,12 +248,6 @@ typedef enum {
      * A restartable sequence abort handler is further identified by a prior
      * marker of type #TRACE_MARKER_TYPE_RSEQ_ABORT.
      */
-    /* Non-exported information since limited to raw offline traces:
-     * For raw offline traces, the value is in the form of the module index and offset
-     * (from the base, not the indexed segment) of type kernel_interrupted_raw_pc_t.
-     * For raw offline traces, a value of 0 can be assumed to target the start of a
-     * block and so there is no loss of accuracy when post-processing.
-     */
     TRACE_MARKER_TYPE_KERNEL_EVENT,
     /**
      * The subsequent instruction is the target of a system call that changes the
@@ -633,7 +627,8 @@ typedef enum {
 #define OFFLINE_FILE_VERSION_KERNEL_INT_PC 4
 #define OFFLINE_FILE_VERSION_HEADER_FIELDS_SWAP 5
 #define OFFLINE_FILE_VERSION_ENCODINGS 6
-#define OFFLINE_FILE_VERSION OFFLINE_FILE_VERSION_ENCODINGS
+#define OFFLINE_FILE_VERSION_XFER_ABS_PC 7
+#define OFFLINE_FILE_VERSION OFFLINE_FILE_VERSION_XFER_ABS_PC
 
 /**
  * Bitfields used to describe the high-level characteristics of both an
@@ -729,7 +724,8 @@ struct _offline_entry_t {
 } END_PACKED_STRUCTURE;
 typedef struct _offline_entry_t offline_entry_t;
 
-// This is the raw marker value for TRACE_MARKER_TYPE_KERNEL_*.
+// This is the raw marker value for TRACE_MARKER_TYPE_KERNEL_*
+// for legacy raw traces prior to OFFLINE_FILE_VERSION_XFER_ABS_PC.
 // It occupies 49 bits and so may require two raw entries.
 typedef union {
     struct {

clients/drcachesim/tests/burst_gencode.cpp

@@ -43,6 +43,9 @@
 #include "raw2trace_directory.h"
 #include "scheduler.h"
 #include <assert.h>
+#ifdef LINUX
+#    include <signal.h>
+#endif
 #include <fstream>
 #include <iostream>
 #include <string>
@@ -57,6 +60,25 @@ namespace {
  * Code generation.
  */
 
+#ifdef LINUX
+#    ifdef X86
+static constexpr int UD2A_LENGTH = 2;
+#    else
+static constexpr int UD2A_LENGTH = 4;
+#    endif
+void
+handle_signal(int signal, siginfo_t *siginfo, ucontext_t *ucxt)
+{
+    if (signal != SIGILL) {
+        std::cerr << "Unexpected signal " << signal << "\n";
+        return;
+    }
+    sigcontext_t *sc = SIGCXT_FROM_UCXT(ucxt);
+    sc->SC_XIP += UD2A_LENGTH;
+    return;
+}
+#endif
+
 class code_generator_t {
 public:
     explicit code_generator_t(bool verbose = false)
@@ -129,6 +151,20 @@ class code_generator_t {
         instrlist_append(ilist,
                          XINST_CREATE_store(dc, OPND_CREATE_MEMPTR(base, -ptrsz),
                                             opnd_create_reg(base)));
+
+#ifdef LINUX
+        // Test a signal in non-module code.
+#    ifdef X86
+        instrlist_append(ilist, INSTR_CREATE_ud2a(dc));
+#    elif defined(AARCH64)
+        // TODO i#4562: creating UDF is not yet supported so we create a
+        // privileged instruction to SIGILL for us.
+        instrlist_append(ilist, INSTR_CREATE_dc_ivac(dc, opnd_create_reg(base)));
+#    else
+        instrlist_append(ilist, INSTR_CREATE_udf(dc, OPND_CREATE_INT(0)));
+#    endif
+#endif
+
 #ifdef ARM
         // XXX: Maybe XINST_CREATE_return should create "bx lr" like we need here
         // instead of the pop into pc which assumes the entry pushed lr?
@@ -252,6 +288,10 @@ post_process()
 static std::string
 gather_trace()
 {
+#ifdef LINUX
+    intercept_signal(SIGILL, handle_signal, false);
+#endif
+
     if (!my_setenv("DYNAMORIO_OPTIONS",
 #if defined(LINUX) && defined(X64)
                    // We pass -satisfy_w_xor_x to further stress that option

clients/drcachesim/tests/raw2trace_unit_tests.cpp

@@ -68,6 +68,43 @@
 #    error Unsupported arch
 #endif
 
+// Module mapper for testing different module bounds but without encodings.
+class test_multi_module_mapper_t : public module_mapper_t {
+public:
+    struct bounds_t {
+        bounds_t(addr_t start, addr_t end)
+            : start(start)
+            , end(end)
+        {
+        }
+        addr_t start;
+        addr_t end;
+    };
+    test_multi_module_mapper_t(const std::vector<bounds_t> &modules)
+        : module_mapper_t(nullptr)
+        , bounds_(modules)
+    {
+        // Clear do_module_parsing error; we can't cleanly make virtual b/c it's
+        // called from the constructor.
+        last_error_ = "";
+    }
+
+protected:
+    void
+    read_and_map_modules() override
+    {
+        for (size_t i = 0; i < bounds_.size(); i++) {
+            modvec_.push_back(module_t("fake_module",
+                                       reinterpret_cast<app_pc>(bounds_[i].start),
+                                       nullptr, 0, bounds_[i].end - bounds_[i].start,
+                                       bounds_[i].end - bounds_[i].start, true));
+        }
+    }
+
+private:
+    std::vector<bounds_t> bounds_;
+};
+
 // Subclasses raw2trace_t and replaces the module_mapper_t with our own version.
 class raw2trace_test_t : public raw2trace_t {
 public:
@@ -97,6 +134,20 @@ class raw2trace_test_t : public raw2trace_t {
             new test_module_mapper_t(&instrs, drcontext));
         set_modmap_(module_mapper_.get());
     }
+    raw2trace_test_t(const std::vector<std::istream *> &input,
+                     const std::vector<std::ostream *> &output,
+                     const std::vector<test_multi_module_mapper_t::bounds_t> &modules,
+                     void *drcontext)
+        : raw2trace_t(nullptr, input, output, {}, INVALID_FILE, nullptr, nullptr,
+                      drcontext,
+                      // The sequences are small so we print everything for easier
+                      // debugging and viewing of what's going on.
+                      4)
+    {
+        module_mapper_ =
+            std::unique_ptr<module_mapper_t>(new test_multi_module_mapper_t(modules));
+        set_modmap_(module_mapper_.get());
+    }
 };
 
 class archive_ostream_test_t : public archive_ostream_t {
@@ -120,13 +171,13 @@ class archive_ostream_test_t : public archive_ostream_t {
 };
 
 offline_entry_t
-make_header()
+make_header(int version = OFFLINE_FILE_VERSION)
 {
     offline_entry_t entry;
     entry.extended.type = OFFLINE_TYPE_EXTENDED;
     entry.extended.ext = OFFLINE_EXT_TYPE_HEADER;
     entry.extended.valueA = OFFLINE_FILE_TYPE_DEFAULT;
-    entry.extended.valueB = OFFLINE_FILE_VERSION;
+    entry.extended.valueB = version;
     return entry;
 }
 
@@ -254,7 +305,8 @@ check_entry(std::vector<trace_entry_t> &entries, int &idx, unsigned short expect
 // Takes ownership of ilist and destroys it.
 bool
 run_raw2trace(void *drcontext, const std::vector<offline_entry_t> raw, instrlist_t *ilist,
-              std::vector<trace_entry_t> &entries, int chunk_instr_count = 0)
+              std::vector<trace_entry_t> &entries, int chunk_instr_count = 0,
+              const std::vector<test_multi_module_mapper_t::bounds_t> &modules = {})
 {
     // We need an istream so we use istringstream.
     std::ostringstream raw_out;
@@ -281,7 +333,7 @@ run_raw2trace(void *drcontext, const std::vector<offline_entry_t> raw, instrlist
         std::string error = raw2trace.do_conversion();
         CHECK(error.empty(), error);
         result = result_stream.str();
-    } else {
+    } else if (modules.empty()) {
         // We need an ostream to capture out.
         std::ostringstream result_stream;
         std::vector<std::ostream *> output;
@@ -292,8 +344,20 @@ run_raw2trace(void *drcontext, const std::vector<offline_entry_t> raw, instrlist
         std::string error = raw2trace.do_conversion();
         CHECK(error.empty(), error);
         result = result_stream.str();
+    } else {
+        // We need an ostream to capture out.
+        std::ostringstream result_stream;
+        std::vector<std::ostream *> output;
+        output.push_back(&result_stream);
+
+        // Run raw2trace with our subclass supplying module bounds.
+        raw2trace_test_t raw2trace(input, output, modules, drcontext);
+        std::string error = raw2trace.do_conversion();
+        CHECK(error.empty(), error);
+        result = result_stream.str();
     }
-    instrlist_clear_and_destroy(drcontext, ilist);
+    if (ilist != nullptr)
+        instrlist_clear_and_destroy(drcontext, ilist);
 
     // Now check the results.
     char *start = &result[0];
@@ -1714,6 +1778,100 @@ test_rseq_side_exit_inverted_with_timestamp(void *drcontext)
         check_entry(entries, idx, TRACE_TYPE_FOOTER, -1));
 }
 
+/* Tests pre-OFFLINE_FILE_VERSION_XFER_ABS_PC (module offset) handling. */
+bool
+test_xfer_modoffs(void *drcontext)
+{
+#ifndef X64
+    // Modoffs was only ever used for X64.
+    return true;
+#else
+    std::cerr << "\n===============\nTesting legacy kernel xfer values\n";
+    std::vector<test_multi_module_mapper_t::bounds_t> modules = {
+        { 100, 150 },
+        { 400, 450 },
+    };
+
+    kernel_interrupted_raw_pc_t interrupt;
+    interrupt.pc.modidx = 1;
+    interrupt.pc.modoffs = 42;
+
+    std::vector<offline_entry_t> raw;
+    // Version is < OFFLINE_FILE_VERSION_XFER_ABS_PC.
+    raw.push_back(make_header(OFFLINE_FILE_VERSION_ENCODINGS));
+    raw.push_back(make_tid());
+    raw.push_back(make_pid());
+    raw.push_back(make_line_size());
+    raw.push_back(make_timestamp());
+    raw.push_back(make_core());
+    raw.push_back(make_marker(TRACE_MARKER_TYPE_KERNEL_EVENT, interrupt.combined_value));
+    raw.push_back(make_exit());
+
+    std::vector<trace_entry_t> entries;
+    if (!run_raw2trace(drcontext, raw, nullptr, entries, 0, modules))
+        return false;
+    int idx = 0;
+    return (
+        check_entry(entries, idx, TRACE_TYPE_HEADER, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FILETYPE) &&
+        check_entry(entries, idx, TRACE_TYPE_THREAD, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_PID, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CACHE_LINE_SIZE) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER,
+                    TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_KERNEL_EVENT,
+                    static_cast<addr_t>(modules[interrupt.pc.modidx].start +
+                                        interrupt.pc.modoffs)) &&
+        check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_FOOTER, -1));
+#endif
+}
+
+/* Tests >=OFFLINE_FILE_VERSION_XFER_ABS_PC (absolute PC) handling. */
+bool
+test_xfer_absolute(void *drcontext)
+{
+    std::cerr << "\n===============\nTesting legacy kernel xfer values\n";
+    std::vector<test_multi_module_mapper_t::bounds_t> modules = {
+        { 100, 150 },
+        { 400, 450 },
+    };
+    constexpr addr_t INT_PC = 442;
+
+    std::vector<offline_entry_t> raw;
+    raw.push_back(make_header(OFFLINE_FILE_VERSION_XFER_ABS_PC));
+    raw.push_back(make_tid());
+    raw.push_back(make_pid());
+    raw.push_back(make_line_size());
+    raw.push_back(make_timestamp());
+    raw.push_back(make_core());
+    raw.push_back(make_marker(TRACE_MARKER_TYPE_KERNEL_EVENT, INT_PC));
+    raw.push_back(make_exit());
+
+    std::vector<trace_entry_t> entries;
+    if (!run_raw2trace(drcontext, raw, nullptr, entries, 0, modules))
+        return false;
+    int idx = 0;
+    return (
+        check_entry(entries, idx, TRACE_TYPE_HEADER, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FILETYPE) &&
+        check_entry(entries, idx, TRACE_TYPE_THREAD, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_PID, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CACHE_LINE_SIZE) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER,
+                    TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID) &&
+        check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_KERNEL_EVENT,
+                    INT_PC) &&
+        check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) &&
+        check_entry(entries, idx, TRACE_TYPE_FOOTER, -1));
+}
+
 int
 main(int argc, const char *argv[])
 {
@@ -1729,7 +1887,8 @@ main(int argc, const char *argv[])
         !test_rseq_rollback_with_chunks(drcontext) || !test_rseq_side_exit(drcontext) ||
         !test_rseq_side_exit_signal(drcontext) ||
         !test_rseq_side_exit_inverted(drcontext) ||
-        !test_rseq_side_exit_inverted_with_timestamp(drcontext))
+        !test_rseq_side_exit_inverted_with_timestamp(drcontext) ||
+        !test_xfer_modoffs(drcontext) || !test_xfer_absolute(drcontext))
         return 1;
     return 0;
 }

clients/drcachesim/tracer/raw2trace.cpp

@@ -1621,15 +1621,15 @@ raw2trace_t::get_marker_value(raw2trace_thread_data_t *tdata,
         return "TRACE_MARKER_TYPE_SPLIT_VALUE unexpected for 32-bit";
 #endif
     }
-#ifdef X64 // 32-bit has the absolute pc as the raw marker value.
+#ifdef X64 // 32-bit always had the absolute pc as the raw marker value.
     if ((*entry)->extended.valueB == TRACE_MARKER_TYPE_KERNEL_EVENT ||
         (*entry)->extended.valueB == TRACE_MARKER_TYPE_RSEQ_ABORT ||
         (*entry)->extended.valueB == TRACE_MARKER_TYPE_KERNEL_XFER) {
-        if (get_version(tdata) >= OFFLINE_FILE_VERSION_KERNEL_INT_PC) {
+        if (get_version(tdata) >= OFFLINE_FILE_VERSION_KERNEL_INT_PC &&
+            get_version(tdata) < OFFLINE_FILE_VERSION_XFER_ABS_PC) {
             // We convert the idx:offs to an absolute PC.
-            // TODO i#2062: For non-module code we don't have the block id and so
-            // cannot use this format.  We should probably just abandon this and
-            // always store the absolute PC.
+            // This doesn't work for non-module-code and is thus only present in
+            // legacy traces.
             kernel_interrupted_raw_pc_t raw_pc;
             raw_pc.combined_value = marker_val;
             DR_ASSERT(raw_pc.pc.modidx != PC_MODIDX_INVALID);
@@ -1641,7 +1641,7 @@ raw2trace_t::get_marker_value(raw2trace_thread_data_t *tdata,
                 marker_val, raw_pc.pc.modidx, modvec_()[raw_pc.pc.modidx].orig_seg_base,
                 pc);
             marker_val = reinterpret_cast<uintptr_t>(pc);
-        } // Else we've already marked as TRACE_ENTRY_VERSION_NO_KERNEL_PC.
+        } // For really old, we've already marked as TRACE_ENTRY_VERSION_NO_KERNEL_PC.
     }
 #endif
     *value = marker_val;