Skip to content

Commit

Permalink
i#2511: isolate raw2trace (#2512)
Browse files Browse the repository at this point in the history
Splits out a self-contained library, which:
- only takes already opened std::istream and std::ostream-compatible files
and doesn't own them, so it can be used with more exotic filesystems;
- takes in the contents of the module file to avoid doing IO from
drmodtrack_offline_read();
- returns errors on failure;
- doesn't depend on dr_frontend.

Preserves the current behavior with raw2trace_helper_t, which walks
directories and opens files.

Adds a ptrace-based test checking that raw2trace doesn't open files.

TESTED: ctest

Fixes #2511
  • Loading branch information
s-kanev authored and derekbruening committed Jul 18, 2017
1 parent f8643b2 commit 66fa2c7
Show file tree
Hide file tree
Showing 11 changed files with 623 additions and 220 deletions.
29 changes: 24 additions & 5 deletions clients/drcachesim/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ add_library(simulator STATIC
simulator/tlb_simulator.cpp
)

add_library(raw2trace STATIC
tracer/raw2trace.cpp
tracer/raw2trace_directory.cpp
)
configure_DynamoRIO_standalone(raw2trace)

add_executable(drcachesim
launcher.cpp
analyzer.cpp
Expand All @@ -87,15 +93,13 @@ add_executable(drcachesim
${zlib_reader}
reader/ipc_reader.cpp
simulator/analyzer_interface.cpp
# We embed the raw2trace conversion for convenience:
tracer/raw2trace.cpp
tracer/instru.cpp
tracer/instru_online.cpp
)
# In order to embed raw2trace we need to be standalone:
configure_DynamoRIO_standalone(drcachesim)
# Link in our tools:
target_link_libraries(drcachesim simulator reuse_distance histogram reuse_time)
target_link_libraries(drcachesim simulator reuse_distance histogram reuse_time raw2trace)
# To avoid dup symbol errors between drinjectlib and the drdecode brought in
# by drfrontendlib we have to explicitly list drdecode up front:
target_link_libraries(drcachesim drdecode drinjectlib drconfiglib drfrontendlib)
Expand Down Expand Up @@ -164,10 +168,10 @@ install_client_header(tracer/drmemtrace.h)

add_executable(drraw2trace
tracer/raw2trace_launcher.cpp
tracer/raw2trace.cpp
tracer/instru.cpp
tracer/instru_online.cpp
)
target_link_libraries(drraw2trace raw2trace)
# To avoid dup symbol errors on some VS builds we list drdecode before DR:
target_link_libraries(drraw2trace drdecode)
configure_DynamoRIO_standalone(drraw2trace)
Expand Down Expand Up @@ -332,8 +336,23 @@ if (BUILD_TESTS)
add_win32_flags(tool.drcacheoff.burst_threads)
target_link_libraries(tool.drcacheoff.burst_threads ${libpthread})

# FIXME i#2099: the weak symbol is not supported on Windows.
if (UNIX)
if (NOT APPLE)
# uses ptrace and looks for linux-specific syscalls
add_executable(tool.drcacheoff.raw2trace_io tests/raw2trace_io.cpp
tracer/instru.cpp
tracer/instru_online.cpp)
configure_DynamoRIO_standalone(tool.drcacheoff.raw2trace_io)
add_win32_flags(tool.drcacheoff.raw2trace_io)
target_link_libraries(tool.drcacheoff.raw2trace_io raw2trace)
use_DynamoRIO_extension(tool.drcacheoff.raw2trace_io droption)
target_link_libraries(tool.drcacheoff.raw2trace_io drdecode)
use_DynamoRIO_extension(tool.drcacheoff.raw2trace_io drcovlib_static)
# Because we're leveraging instru_online code we have to link with drutil:
use_DynamoRIO_extension(tool.drcacheoff.raw2trace_io drutil_static)
endif ()

# FIXME i#2099: the weak symbol is not supported on Windows.
add_executable(tool.drcacheoff.burst_client tests/burst_static.cpp)
append_property_list(TARGET tool.drcacheoff.burst_client
COMPILE_DEFINITIONS "TEST_APP_DR_CLIENT_MAIN")
Expand Down
8 changes: 6 additions & 2 deletions clients/drcachesim/analyzer_multi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
# include "reader/compressed_file_reader.h"
#endif
#include "reader/ipc_reader.h"
#include "tracer/raw2trace_directory.h"
#include "tracer/raw2trace.h"

analyzer_multi_t::analyzer_multi_t()
Expand All @@ -65,8 +66,11 @@ analyzer_multi_t::analyzer_multi_t()
trace_iter = existing;
else {
delete existing;
raw2trace_t raw2trace(op_indir.get_value(), tracefile);
raw2trace.do_conversion();
raw2trace_directory_t dir(op_indir.get_value(), tracefile);
raw2trace_t raw2trace(dir.modfile_bytes, dir.thread_files, &dir.out_file);
std::string error = raw2trace.do_conversion();
if (!error.empty())
ERRMSG("raw2trace failed: %s", error.c_str());
trace_iter = new file_reader_t(tracefile.c_str());
}
// We don't support a compressed file here (is_complete() is too hard
Expand Down
1 change: 1 addition & 0 deletions clients/drcachesim/common/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#define BUFFER_SIZE_ELEMENTS(buf) (BUFFER_SIZE_BYTES(buf) / sizeof(buf[0]))
#define BUFFER_LAST_ELEMENT(buf) buf[BUFFER_SIZE_ELEMENTS(buf) - 1]
#define NULL_TERMINATE_BUFFER(buf) BUFFER_LAST_ELEMENT(buf) = 0
#define TESTANY(mask, var) (((mask) & (var)) != 0)

#define BOOLS_MATCH(b1, b2) (!!(b1) == !!(b2))

Expand Down
5 changes: 5 additions & 0 deletions clients/drcachesim/tests/raw2trace-simple.templatex
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
\[drmemtrace\]: Reading module file from memory
open|close
\[drmemtrace\]: Successfully read [0-9]+ modules
\[drmemtrace\]: Successfully converted [0-9]+ thread files
Processed
166 changes: 166 additions & 0 deletions clients/drcachesim/tests/raw2trace_io.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/* **********************************************************
* Copyright (c) 2017 Google, Inc. All rights reserved.
* **********************************************************/

/*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* * Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* * Neither the name of Google, Inc. nor the names of its contributors may be
* used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL GOOGLE, INC. OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
* DAMAGE.
*/

/* This application processes a set of existing raw files with raw2trace_t.
* It uses ptrace to make sure raw2trace only interacts with the filesystem when expected
* -- it doesn't open/close any files outside of mapping modules (which should be local).
*
* XXX: We use ptrace as opposed to running under memtrace with replaced file operations
* because raw2trace uses drmodtrack, which doesn't isolate under static memtrace.
*/

#include "droption.h"
#include "tracer/raw2trace.h"
#include "tracer/raw2trace_directory.h"
#include <cassert>
#include <errno.h>
#include <fcntl.h>
#include <iostream>
#include <string>
#include <sys/ptrace.h>
#include <sys/user.h>
#include <sys/wait.h>
#include <syscall.h>
#include <unistd.h>

static droption_t<std::string> op_indir(DROPTION_SCOPE_FRONTEND, "indir", "",
"[Required] Directory with trace input files",
"Specifies a directory with raw files.");

static droption_t<std::string> op_out(DROPTION_SCOPE_FRONTEND, "out", "",
"[Required] Path to output file",
"Specifies the path to the output file.");
#if defined(X64)
# define SYS_NUM(r) (r).orig_rax
#elif defined(X86)
# define SYS_NUM(r) (r).orig_eax
#else
# error "Test only supports x86"
#endif

#ifndef LINUX
# error "Test only supports Linux"
#endif

int
main(int argc, const char *argv[])
{
std::string parse_err;
if (!droption_parser_t::parse_argv(DROPTION_SCOPE_FRONTEND, argc, (const char **)argv,
&parse_err, NULL) ||
op_indir.get_value().empty() || op_out.get_value().empty()) {
std::cerr << "Usage error: " << parse_err << "\nUsage:\n"
<< droption_parser_t::usage_short(DROPTION_SCOPE_ALL);
return 1;
}

/* Open input/output files outside of traced region. And explicitly don't destroy dir,
* so they never get closed.
*/
raw2trace_directory_t *dir =
new raw2trace_directory_t(op_indir.get_value(), op_out.get_value());

pid_t pid = fork();
if (pid == -1) {
std::cerr << "Fork failed\n";
return 1;
}
if (pid != 0) {
/* parent */
long res;
int status;
while (true) {
int wait_res = waitpid(pid, &status, __WALL);
if (wait_res < 0) {
if (errno == EINTR)
continue;
perror("Failed waiting");
return 1;
}
if (WIFEXITED(status))
break;
user_regs_struct regs;
res = ptrace(PTRACE_GETREGS, pid, NULL, &regs);
if (res < 0) {
perror("ptrace failed");
return 1;
}
/* We don't distinguish syscall entry/exit because orig_rax is set
* on both.
*/
if (SYS_NUM(regs) == __NR_open || SYS_NUM(regs) == __NR_openat ||
SYS_NUM(regs) == __NR_creat) {
std::cerr << "open\n";
} else if (SYS_NUM(regs) == __NR_close) {
std::cerr << "close\n";
}
res = ptrace(PTRACE_SYSCALL, pid, NULL, 0);
if (res < 0) {
perror("ptrace failed");
return 1;
}
}
if (WIFEXITED(status)) {
return 0;
}
res = ptrace(PTRACE_DETACH, pid, NULL, NULL);
if (res < 0) {
perror("ptrace failed");
return 1;
}
std::cerr << "Detached\n";
return 0;
} else {
/* child */
long res = ptrace(PTRACE_TRACEME, 0, NULL, NULL);
if (res < 0) {
perror("ptrace me failed");
return 1;
}
/* Force a wait until parent attaches, so we don't race on the fork. */
raise(SIGSTOP);

/* Sycalls below will be ptraced. We don't expect any open/close calls outside of
* raw2trace::read_and_map_modules().
*/
raw2trace_t raw2trace(dir->modfile_bytes, dir->thread_files, &dir->out_file,
GLOBAL_DCONTEXT, 1);
std::string error = raw2trace.do_conversion();
if (!error.empty()) {
std::cerr << "raw2trace failed " << error << "\n";
return 1;
}
std::cerr << "Processed\n";
return 0;
}
return 0;
}
Loading

4 comments on commit 66fa2c7

@fhahn
Copy link
Contributor

@fhahn fhahn commented on 66fa2c7 Jul 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this commit broke AArch32 builds:

http://dynamorio.org/CDash/viewBuildError.php?buildid=26574

I'm surprised the build failure was not picked up by Travis, as I though it cross-compiles on AArch32. Maybe the tests are not built?

@derekbruening
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this commit broke AArch32 builds

@s-kanev -- I will fix this.

I'm surprised the build failure was not picked up by Travis, as I though it cross-compiles on AArch32. Maybe the tests are not built?

Yes, looking at the runsuite code, the tests are not built. I will add tests to the 2 debug builds.

@derekbruening
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derekbruening
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit fails to report errors in merge_and_process_thread_files(): errors instead turn into a truncated output file with success reported back to the user. Fix is in #2636.

Please sign in to comment.