DynamoRIO · derekbruening · Oct 19, 2023 · Oct 19, 2023 · Oct 19, 2023 · Oct 19, 2023
diff --git a/api/docs/release.dox b/api/docs/release.dox
@@ -132,6 +132,10 @@ changes:
    INT_MATH has been removed and replaced with MATH.
    FP_MATH has been removed and replaced with FP|MATH.
    The enumeration was organized in a different order, the old numbers become invalid
+ - The #dynamorio::drmemtrace::TRACE_MARKER_TYPE_FUNC_RETVAL marker for system
+   calls changed to contain the actual return value, rather than just whether
+   successful.  A new marker #dynamorio::drmemtrace::TRACE_MARKER_TYPE_SYSCALL_FAILED
+   was added to indicate failure.
 
 Further non-compatibility-affecting changes include:
  - Added core-sharded analysis tool support where traces are sharded by

diff --git a/clients/drcachesim/common/trace_entry.h b/clients/drcachesim/common/trace_entry.h
@@ -322,7 +322,7 @@ typedef enum {
      * the drmemtrace_get_funclist_path() function's documentation.
      *
      * This marker is also used to record parameter values for certain system calls such
-     * as for #OFFLINE_FILE_TYPE_BLOCKING_SYSCALLS.  These use
+     * as for #OFFLINE_FILE_TYPE_BLOCKING_SYSCALLS or -record_syscall.  These use
      * large identifiers equal to
      * #func_trace_t::TRACE_FUNC_ID_SYSCALL_BASE plus the system
      * call number (for 32-bit marker values just the bottom 16 bits of the system call
@@ -348,20 +348,18 @@ typedef enum {
      * #TRACE_MARKER_TYPE_FUNC_ID marker entry. The number of such
      * entries for one function invocation is equal to the specified argument in
      * -record_function (or pre-defined functions in -record_heap_value if
-     * -record_heap is specified).
+     * -record_heap is specified) or -record_syscall.
      */
     TRACE_MARKER_TYPE_FUNC_ARG,
 
     /**
      * The marker value contains the return value of the just-entered function,
      * whose id is specified by the closest previous
-     * #TRACE_MARKER_TYPE_FUNC_ID marker entry
+     * #TRACE_MARKER_TYPE_FUNC_ID marker entry.  This is a
+     * pointer-sized value from the conventional return value register.
      *
-     * The marker value for system calls (see
-     * #func_trace_t::TRACE_FUNC_ID_SYSCALL_BASE) is either 0
-     * (failure) or 1 (success), as obtained from dr_syscall_get_result_ex() via the
-     * "succeeded" field of #dr_syscall_result_info_t.  See the corresponding
-     * documentation for caveats about the accuracy of this value.
+     * For system calls, this may not be enough to determine whether the call
+     * succeeded. See #TRACE_MARKER_TYPE_SYSCALL_FAILED.
      */
     TRACE_MARKER_TYPE_FUNC_RETVAL,
 
@@ -542,6 +540,18 @@ typedef enum {
     // branch.  The reader converts this to the memref_t "indirect_branch_target" field.
     TRACE_MARKER_TYPE_BRANCH_TARGET,
 
+    /**
+     * This marker is emitted for system calls whose parameters are traced with
+     * -record_syscall.  It is emitted immediately after #TRACE_MARKER_TYPE_FUNC_RETVAL
+     * if prior the system call (whose id is specified by the closest previous
+     * #TRACE_MARKER_TYPE_FUNC_ID marker entry) failed.  Whether it failed is obtained
+     * from dr_syscall_get_result_ex() via the "succeeded" field of
+     * #dr_syscall_result_info_t.  See the corresponding documentation for caveats about
+     * the accuracy of this determination.  The marker value is the "errno_value" field
+     * of #dr_syscall_result_info_t.
+     */
+    TRACE_MARKER_TYPE_SYSCALL_FAILED,
+
     // ...
     // These values are reserved for future built-in marker types.
     // ...

diff --git a/clients/drcachesim/docs/drcachesim.dox.in b/clients/drcachesim/docs/drcachesim.dox.in
@@ -1482,8 +1482,9 @@ The -record_heap parameter requests recording of a pre-determined set
 of functions related to heap allocation.  The -record_heap_value
 paramter controls the contents of this set.
 
-The tracer also supports recording system call argument and success
-values via the option -record_syscall, which functions similarly to
+The tracer also supports recording system call argument and return
+values (along with whether the call failued) via the option -record_syscall, which
+functions similarly to
 -record_function with the system call number replacing the function
 name.
 

diff --git a/clients/drcachesim/tests/allasm-repstr-basic-counts.templatex b/clients/drcachesim/tests/allasm-repstr-basic-counts.templatex
@@ -11,45 +11,45 @@ Adios world!
 ---- <application exited with code 0> ----
 Basic counts tool results:
 Total counts:
-          98 total \(fetched\) instructions
-          26 total unique \(fetched\) instructions
+         103 total \(fetched\) instructions
+          31 total unique \(fetched\) instructions
            4 total non-fetched instructions
            0 total prefetches
            7 total data loads
            5 total data stores
            0 total icache flushes
            0 total dcache flushes
            1 total threads
-          44 total scheduling markers
+          48 total scheduling markers
            0 total transfer markers
            0 total function id markers
            0 total function return address markers
            0 total function argument markers
            0 total function return value markers
            0 total physical address \+ virtual address marker pairs
            0 total physical address unavailable markers
-          11 total system call number markers
+          12 total system call number markers
            0 total blocking system call markers
            4 total other markers
-         102 total encodings
+         107 total encodings
 Thread [0-9]* counts:
-          98 \(fetched\) instructions
-          26 unique \(fetched\) instructions
+         103 \(fetched\) instructions
+          31 unique \(fetched\) instructions
            4 non-fetched instructions
            0 prefetches
            7 data loads
            5 data stores
            0 icache flushes
            0 dcache flushes
-          44 scheduling markers
+          48 scheduling markers
            0 transfer markers
            0 function id markers
            0 function return address markers
            0 function argument markers
            0 function return value markers
            0 physical address \+ virtual address marker pairs
            0 physical address unavailable markers
-          11 system call number markers
+          12 system call number markers
            0 blocking system call markers
            4 other markers
-         102 encodings
+         107 encodings
diff --git a/clients/drcachesim/tests/allasm_repstr.asm b/clients/drcachesim/tests/allasm_repstr.asm
@@ -1,5 +1,5 @@
  /* **********************************************************
- * Copyright (c) 2021-2022 Google, Inc.  All rights reserved.
+ * Copyright (c) 2021-2023 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -72,6 +72,13 @@ repeat:
         cmp      ebx, 0
         jnz      repeat
 
+        // Test a syscall failure.
+        mov      rdi, 42          // Invalid file descriptor.
+        lea      rsi, hello_str
+        mov      rdx, 13          // sizeof(hello_str)
+        mov      eax, 1         // SYS_write
+        syscall
+
         // Exit.
         mov      rdi, 0           // exit code
         mov      eax, 231         // SYS_exit_group

diff --git a/clients/drcachesim/tests/burst_futex.cpp b/clients/drcachesim/tests/burst_futex.cpp
@@ -187,7 +187,7 @@ test_main(int argc, const char *argv[])
         }
         if (memref.marker.marker_type == TRACE_MARKER_TYPE_FUNC_RETVAL) {
             // Should have succeeded.
-            assert(memref.marker.marker_value == 1);
+            assert(memref.marker.marker_value == 0);
         }
     }
     assert(saw_maybe_blocking);

diff --git a/clients/drcachesim/tests/offline-allasm-record-syscall.templatex b/clients/drcachesim/tests/offline-allasm-record-syscall.templatex
@@ -19,6 +19,16 @@ Adios world!
           50          20:   .* <marker: function argument 0x.*>
           51          20:   .* <marker: function argument 0xd>
           52          20:   .* <marker: function==syscall #1>
-          53          20:   .* <marker: function return value 0x1>
+          53          20:   .* <marker: function return value 0xd>
           54          20:   .* <marker: timestamp .*>
+.*
+         246         100:   .* <marker: system call 1>
+         247         100:   .* <marker: maybe-blocking system call>
+         248         100:   .* <marker: function==syscall #1>
+         249         100:   .* <marker: function argument 0x2a>
+         250         100:   .* <marker: function argument 0x.*>
+         251         100:   .* <marker: function argument 0xd>
+         252         100:   .* <marker: function==syscall #1>
+         253         100:   .* <marker: function return value 0xfffffffffffffff7>
+         254         100:   .* <marker: system call failed: 9>
 .*
diff --git a/clients/drcachesim/tests/offline-allasm-repstr-basic-counts.templatex b/clients/drcachesim/tests/offline-allasm-repstr-basic-counts.templatex
@@ -10,45 +10,45 @@ Adios world!
 Adios world!
 Basic counts tool results:
 Total counts:
-          98 total \(fetched\) instructions
-          26 total unique \(fetched\) instructions
+         103 total \(fetched\) instructions
+          31 total unique \(fetched\) instructions
            4 total non-fetched instructions
            0 total prefetches
            7 total data loads
            5 total data stores
            0 total icache flushes
            0 total dcache flushes
            1 total threads
-          44 total scheduling markers
+          48 total scheduling markers
            0 total transfer markers
            0 total function id markers
            0 total function return address markers
            0 total function argument markers
            0 total function return value markers
            0 total physical address \+ virtual address marker pairs
            0 total physical address unavailable markers
-          11 total system call number markers
-          10 total blocking system call markers
+          12 total system call number markers
+          11 total blocking system call markers
            5 total other markers
-          26 total encodings
+          31 total encodings
 Thread [0-9]* counts:
-          98 \(fetched\) instructions
-          26 unique \(fetched\) instructions
+         103 \(fetched\) instructions
+          31 unique \(fetched\) instructions
            4 non-fetched instructions
            0 prefetches
            7 data loads
            5 data stores
            0 icache flushes
            0 dcache flushes
-          44 scheduling markers
+          48 scheduling markers
            0 transfer markers
            0 function id markers
            0 function return address markers
            0 function argument markers
            0 function return value markers
            0 physical address \+ virtual address marker pairs
            0 physical address unavailable markers
-          11 system call number markers
-          10 blocking system call markers
+          12 system call number markers
+          11 blocking system call markers
            5 other markers
-          26 encodings
+          31 encodings
diff --git a/clients/drcachesim/tests/offline-windows-asm.templatex b/clients/drcachesim/tests/offline-windows-asm.templatex
@@ -22,8 +22,8 @@ Adios world!
 Hit tracing window #5 limit: disabling tracing.
 Basic counts tool results:
 Total counts:
-          53 total \(fetched\) instructions
-          21 total unique \(fetched\) instructions
+          55 total \(fetched\) instructions
+          23 total unique \(fetched\) instructions
            4 total non-fetched instructions
            0 total prefetches
            7 total data loads
@@ -40,9 +40,9 @@ Total counts:
            0 total physical address \+ virtual address marker pairs
            0 total physical address unavailable markers
            6 total system call number markers
-           5 total blocking system call markers
+           6 total blocking system call markers
           12 total other markers
-          21 total encodings
+          23 total encodings
 Total windows: 7
 Window #0:
           15 window \(fetched\) instructions
@@ -150,8 +150,8 @@ Window #4:
            1 window other markers
            0 window encodings
 Window #5:
-           6 window \(fetched\) instructions
-           6 window unique \(fetched\) instructions
+           8 window \(fetched\) instructions
+           8 window unique \(fetched\) instructions
            0 window non-fetched instructions
            0 window prefetches
            0 window data loads
@@ -167,9 +167,9 @@ Window #5:
            0 window physical address \+ virtual address marker pairs
            0 window physical address unavailable markers
            1 window system call number markers
-           0 window blocking system call markers
+           1 window blocking system call markers
            1 window other markers
-           3 window encodings
+           5 window encodings
 Window #6:
            0 window \(fetched\) instructions
            0 window unique \(fetched\) instructions

diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
@@ -396,6 +396,10 @@ view_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
             std::cerr << "<marker: function return value 0x" << std::hex
                       << memref.marker.marker_value << std::dec << ">\n";
             break;
+        case TRACE_MARKER_TYPE_SYSCALL_FAILED:
+            std::cerr << "<marker: system call failed: " << memref.marker.marker_value
+                      << ">\n";
+            break;
         case TRACE_MARKER_TYPE_RECORD_ORDINAL:
             std::cerr << "<marker: record ordinal 0x" << std::hex
                       << memref.marker.marker_value << std::dec << ">\n";

diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp
@@ -1632,23 +1632,21 @@ event_post_syscall(void *drcontext, int sysnum)
         if (hashtable_lookup(&syscall2args,
                              reinterpret_cast<void *>(static_cast<ptr_int_t>(sysnum))) !=
             nullptr) {
-            dr_syscall_result_info_t info = {
-                sizeof(info),
-            };
+            dr_syscall_result_info_t info = {};
+            info.size = sizeof(info);
+            info.use_errno = true;
             dr_syscall_get_result_ex(drcontext, &info);
             BUF_PTR(data->seg_base) += instru->append_marker(
                 BUF_PTR(data->seg_base), TRACE_MARKER_TYPE_FUNC_ID,
                 static_cast<uintptr_t>(func_trace_t::TRACE_FUNC_ID_SYSCALL_BASE) +
                     IF_X64_ELSE(sysnum, (sysnum & 0xffff)));
-            /* XXX i#5843: Return values are complex and can include more than just
-             * the primary register value.  Since we care mostly just about failure,
-             * we use the "succeeded" field.  However, this is not accurate for all
-             * syscalls.  Plus, would the scheduler want to know about various
-             * successful return values which indicate how many waiters were woken
-             * up and other data?
-             */
             BUF_PTR(data->seg_base) += instru->append_marker(
-                BUF_PTR(data->seg_base), TRACE_MARKER_TYPE_FUNC_RETVAL, info.succeeded);
+                BUF_PTR(data->seg_base), TRACE_MARKER_TYPE_FUNC_RETVAL, info.value);
+            if (!info.succeeded) {
+                BUF_PTR(data->seg_base) += instru->append_marker(
+                    BUF_PTR(data->seg_base), TRACE_MARKER_TYPE_SYSCALL_FAILED,
+                    info.errno_value);
+            }
         }
     }
 #endif