GitLab

This patch fixed the base::trace_event::TraceEvent::Reset()
method to ensure that it never creates dangling pointers.

This can happen in the following case:

- TraceEvent::Initialize() is called on an instance, with
  TRACE_EVENT_FLAG_COPY set in the |flags| argument.

  This will copy argument names, copyable string values,
  as well as the name and scope into a single heap
  allocated buffer backed by |parameter_copy_storage_|,
  and will also adjust all internal pointer fields to
  point to it.

- TraceEvent::Reset() is called on the same instance,
  this frees the storage area, but before this CL did not
  update the internal pointers, who were now dangling
  into heap-free memory!

- Later, some code will iterate over the arguments with
  a loop like:

    for (int i = 0;
         i < kTraceMaxNumArgs && arg_names_[i] != nullptr;
         ++i) {
      ...
    }

  The assumption being that an arg_names_[i] value of
  nullptr indicates the end of list. Unfortunately, in
  the case above, this will read completely invalid
  values from memory.

+ Fix TraceEvent::MoveFrom() to call other->Reset() to ensure
  that the source instance is left in consistent state.

I believe this is the source of flakiness on many tests
related to TraceEvent, and hope this fixes it.

BUG=905624,899813
R=oystene@chromium.org,primiano@chromium.org,alexilin@chromium.org,pkl@chromium.org

Change-Id: I63cbadc728130cddc68b8c92b28e1e3f584793f4
Reviewed-on: https://chromium-review.googlesource.com/c/1340308
Commit-Queue: David Turner <digit@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609207}