From 6e3a93f761a2a16dfaa3be15d47a8057d28734be Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 10 Jan 2025 13:30:36 -0500 Subject: [PATCH 1/2] i#7180: Add drmemtrace support for stdin via "-infile -" Adds support for reading a single drmemtrace file from stdin when "-infile -" is specified. The file can be uncompressed or in gzip format. Adds a UNIX-only test which was easiest to create directly using bash. Fixes #7180 --- api/docs/release.dox | 2 ++ clients/drcachesim/common/options.cpp | 6 +++-- .../reader/compressed_file_reader.cpp | 10 +++++++-- clients/drcachesim/tests/offline-stdin.expect | 16 ++++++++++++++ suite/tests/CMakeLists.txt | 22 +++++++++++++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 clients/drcachesim/tests/offline-stdin.expect diff --git a/api/docs/release.dox b/api/docs/release.dox index 82471330ab5..40c40650760 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -133,6 +133,8 @@ Further non-compatibility-affecting changes include: a pure-static application via a new configure_DynamoRIO_static_client() CMake function and new "_drstatic" static library CMake targets for the provided extension libraries. + - Added support for reading a single drmemtrace trace file from stdin + via "-infile -". **************************************************
diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 58047dd40ad..642c30561f6 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2015-2024 Google, Inc. All rights reserved. + * Copyright (c) 2015-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -97,7 +97,9 @@ droption_t op_indir( droption_t op_infile( DROPTION_SCOPE_ALL, "infile", "", "Offline legacy file for input to the simulator", "Directs the simulator to use a single all-threads-interleaved-into-one trace file. " - "This is a legacy file format that is no longer produced."); + "This is a legacy file format that is no longer produced as it does not support " + "mixing multiple inputs nor using auxiliary metadata files. Passing '-' will read " + "from stdin as plain or gzip-compressed data."); droption_t op_jobs( DROPTION_SCOPE_ALL, "jobs", -1, "Number of parallel jobs", diff --git a/clients/drcachesim/reader/compressed_file_reader.cpp b/clients/drcachesim/reader/compressed_file_reader.cpp index 5007f558631..b0a62ed56dd 100644 --- a/clients/drcachesim/reader/compressed_file_reader.cpp +++ b/clients/drcachesim/reader/compressed_file_reader.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2017-2023 Google, Inc. All rights reserved. + * Copyright (c) 2017-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -52,7 +52,13 @@ namespace drmemtrace { bool open_single_file_common(const std::string &path, gzFile &out) { - out = gzopen(path.c_str(), "rb"); + if (path == "-") { + // We assume stdin is 0. + // We do not use "stdin->_fileno" as that is not portable. + constexpr int STDIN_FD = 0; + out = gzdopen(STDIN_FD, "rb"); + } else + out = gzopen(path.c_str(), "rb"); return out != nullptr; } diff --git a/clients/drcachesim/tests/offline-stdin.expect b/clients/drcachesim/tests/offline-stdin.expect new file mode 100644 index 00000000000..0ada73ad6e0 --- /dev/null +++ b/clients/drcachesim/tests/offline-stdin.expect @@ -0,0 +1,16 @@ +Output format: +<--record#-> <--instr#->: <---tid---> +------------------------------------------------------------ + 1 0: 1257596 + 2 0: 1257596 + 3 0: 1257596 + 4 0: 1257596 + 5 0: 1257596 + 6 0: 1257596 + 7 0: 1257596 + 8 1: 1257596 ifetch 4 byte\(s\) @ 0x00007f3e429fce35 48 8b 45 f8 mov -0x08\(%rbp\), %rax + 9 1: 1257596 read 8 byte\(s\) @ 0x00007ffefb03e128 by PC 0x00007f3e429fce35 + 10 2: 1257596 ifetch 8 byte\(s\) @ 0x00007f3e429fce39 48 8d 14 c5 00 00 00 lea 0x00\(,%rax,8\), %rdx + 10 2: 1257596 00 +View tool results: + 2 : total instructions diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 357f60c3f82..9b92963b0df 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4006,6 +4006,28 @@ if (BUILD_CLIENTS) # Avoid the default core-sharded from re-scheduling the trace. "-indir ${core_sharded_dir} -tool schedule_stats -only_shards 2,3 -no_core_sharded" "") set(tool.core_on_disk_rawtemp ON) # no preprocessor + + # Test reading a file over stdin. + # We rely on there being just one subfile in this zip file: gunzip only + # extracts the first one (bsdtar can extract all but is not likely installed). + # This is UNIX only so we rely on cat and gunzip being available. + find_program(CAT_EXE cat DOC "path to cat") + find_program(GUNZIP_EXE gunzip DOC "path to gunzip") + find_program(BASH_EXE bash DOC "path to bash") + if (CAT_EXE AND GUNZIP_EXE AND BASH_EXE) + # Easiest to make our pipeline in bash with a direct add_test() + # (although this bypasses the torun() features used by all other tests). + set(infile "${core_sharded_dir}/drmemtrace.core.0.trace.zip") + add_test(tool.drcacheoff.stdin + ${BASH_EXE} -c + "${CAT_EXE} ${infile} | ${GUNZIP_EXE} | ${drrun_path} -t drmemtrace -tool view -sim_refs 10 -infile -") + file(READ "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests/offline-stdin.expect" + stdin_expect) + set_tests_properties(tool.drcacheoff.stdin PROPERTIES + PASS_REGULAR_EXPRESSION "${stdin_expect}") + else () + message("Failed to find cat, gunzip, and bash: not running stdin test") + endif () endif () endif () From 27ca5a4ab572db9587ebd9d8839e1bcfab77397a Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 10 Jan 2025 22:08:57 -0500 Subject: [PATCH 2/2] Review suggestions --- clients/drcachesim/common/options.cpp | 13 ++++++++----- .../drcachesim/reader/compressed_file_reader.cpp | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 642c30561f6..cebeca16504 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -95,11 +95,14 @@ droption_t op_indir( "delete files here."); droption_t op_infile( - DROPTION_SCOPE_ALL, "infile", "", "Offline legacy file for input to the simulator", - "Directs the simulator to use a single all-threads-interleaved-into-one trace file. " - "This is a legacy file format that is no longer produced as it does not support " - "mixing multiple inputs nor using auxiliary metadata files. Passing '-' will read " - "from stdin as plain or gzip-compressed data."); + DROPTION_SCOPE_ALL, "infile", "", "Single offline trace input file", + "Directs the framework to use a single trace file. This could be a legacy " + "all-software-threads-interleaved-into-one trace file, a core-sharded single " + "hardware thread file mixing multiple software threads, or a single software " + "thread selected from a directory (though in that case it is better to use " + "-only_thread, -only_threads, or -only_shards). This method of input does " + "not support any features that require auxiliary metadata files. " + "Passing '-' will read from stdin as plain or gzip-compressed data."); droption_t op_jobs( DROPTION_SCOPE_ALL, "jobs", -1, "Number of parallel jobs", diff --git a/clients/drcachesim/reader/compressed_file_reader.cpp b/clients/drcachesim/reader/compressed_file_reader.cpp index b0a62ed56dd..9e441c1adab 100644 --- a/clients/drcachesim/reader/compressed_file_reader.cpp +++ b/clients/drcachesim/reader/compressed_file_reader.cpp @@ -54,7 +54,8 @@ open_single_file_common(const std::string &path, gzFile &out) { if (path == "-") { // We assume stdin is 0. - // We do not use "stdin->_fileno" as that is not portable. + // We do not use the glibc-specific "stdin->_fileno" method of finding the + // stdin descriptor number as that is not portable to musl or other libc. constexpr int STDIN_FD = 0; out = gzdopen(STDIN_FD, "rb"); } else