-
Notifications
You must be signed in to change notification settings - Fork 593
i#6751 trace_entry_t: pretty printer #6771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 15 commits
b48eed0
20db00b
dd80234
b10b1a2
00eefa0
8788d3b
793f6a4
c7881a6
b3a3d38
2af0d67
8b1ef16
73f8295
58c6032
cdb3eaa
612a8ea
81e6b24
81ff649
c940f18
5390027
7baee4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -165,13 +165,15 @@ add_exported_library(drmemtrace_opcode_mix STATIC tools/opcode_mix.cpp) | |
add_exported_library(drmemtrace_syscall_mix STATIC tools/syscall_mix.cpp) | ||
edeiana marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That page should be updated to point at this new tool, but please leave the |
||
add_exported_library(drmemtrace_view STATIC tools/view.cpp) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a unit test similar to clients/drcachesim/tests/view_test.cpp. Also some test that verifies the output for some checked in raw trace (e.g. clients/drcachesim/tests/drmemtrace.threadsig.aarch64.raw/raw) like clients/drcachesim/tests/offline-view.templatex. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's in the TODO (see PR description). Here's what it prints now (a short "representative" snippet, note that I OMITTED a few things because I wasn't sure I could share them here... the actual output has those values):
Is it acceptable? Do we need "--------" dividers? A header at the top? Do we want to number each line? Tabs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's worth spending any more time on this (esp for things like dividers and ordinals that the main view tool already has): this PR is just adding an internal tool that honestly will likely be rarely used, as evidenced by it not being needed in the past. Even now there likely is no use case where you wouldn't also go run the final view tool afterward, since that's the real view seen by tools. E.g., for your own ISA transform and filtering, you can't just go by the output of this record view tool: you have to make sure it works at the memref_t layer, so you have to use the real view tool plus invariant checks anyway. This internal tool is only a limited-use debugging step pretty much always followed up by using the "real" pretty printer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking more about when I've looked at the on-disk view with Not sure there's an action item here: just an observation that this new tool is not enough to debug all on-disk issues as it abstracts away the file splits and I'm not sure it would have been useful the last time I did that. Adding ordinals would help -- but I don't know it's worth the effort. I would probably go run the regular view tool skipped to the chunk transition and take the closest timestamps and search for that in this tool's output or sthg. But I would again want to be at the real file layer instead of the trace_entry_t stream layer's abstracting away of the file divisions. |
||
add_exported_library(drmemtrace_func_view STATIC tools/func_view.cpp) | ||
add_exported_library(drmemtrace_record_view STATIC tools/record_view.cpp) | ||
add_exported_library(drmemtrace_invariant_checker STATIC tools/invariant_checker.cpp) | ||
add_exported_library(drmemtrace_schedule_stats STATIC tools/schedule_stats.cpp) | ||
|
||
target_link_libraries(drmemtrace_invariant_checker drdecode) | ||
|
||
configure_DynamoRIO_standalone(drmemtrace_opcode_mix) | ||
configure_DynamoRIO_standalone(drmemtrace_view) | ||
configure_DynamoRIO_standalone(drmemtrace_record_view) | ||
configure_DynamoRIO_standalone(drmemtrace_invariant_checker) | ||
|
||
# We combine the cache and TLB simulators as they share code already. | ||
|
@@ -276,8 +278,8 @@ configure_DynamoRIO_standalone(drcachesim) | |
target_link_libraries(drcachesim drmemtrace_simulator drmemtrace_reuse_distance | ||
drmemtrace_histogram drmemtrace_reuse_time drmemtrace_basic_counts | ||
drmemtrace_opcode_mix drmemtrace_syscall_mix drmemtrace_view drmemtrace_func_view | ||
drmemtrace_raw2trace directory_iterator drmemtrace_invariant_checker | ||
drmemtrace_schedule_stats drmemtrace_record_filter) | ||
drmemtrace_record_view drmemtrace_raw2trace directory_iterator | ||
drmemtrace_invariant_checker drmemtrace_schedule_stats drmemtrace_record_filter) | ||
if (UNIX) | ||
target_link_libraries(drcachesim dl) | ||
endif () | ||
|
@@ -357,6 +359,7 @@ install_client_nonDR_header(drmemtrace simulator/cache_simulator_create.h) | |
install_client_nonDR_header(drmemtrace simulator/tlb_simulator_create.h) | ||
install_client_nonDR_header(drmemtrace tools/view_create.h) | ||
install_client_nonDR_header(drmemtrace tools/func_view_create.h) | ||
install_client_nonDR_header(drmemtrace tools/record_view_create.h) | ||
# TODO i#6412: Create a separate directory for non-tracer headers so that | ||
# we can more cleanly separate tracer and raw2trace code. | ||
install_client_nonDR_header(drmemtrace tracer/raw2trace.h) | ||
|
@@ -578,6 +581,7 @@ restore_nonclient_flags(drmemtrace_opcode_mix) | |
restore_nonclient_flags(drmemtrace_syscall_mix) | ||
restore_nonclient_flags(drmemtrace_view) | ||
restore_nonclient_flags(drmemtrace_func_view) | ||
restore_nonclient_flags(drmemtrace_record_view) | ||
restore_nonclient_flags(drmemtrace_record_filter) | ||
restore_nonclient_flags(drmemtrace_analyzer) | ||
restore_nonclient_flags(drmemtrace_invariant_checker) | ||
|
@@ -644,6 +648,7 @@ add_win32_flags(drmemtrace_opcode_mix) | |
add_win32_flags(drmemtrace_syscall_mix) | ||
add_win32_flags(drmemtrace_view) | ||
add_win32_flags(drmemtrace_func_view) | ||
add_win32_flags(drmemtrace_record_view) | ||
add_win32_flags(drmemtrace_record_filter) | ||
add_win32_flags(drmemtrace_analyzer) | ||
add_win32_flags(drmemtrace_invariant_checker) | ||
|
@@ -821,8 +826,9 @@ if (BUILD_TESTS) | |
drmemtrace_raw2trace drmemtrace_simulator drmemtrace_reuse_distance | ||
drmemtrace_histogram drmemtrace_reuse_time drmemtrace_basic_counts | ||
drmemtrace_opcode_mix drmemtrace_syscall_mix drmemtrace_view drmemtrace_func_view | ||
drmemtrace_raw2trace directory_iterator drmemtrace_invariant_checker | ||
drmemtrace_schedule_stats drmemtrace_analyzer drmemtrace_record_filter) | ||
drmemtrace_record_view drmemtrace_raw2trace directory_iterator | ||
drmemtrace_invariant_checker drmemtrace_schedule_stats drmemtrace_analyzer | ||
drmemtrace_record_filter) | ||
if (UNIX) | ||
target_link_libraries(tool.drcachesim.core_sharded dl) | ||
endif () | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,8 @@ | |
namespace dynamorio { | ||
namespace drmemtrace { | ||
|
||
/* Keep synched with trace_type_t enum in trace_entry.h. | ||
*/ | ||
const char *const trace_type_names[] = { | ||
"read", | ||
"write", | ||
|
@@ -88,5 +90,118 @@ const char *const trace_type_names[] = { | |
"untaken_jump", | ||
}; | ||
|
||
/* Keep synched with trace_version_t enum in trace_entry.h. | ||
*/ | ||
const char *const trace_version_names[] = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since for trace version the enum value matters, let's add the corresponding enum value too to these strings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As in, for example: "trace_entry_version_no_kernel_pc" -> "trace_entry_version_no_kernel_pc_2" ? (2 is the enum value) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm considering a use case where the user wants to know whether their trace has a version more recent than some other version (some of our tools want a minimum trace version). Having the enum value would be easier, but I guess the user can always look up the enum value themselves using just the enum string. So it's fine either way I guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we print the enum value along side the string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, I didn't understand that the enum value had a meaning (larger == more recent trace version?). |
||
"<unknown>", "<unknown>", "no_kernel_pc", "kernel_pc", | ||
"encodings", "branch_info", "frequent_timestamps", | ||
}; | ||
|
||
/* Keep synched with trace_marker_type_t enum in trace_entry.h. | ||
*/ | ||
const char *const trace_marker_names[] = { | ||
edeiana marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would we want the strings at clients/drcachesim/tools/view.cpp to be replaced by these? Maybe. Add a TODO at view.cpp. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would change all the output recorded in the docs here and in the sample traces repo and in internal docs so would not be taken lightly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reverse might be an easier thing to do. See all the comments about sharing code: eliminate this array and use the existing public pretty-printer's marker printing code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I modified this array to be consistent with view.cpp (which, now, view.cpp also uses). Now, |
||
"marker: kernel xfer", /* TRACE_MARKER_TYPE_KERNEL_EVENT */ | ||
"marker: syscall xfer", /* TRACE_MARKER_TYPE_KERNEL_XFER */ | ||
"marker: timestamp", /* TRACE_MARKER_TYPE_TIMESTAMP */ | ||
"marker: cpu id", /* TRACE_MARKER_TYPE_CPU_ID */ | ||
"marker: function", /* TRACE_MARKER_TYPE_FUNC_ID */ | ||
"marker: function return address", /* TRACE_MARKER_TYPE_FUNC_RETADDR */ | ||
"marker: function argument", /* TRACE_MARKER_TYPE_FUNC_ARG */ | ||
"marker: function return value", /* TRACE_MARKER_TYPE_FUNC_RETVAL */ | ||
"marker: split value", /* TRACE_MARKER_TYPE_SPLIT_VALUE */ | ||
"marker: filetype", /* TRACE_MARKER_TYPE_FILETYPE */ | ||
"marker: cache line size", /* TRACE_MARKER_TYPE_CACHE_LINE_SIZE */ | ||
"marker: instruction count", /* TRACE_MARKER_TYPE_INSTRUCTION_COUNT */ | ||
"marker: version", /* TRACE_MARKER_TYPE_VERSION */ | ||
"marker: rseq abort", /* TRACE_MARKER_TYPE_RSEQ_ABORT */ | ||
"marker: window", /* TRACE_MARKER_TYPE_WINDOW_ID */ | ||
"marker: physical address", /* TRACE_MARKER_TYPE_PHYSICAL_ADDRESS */ | ||
"marker: physical address not available", /* TRACE_MARKER_TYPE_PHYSICAL_ADDRESS_NOT_ | ||
AVAILABLE */ | ||
"marker: virtual address", /* TRACE_MARKER_TYPE_VIRTUAL_ADDRESS */ | ||
"marker: page size", /* TRACE_MARKER_TYPE_PAGE_SIZE */ | ||
"marker: system call idx", /* TRACE_MARKER_TYPE_SYSCALL_IDX */ | ||
"marker: chunk instruction count", /* TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT */ | ||
"marker: chunk footer", /* TRACE_MARKER_TYPE_CHUNK_FOOTER */ | ||
"marker: record ordinal", /* TRACE_MARKER_TYPE_RECORD_ORDINAL */ | ||
"marker: filter endpoint", /* TRACE_MARKER_TYPE_FILTER_ENDPOINT */ | ||
"marker: rseq entry", /* TRACE_MARKER_TYPE_RSEQ_ENTRY */ | ||
"marker: system call", /* TRACE_MARKER_TYPE_SYSCALL */ | ||
"marker: maybe-blocking system call", /* TRACE_MARKER_TYPE_MAYBE_BLOCKING_SYSCALL */ | ||
"marker: trace start for system call", /* TRACE_MARKER_TYPE_SYSCALL_TRACE_START */ | ||
"marker: trace end for system call", /* TRACE_MARKER_TYPE_SYSCALL_TRACE_END */ | ||
"marker: indirect branch target", /* TRACE_MARKER_TYPE_BRANCH_TARGET */ | ||
"marker: system call failed", /* TRACE_MARKER_TYPE_SYSCALL_FAILED */ | ||
"marker: direct switch to thread", /* TRACE_MARKER_TYPE_DIRECT_THREAD_SWITCH */ | ||
"marker: wait for another core", /* TRACE_MARKER_TYPE_CORE_WAIT */ | ||
"marker: core is idle", /* TRACE_MARKER_TYPE_CORE_IDLE */ | ||
"marker: trace start for context switch", /* TRACE_MARKER_TYPE_CONTEXT_SWITCH_START */ | ||
"marker: trace end for context switch", /* TRACE_MARKER_TYPE_CONTEXT_SWITCH_END */ | ||
"marker: vector length", /* TRACE_MARKER_TYPE_VECTOR_LENGTH */ | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: unused", | ||
"marker: reserved end", /* TRACE_MARKER_TYPE_RESERVED_END */ | ||
}; | ||
edeiana marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
} // namespace drmemtrace | ||
} // namespace dynamorio |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,8 @@ | |
#define _TRACE_ENTRY_H_ 1 | ||
|
||
#include <memory> | ||
#include <sstream> | ||
#include <string> | ||
#include <stddef.h> | ||
#include <stdint.h> | ||
|
||
|
@@ -652,6 +654,8 @@ enum class func_trace_t : uint64_t { // VS2019 won't infer 64-bit with "enum {". | |
}; | ||
|
||
extern const char *const trace_type_names[]; | ||
extern const char *const trace_version_names[]; | ||
extern const char *const trace_marker_names[]; | ||
|
||
/** | ||
* Returns whether the type represents an instruction fetch. | ||
|
@@ -983,6 +987,121 @@ trace_arch_string(offline_file_type_t type) | |
: "unspecified"))); | ||
} | ||
|
||
/* Returns a string representation of marker type and corresponding marker value (if any) | ||
* together. | ||
*/ | ||
static inline std::string | ||
trace_marker_type_value_as_string(trace_marker_type_t marker_type, uintptr_t marker_value) | ||
{ | ||
std::stringstream ss; | ||
const char *marker_name = trace_marker_names[marker_type]; | ||
switch (marker_type) { | ||
/* Handle all the cases where marker_value doesn't matter. | ||
*/ | ||
case TRACE_MARKER_TYPE_FILTER_ENDPOINT: | ||
case TRACE_MARKER_TYPE_MAYBE_BLOCKING_SYSCALL: | ||
case TRACE_MARKER_TYPE_CORE_WAIT: | ||
case TRACE_MARKER_TYPE_CORE_IDLE: ss << "<" << marker_name << ">\n"; break; | ||
|
||
/* Handle all the cases where we simply print <marker_type marker_value>. | ||
*/ | ||
case TRACE_MARKER_TYPE_TIMESTAMP: | ||
case TRACE_MARKER_TYPE_CPU_ID: | ||
case TRACE_MARKER_TYPE_INSTRUCTION_COUNT: | ||
case TRACE_MARKER_TYPE_CACHE_LINE_SIZE: | ||
case TRACE_MARKER_TYPE_PAGE_SIZE: | ||
case TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT: | ||
case TRACE_MARKER_TYPE_SYSCALL: | ||
case TRACE_MARKER_TYPE_DIRECT_THREAD_SWITCH: | ||
case TRACE_MARKER_TYPE_WINDOW_ID: | ||
case TRACE_MARKER_TYPE_SYSCALL_IDX: | ||
ss << "<" << marker_name << " " << marker_value << ">\n"; | ||
break; | ||
/* Handle all the cases where we simply print <marker_type 0xmarker_value>. | ||
*/ | ||
case TRACE_MARKER_TYPE_FUNC_RETADDR: | ||
case TRACE_MARKER_TYPE_FUNC_ARG: | ||
case TRACE_MARKER_TYPE_FUNC_RETVAL: | ||
case TRACE_MARKER_TYPE_RECORD_ORDINAL: | ||
case TRACE_MARKER_TYPE_SPLIT_VALUE: | ||
case TRACE_MARKER_TYPE_BRANCH_TARGET: | ||
ss << "<" << marker_name << " 0x" << std::hex << marker_value << std::dec | ||
<< ">\n"; | ||
break; | ||
/* Handle all remaining cases where we want to print a more informative output. | ||
*/ | ||
case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: | ||
case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: | ||
ss << "<" << marker_name << " number " << marker_value << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_CONTEXT_SWITCH_START: | ||
case TRACE_MARKER_TYPE_CONTEXT_SWITCH_END: | ||
ss << "<" << marker_name << " type " << marker_value << ">\n"; | ||
break; | ||
/* We don't have a way to know the trace version here. This might be an offset, | ||
* but we don't make any distinction. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please elaborate as a new reader will have no idea what you mean: I only know b/c I remember a legacy version having offset instead of abs addr. |
||
*/ | ||
case TRACE_MARKER_TYPE_KERNEL_XFER: | ||
case TRACE_MARKER_TYPE_KERNEL_EVENT: | ||
ss << "<" << marker_name << " from 0x" << std::hex << marker_value << std::dec | ||
<< ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_VERSION: | ||
ss << "<" << marker_name << " " << static_cast<int>(marker_value) << " " | ||
<< trace_version_names[marker_value] << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_FILETYPE: | ||
ss << "<" << marker_name << " 0x" << std::hex | ||
<< static_cast<intptr_t>(marker_value) << std::dec << " " | ||
<< trace_arch_string((offline_file_type_t)marker_value) << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_RSEQ_ABORT: | ||
ss << "<" << marker_name << " from 0x" << std::hex << marker_value << std::dec | ||
<< " to handler>\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_RSEQ_ENTRY: | ||
ss << "<" << marker_name << " with end at 0x" << std::hex << marker_value | ||
<< std::dec << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_CHUNK_FOOTER: | ||
ss << "<" << marker_name << " #" << marker_value << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_PHYSICAL_ADDRESS: | ||
ss << "<" << marker_name << " for following virtual: 0x" << std::hex | ||
<< marker_value << std::dec << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_VIRTUAL_ADDRESS: | ||
ss << "<" << marker_name << " for prior physical: 0x" << std::hex << marker_value | ||
<< std::dec << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_PHYSICAL_ADDRESS_NOT_AVAILABLE: | ||
ss << "<" << marker_name << " for 0x" << std::hex << marker_value << std::dec | ||
<< ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_FUNC_ID: | ||
if (marker_value >= | ||
static_cast<intptr_t>(func_trace_t::TRACE_FUNC_ID_SYSCALL_BASE)) { | ||
ss << "<" << marker_name << "==syscall #" | ||
<< (marker_value - | ||
static_cast<uintptr_t>(func_trace_t::TRACE_FUNC_ID_SYSCALL_BASE)) | ||
<< ">\n"; | ||
} else { | ||
ss << "<" << marker_name << " #" << marker_value << ">\n"; | ||
} | ||
break; | ||
case TRACE_MARKER_TYPE_SYSCALL_FAILED: | ||
ss << "<" << marker_name << ": " << marker_value << ">\n"; | ||
break; | ||
case TRACE_MARKER_TYPE_VECTOR_LENGTH: | ||
ss << "<" << marker_name << " " << marker_value << " bytes>\n"; | ||
break; | ||
default: | ||
ss << "<marker: type " << marker_type << "; value " << marker_value << ">\n"; | ||
break; | ||
} | ||
return ss.str(); | ||
} | ||
|
||
/* We have non-client targets including this header that do not include API | ||
* headers defining IF_X86_ELSE, etc. Those don't need this function so we | ||
* simply exclude them. | ||
|
Uh oh!
There was an error while loading. Please reload this page.