Skip to content

Commit

Permalink
i#5954: Adjust rseq trace path to match real path
Browse files Browse the repository at this point in the history
With our run-twice rseq solution, we first run an instrumented path
through the rseq region which goes into the recorded drmemtrace trace.
But the native rseq execution might take a side exit or abort.  We
already had code to try to roll back the committing store on an abort,
but it was fragile as a timestamp in between thwarted it.  We had no
code to make a side exit match and as a result we'd see PC
discontinuities.

Here, we solve both problems with the following approach:

First, we have DR add a new DR_NOTE_RSEQ_ENTRY label to the top of the
first block in an rseq region.  This label's data stores the end PC
and the abort handler PC (we do not currently use the latter in
drmemtrace: once we revert #4041 we may need to do so).

Next, the tracer turns that label into a new
TRACE_MARKER_TYPE_RSEQ_ENTRY marker with the end PC as the marker
value.

Finally, rawtrace uses the new label to fix up the trace.  When it
sees the entry marker, it starts buffering instead of writing out
records.  While buffering, it remembers the decode PC of each
insruction (for encoding entries), plus the PC + target of each
branch.  When it sees the rseq end PC, it sets a flag, and on the next
instruction it has the rseq exit PC.  If it sees an rseq abort marker
or a signal marker with the flag set, it uses the interrupted PC as
the rseq exit PC (this avoids having to wait for the other side of the
signal handler).

With this rseq exit PC, raw2trace takes the following actions
depending on the PC value:
+ End PC fall-through: completed; emit entire buffer.
+ Committing store PC: roll back final instr in buffer.
  (Once #4041 reverts this to the handler PC we'll use the handler PC here
  and will add a new marker to hold that data.)
+ Target of a branch in the buffer: roll back to that branch.
  If multiple: take most recent.
+ None of the above: there cannot be an indirect branch we observed so
  we assume this is an inverted cbr.  If there is a forward direct cbr
  which was taken and stayed in the region, we synthesize a direct
  jump in the gap it jumped over.  This is not perfect but is the
  best we can readily do.

We leave the old rollback method (which didn't handle a timestamp in
between) in place but it is only enabled if we see an rseq abort
marker without the new rseq entry marker first.  We decided to not
bump the trace version nor add a new filetype to indicate the presence
of rseq entry markers as we can support legacy traces this way (at
least as well as they were supported before).

We give up on identifying which of multiple indirect rseq side exits were taken.
We give up on distinguishing a side exit whose target is the end PC fall-through from a
completion.

For emitting the accumulated rseq buffer, the delayed branch code
added for #5941 will handle chunk boundaries.

A side improvements along the way: we add a branch target pc to
instr_summary_t and avoided increasing its space by removing the
previously-stored next_pc and computing it from the length.

Adds a number of unit tests for rseq rollback and side exits, along
with a more-real instance in the linux.rseq test.  This was also
tested on a large proprietary application with multiple real instances
of rseq aborts and side exits.

Issue: #5953, #5953, #4041
Fixes #5953
Fixes #5954
  • Loading branch information
derekbruening committed Apr 11, 2023
1 parent 282a691 commit 0a635dd
Show file tree
Hide file tree
Showing 18 changed files with 1,264 additions and 52 deletions.
2 changes: 2 additions & 0 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ Further non-compatibility-affecting changes include:
query key attributes of each input trace.
- Added instr_create_1dst_6src() convenience function that returns an instr_t
with one destination and six sources.
- Added a new label to help in handling "rseq" (Linux restartable sequence) regions:
#DR_NOTE_RSEQ_ENTRY.

**************************************************
<hr>
Expand Down
19 changes: 18 additions & 1 deletion clients/drcachesim/common/trace_entry.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2015-2022 Google, Inc. All rights reserved.
* Copyright (c) 2015-2023 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -424,6 +424,13 @@ typedef enum {
* warmup part of the trace ends.
*/
TRACE_MARKER_TYPE_FILTER_ENDPOINT,

/**
* Indicates the start of an "rseq" (Linux restartable sequence) region. The marker
* value holds the end PC of the region (this is the PC after the committing store).
*/
TRACE_MARKER_TYPE_RSEQ_ENTRY,

// ...
// These values are reserved for future built-in marker types.
// ...
Expand Down Expand Up @@ -487,6 +494,16 @@ type_has_address(const trace_type_t type)
type == TRACE_TYPE_DATA_FLUSH || type == TRACE_TYPE_DATA_FLUSH_END;
}

/** Returns whether the type represents an operand of an instruction. */
static inline bool
type_is_data(const trace_type_t type)
{
return type == TRACE_TYPE_INSTR_MAYBE_FETCH || type_is_prefetch(type) ||
type == TRACE_TYPE_READ || type == TRACE_TYPE_WRITE ||
type == TRACE_TYPE_INSTR_FLUSH || type == TRACE_TYPE_INSTR_FLUSH_END ||
type == TRACE_TYPE_DATA_FLUSH || type == TRACE_TYPE_DATA_FLUSH_END;
}

static inline bool
marker_type_is_function_marker(const trace_marker_type_t mark)
{
Expand Down
655 changes: 647 additions & 8 deletions clients/drcachesim/tests/raw2trace_unit_tests.cpp

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion clients/drcachesim/tools/view.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2017-2022 Google, Inc. All rights reserved.
* Copyright (c) 2017-2023 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -310,6 +310,10 @@ view_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
std::cerr << "<marker: rseq abort from 0x" << std::hex
<< memref.marker.marker_value << std::dec << " to handler>\n";
break;
case TRACE_MARKER_TYPE_RSEQ_ENTRY:
std::cerr << "<marker: rseq entry with end at 0x" << std::hex
<< memref.marker.marker_value << std::dec << ">\n";
break;
case TRACE_MARKER_TYPE_KERNEL_XFER:
if (trace_version_ <= TRACE_ENTRY_VERSION_NO_KERNEL_PC) {
// Legacy traces just have the module offset.
Expand Down
11 changes: 10 additions & 1 deletion clients/drcachesim/tracer/instru.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2016-2022 Google, Inc. All rights reserved.
* Copyright (c) 2016-2023 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -250,6 +250,9 @@ class instru_t {
instrument_instr_encoding(void *drcontext, void *tag, void *bb_field,
instrlist_t *ilist, instr_t *where, reg_id_t reg_ptr,
int adjust, instr_t *app) = 0;
virtual int
instrument_rseq_entry(void *drcontext, instrlist_t *ilist, instr_t *where,
instr_t *rseq_label, reg_id_t reg_ptr, int adjust) = 0;

virtual void
bb_analysis(void *drcontext, void *tag, void **bb_field, instrlist_t *ilist,
Expand Down Expand Up @@ -364,6 +367,9 @@ class online_instru_t : public instru_t {
instrument_instr_encoding(void *drcontext, void *tag, void *bb_field,
instrlist_t *ilist, instr_t *where, reg_id_t reg_ptr,
int adjust, instr_t *app) override;
int
instrument_rseq_entry(void *drcontext, instrlist_t *ilist, instr_t *where,
instr_t *rseq_label, reg_id_t reg_ptr, int adjust) override;

void
bb_analysis(void *drcontext, void *tag, void **bb_field, instrlist_t *ilist,
Expand Down Expand Up @@ -447,6 +453,9 @@ class offline_instru_t : public instru_t {
instrument_instr_encoding(void *drcontext, void *tag, void *bb_field,
instrlist_t *ilist, instr_t *where, reg_id_t reg_ptr,
int adjust, instr_t *app) override;
int
instrument_rseq_entry(void *drcontext, instrlist_t *ilist, instr_t *where,
instr_t *rseq_label, reg_id_t reg_ptr, int adjust) override;

void
bb_analysis(void *drcontext, void *tag, void **bb_field, instrlist_t *ilist,
Expand Down
30 changes: 29 additions & 1 deletion clients/drcachesim/tracer/instru_offline.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2016-2022 Google, Inc. All rights reserved.
* Copyright (c) 2016-2023 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -779,6 +779,34 @@ offline_instru_t::instrument_instr_encoding(void *drcontext, void *tag, void *bb
return adjust;
}

int
offline_instru_t::instrument_rseq_entry(void *drcontext, instrlist_t *ilist,
instr_t *where, instr_t *rseq_label,
reg_id_t reg_ptr, int adjust)
{
dr_instr_label_data_t *data = instr_get_label_data_area(rseq_label);
reg_id_t reg_tmp;
drreg_status_t res =
drreg_reserve_register(drcontext, ilist, where, reg_vector_, &reg_tmp);
DR_ASSERT(res == DRREG_SUCCESS); // Can't recover.
// We may need 2 entries for our marker. We write the entry
// marker with payload data[0]==rseq end. We don't need data[1]==rseq handler
// in raw2trace so we do not bother to emit a marker for it.
offline_entry_t entries[2];
int size =
append_marker((byte *)entries, TRACE_MARKER_TYPE_RSEQ_ENTRY, data->data[0]);
DR_ASSERT(size % sizeof(offline_entry_t) == 0);
size /= sizeof(offline_entry_t);
DR_ASSERT(size <= static_cast<int>(sizeof(entries)));
for (int i = 0; i < size; i++) {
adjust += insert_save_entry(drcontext, ilist, where, reg_ptr, reg_tmp, adjust,
&entries[i]);
}
res = drreg_unreserve_register(drcontext, ilist, where, reg_tmp);
DR_ASSERT(res == DRREG_SUCCESS); // Can't recover.
return adjust;
}

void
offline_instru_t::bb_analysis(void *drcontext, void *tag, void **bb_field,
instrlist_t *ilist, bool repstr_expanded)
Expand Down
10 changes: 10 additions & 0 deletions clients/drcachesim/tracer/instru_online.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,16 @@ online_instru_t::instrument_instr_encoding(void *drcontext, void *tag, void *bb_
return adjust;
}

int
online_instru_t::instrument_rseq_entry(void *drcontext, instrlist_t *ilist,
instr_t *where, instr_t *rseq_label,
reg_id_t reg_ptr, int adjust)
{
// TODO i#5953: Design a way to roll back records already sent to the analyzer.
// For now we live with discontinuities with online mode.
return adjust;
}

int
online_instru_t::instrument_ibundle(void *drcontext, instrlist_t *ilist, instr_t *where,
reg_id_t reg_ptr, int adjust, instr_t **delay_instrs,
Expand Down
Loading

0 comments on commit 0a635dd

Please sign in to comment.