Skip to content

Commit

Permalink
[lldb/Reproducers] Fix API boundary tracking bug
Browse files Browse the repository at this point in the history
When recording the result from the LLDB_RECORD_RESULT macro, we need to
update the boundary so we capture the copy constructor. However, when
called to record the this pointer of the (copy) constructor itself, the
boundary should not be toggled, because it is called from the
LLDB_RECORD_CONSTRUCTOR macro, which might be followed by other API
calls.

This manifested itself as an object encountered during replay that we
hadn't seen before. The index-to-object mapping would return a nullptr
and lldb would crash.
  • Loading branch information
JDevlieghere committed Jan 30, 2020
1 parent 8b73768 commit 05badc6
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions lldb/include/lldb/Utility/ReproducerInstrumentation.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ template <typename... Ts> inline std::string stringify_args(const Ts &... ts) {
sb_recorder.Record(data.GetSerializer(), data.GetRegistry(), \
&lldb_private::repro::construct<Class Signature>::doit, \
__VA_ARGS__); \
sb_recorder.RecordResult(this); \
sb_recorder.RecordResult(this, false); \
}

#define LLDB_RECORD_CONSTRUCTOR_NO_ARGS(Class) \
Expand All @@ -107,7 +107,7 @@ template <typename... Ts> inline std::string stringify_args(const Ts &... ts) {
LLDB_GET_INSTRUMENTATION_DATA()) { \
sb_recorder.Record(data.GetSerializer(), data.GetRegistry(), \
&lldb_private::repro::construct<Class()>::doit); \
sb_recorder.RecordResult(this); \
sb_recorder.RecordResult(this, false); \
}

#define LLDB_RECORD_METHOD(Result, Class, Method, Signature, ...) \
Expand Down Expand Up @@ -175,7 +175,7 @@ template <typename... Ts> inline std::string stringify_args(const Ts &... ts) {
static_cast<Result (*)()>(&Class::Method)); \
}

#define LLDB_RECORD_RESULT(Result) sb_recorder.RecordResult(Result);
#define LLDB_RECORD_RESULT(Result) sb_recorder.RecordResult(Result, true);

/// The LLDB_RECORD_DUMMY macro is special because it doesn't actually record
/// anything. It's used to track API boundaries when we cannot record for
Expand Down Expand Up @@ -716,8 +716,16 @@ class Recorder {
}

/// Record the result of a function call.
template <typename Result> Result RecordResult(Result &&r) {
UpdateBoundary();
template <typename Result>
Result RecordResult(Result &&r, bool update_boundary) {
// When recording the result from the LLDB_RECORD_RESULT macro, we need to
// update the boundary so we capture the copy constructor. However, when
// called to record the this pointer of the (copy) constructor, the
// boundary should not be toggled, because it is called from the
// LLDB_RECORD_CONSTRUCTOR macro, which might be followed by other API
// calls.
if (update_boundary)
UpdateBoundary();
if (m_serializer && ShouldCapture()) {
assert(!m_result_recorded);
m_serializer->SerializeAll(r);
Expand Down

0 comments on commit 05badc6

Please sign in to comment.