AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Tracing cleanup and bugfixes

Browse Source 
TraceID was unused, remove it.

Simplify casting logic by using the same union helper as the macros.
This fixes a bug that was present in the bool handling - we were
treating the union value as a pointer, so we were dereferencing
random stack memory. Luckily it never crashes, we did get the wrong
values for bools.

Bug: skia:
Change-Id: I15d44756214f34c1f6479980d9a487ac7f3d8f6c
Reviewed-on: https://skia-review.googlesource.com/25801
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Brian Osman 2017-07-21 15:30:14 -04:00 committed by Skia Commit-Bot
parent aaa3056e46
commit de6e5bf33f
3 changed files with 21 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/core/SkTraceEvent.h
View File

@ -143,41 +143,6 @@ namespace tracing_internals {
const int kZeroNumArgs = 0;
const uint64_t kNoEventId = 0;
// TraceID encapsulates an ID that can either be an integer or pointer. Pointers
// are by default mangled with the Process ID so that they are unlikely to
// collide when the same pointer is used on different processes.
class TraceID {
public:
TraceID(const void* id, unsigned char* flags)
: data_(static_cast<uint64_t>(
reinterpret_cast<uintptr_t>(id))) {
*flags |= TRACE_EVENT_FLAG_MANGLE_ID;
}
TraceID(uint64_t id, unsigned char* flags)
: data_(id) { (void)flags; }
TraceID(unsigned int id, unsigned char* flags)
: data_(id) { (void)flags; }
TraceID(unsigned short id, unsigned char* flags)
: data_(id) { (void)flags; }
TraceID(unsigned char id, unsigned char* flags)
: data_(id) { (void)flags; }
TraceID(long long id, unsigned char* flags)
: data_(static_cast<uint64_t>(id)) { (void)flags; }
TraceID(long id, unsigned char* flags)
: data_(static_cast<uint64_t>(id)) { (void)flags; }
TraceID(int id, unsigned char* flags)
: data_(static_cast<uint64_t>(id)) { (void)flags; }
TraceID(short id, unsigned char* flags)
: data_(static_cast<uint64_t>(id)) { (void)flags; }
TraceID(signed char id, unsigned char* flags)
: data_(static_cast<uint64_t>(id)) { (void)flags; }
uint64_t data() const { return data_; }
private:
uint64_t data_;
};
// Simple union to store various types as uint64_t.
union TraceValueUnion {
bool as_bool;

22
tools/trace/SkChromeTracingTracer.cpp
View File

@ -23,6 +23,9 @@ SkEventTracer::Handle SkChromeTracingTracer::addTraceEvent(char phase,
const uint8_t* argTypes,
const uint64_t* argValues,
uint8_t flags) {
// TODO: Respect flags (or assert). INSTANT events encode scope in flags, should be stored
// using "s" key in JSON. COPY flag should be supported or rejected.
Json::Value traceEvent;
char phaseString[2] = { phase, 0 };
traceEvent["ph"] = phaseString;
@ -32,30 +35,35 @@ SkEventTracer::Handle SkChromeTracingTracer::addTraceEvent(char phase,
std::chrono::duration<double, std::nano> ns = now.time_since_epoch();
traceEvent["ts"] = ns.count() * 1E-3;
traceEvent["tid"] = static_cast<Json::Int64>(SkGetThreadID());
traceEvent["pid"] = 1; // TODO
// Trace events *must* include a process ID, but for internal tools this isn't particularly
// important (and certainly not worth adding a cross-platform API to get it).
traceEvent["pid"] = 0;
if (numArgs) {
Json::Value args;
skia::tracing_internals::TraceValueUnion value;
for (int i = 0; i < numArgs; ++i) {
value.as_uint = argValues[i];
switch (argTypes[i]) {
case TRACE_VALUE_TYPE_BOOL:
args[argNames[i]] = (*reinterpret_cast<bool*>(argValues[i]) ? true : false);
args[argNames[i]] = value.as_bool;
break;
case TRACE_VALUE_TYPE_UINT:
args[argNames[i]] = static_cast<uint32_t>(argValues[i]);
args[argNames[i]] = static_cast<Json::UInt64>(argValues[i]);
break;
case TRACE_VALUE_TYPE_INT:
args[argNames[i]] = static_cast<int32_t>(argValues[i]);
args[argNames[i]] = static_cast<Json::Int64>(argValues[i]);
break;
case TRACE_VALUE_TYPE_DOUBLE:
args[argNames[i]] = *SkTCast<const double*>(&argValues[i]);
args[argNames[i]] = value.as_double;
break;
case TRACE_VALUE_TYPE_POINTER:
args[argNames[i]] = reinterpret_cast<void*>(argValues[i]);
args[argNames[i]] = value.as_pointer;
break;
case TRACE_VALUE_TYPE_STRING:
case TRACE_VALUE_TYPE_COPY_STRING:
args[argNames[i]] = reinterpret_cast<const char*>(argValues[i]);
args[argNames[i]] = value.as_string;
break;
default:
args[argNames[i]] = "<unknown type>";

13
tools/trace/SkDebugfTracer.cpp
View File

@ -24,11 +24,11 @@ SkEventTracer::Handle SkDebugfTracer::addTraceEvent(char phase,
} else {
args.append(" ");
}
skia::tracing_internals::TraceValueUnion value;
value.as_uint = argValues[i];
switch (argTypes[i]) {
case TRACE_VALUE_TYPE_BOOL:
args.appendf("%s=%s",
argNames[i],
(*reinterpret_cast<bool*>(argValues[i]) ? "true" : "false"));
args.appendf("%s=%s", argNames[i], value.as_bool ? "true" : "false");
break;
case TRACE_VALUE_TYPE_UINT:
args.appendf("%s=%u", argNames[i], static_cast<uint32_t>(argValues[i]));
@ -37,16 +37,15 @@ SkEventTracer::Handle SkDebugfTracer::addTraceEvent(char phase,
args.appendf("%s=%d", argNames[i], static_cast<int32_t>(argValues[i]));
break;
case TRACE_VALUE_TYPE_DOUBLE:
args.appendf("%s=%g", argNames[i], *SkTCast<const double*>(&argValues[i]));
args.appendf("%s=%g", argNames[i], value.as_double);
break;
case TRACE_VALUE_TYPE_POINTER:
args.appendf("%s=0x%p", argNames[i], reinterpret_cast<void*>(argValues[i]));
args.appendf("%s=0x%p", argNames[i], value.as_pointer);
break;
case TRACE_VALUE_TYPE_STRING:
case TRACE_VALUE_TYPE_COPY_STRING: {
static constexpr size_t kMaxLen = 20;
const char* str = reinterpret_cast<const char*>(argValues[i]);
SkString string(str);
SkString string(value.as_string);
size_t truncAt = string.size();
size_t newLineAt = SkStrFind(string.c_str(), "\n");
if (newLineAt > 0) {