Commit 375bf0eb authored by lpy's avatar lpy Committed by Commit bot

[Tracing] Minor bugs fix.

1. The third parameter of strncpy should be the length of source string.
2. Value contains " should be valid.

BUG=v8:4561

Review-Url: https://codereview.chromium.org/2232683002
Cr-Commit-Position: refs/heads/master@{#38563}
parent 1cefcd4d
...@@ -21,12 +21,12 @@ V8_INLINE static size_t GetAllocLength(const char* str) { ...@@ -21,12 +21,12 @@ V8_INLINE static size_t GetAllocLength(const char* str) {
// Copies |*member| into |*buffer|, sets |*member| to point to this new // Copies |*member| into |*buffer|, sets |*member| to point to this new
// location, and then advances |*buffer| by the amount written. // location, and then advances |*buffer| by the amount written.
V8_INLINE static void CopyTraceObjectParameter(char** buffer, V8_INLINE static void CopyTraceObjectParameter(char** buffer,
const char** member, const char** member) {
const char* end) {
if (*member) { if (*member) {
strncpy(*buffer, *member, end - *buffer); size_t length = strlen(*member) + 1;
strncpy(*buffer, *member, length);
*member = *buffer; *member = *buffer;
*buffer += strlen(*member) + 1; *buffer += length;
} }
} }
...@@ -81,17 +81,16 @@ void TraceObject::Initialize(char phase, const uint8_t* category_enabled_flag, ...@@ -81,17 +81,16 @@ void TraceObject::Initialize(char phase, const uint8_t* category_enabled_flag,
// to free old memory. // to free old memory.
delete[] parameter_copy_storage_; delete[] parameter_copy_storage_;
char* ptr = parameter_copy_storage_ = new char[alloc_size]; char* ptr = parameter_copy_storage_ = new char[alloc_size];
const char* end = ptr + alloc_size;
if (copy) { if (copy) {
CopyTraceObjectParameter(&ptr, &name_, end); CopyTraceObjectParameter(&ptr, &name_);
CopyTraceObjectParameter(&ptr, &scope_, end); CopyTraceObjectParameter(&ptr, &scope_);
for (int i = 0; i < num_args_; ++i) { for (int i = 0; i < num_args_; ++i) {
CopyTraceObjectParameter(&ptr, &arg_names_[i], end); CopyTraceObjectParameter(&ptr, &arg_names_[i]);
} }
} }
for (int i = 0; i < num_args_; ++i) { for (int i = 0; i < num_args_; ++i) {
if (arg_is_copy[i]) { if (arg_is_copy[i]) {
CopyTraceObjectParameter(&ptr, &arg_values_[i].as_string, end); CopyTraceObjectParameter(&ptr, &arg_values_[i].as_string);
} }
} }
} }
......
...@@ -15,11 +15,11 @@ namespace tracing { ...@@ -15,11 +15,11 @@ namespace tracing {
// Currently we do not support JSON-escaping strings in trace arguments. // Currently we do not support JSON-escaping strings in trace arguments.
// Thus we perform an IsJSONString() check before writing any string argument. // Thus we perform an IsJSONString() check before writing any string argument.
// In particular, this means strings cannot have control characters or " or \. // In particular, this means strings cannot have control characters or \.
V8_INLINE static bool IsJSONString(const char* str) { V8_INLINE static bool IsJSONString(const char* str) {
size_t len = strlen(str); size_t len = strlen(str);
for (size_t i = 0; i < len; ++i) { for (size_t i = 0; i < len; ++i) {
if (iscntrl(str[i]) || str[i] == '\"' || str[i] == '\\') { if (iscntrl(str[i]) || str[i] == '\\') {
return false; return false;
} }
} }
...@@ -74,8 +74,8 @@ void JSONTraceWriter::AppendArgValue(uint8_t type, ...@@ -74,8 +74,8 @@ void JSONTraceWriter::AppendArgValue(uint8_t type,
case TRACE_VALUE_TYPE_COPY_STRING: case TRACE_VALUE_TYPE_COPY_STRING:
// Strings are currently not JSON-escaped, so we need to perform a check // Strings are currently not JSON-escaped, so we need to perform a check
// to see if they are valid JSON strings. // to see if they are valid JSON strings.
CHECK(value.as_string == nullptr || IsJSONString(value.as_string)); CHECK(value.as_string != nullptr && IsJSONString(value.as_string));
stream_ << "\"" << (value.as_string ? value.as_string : "NULL") << "\""; stream_ << (value.as_string ? value.as_string : "NULL");
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
......
...@@ -217,8 +217,9 @@ TEST(TestTracingControllerMultipleArgsAndCopy) { ...@@ -217,8 +217,9 @@ TEST(TestTracingControllerMultipleArgsAndCopy) {
double jj4 = std::numeric_limits<double>::infinity(); double jj4 = std::numeric_limits<double>::infinity();
double jj5 = -std::numeric_limits<double>::infinity(); double jj5 = -std::numeric_limits<double>::infinity();
void* kk = &aa; void* kk = &aa;
const char* ll = "100"; const char* ll = "\"100\"";
std::string mm = "INIT"; std::string mm = "INIT";
std::string mmm = "\"INIT\"";
// Create a scope for the tracing controller to terminate the trace writer. // Create a scope for the tracing controller to terminate the trace writer.
{ {
...@@ -250,19 +251,20 @@ TEST(TestTracingControllerMultipleArgsAndCopy) { ...@@ -250,19 +251,20 @@ TEST(TestTracingControllerMultipleArgsAndCopy) {
TRACE_EVENT1("v8", "v8.Test.jj5", "jj5", jj5); TRACE_EVENT1("v8", "v8.Test.jj5", "jj5", jj5);
TRACE_EVENT1("v8", "v8.Test.kk", "kk", kk); TRACE_EVENT1("v8", "v8.Test.kk", "kk", kk);
TRACE_EVENT1("v8", "v8.Test.ll", "ll", ll); TRACE_EVENT1("v8", "v8.Test.ll", "ll", ll);
TRACE_EVENT1("v8", "v8.Test.mm", "mm", TRACE_STR_COPY(mm.c_str())); TRACE_EVENT1("v8", "v8.Test.mm", "mm", TRACE_STR_COPY(mmm.c_str()));
TRACE_EVENT2("v8", "v8.Test2.1", "aa", aa, "ll", ll); TRACE_EVENT2("v8", "v8.Test2.1", "aa", aa, "ll", ll);
TRACE_EVENT2("v8", "v8.Test2.2", "mm1", TRACE_STR_COPY(mm.c_str()), "mm2", TRACE_EVENT2("v8", "v8.Test2.2", "mm1", TRACE_STR_COPY(mmm.c_str()), "mm2",
TRACE_STR_COPY(mm.c_str())); TRACE_STR_COPY(mmm.c_str()));
// Check copies are correct. // Check copies are correct.
TRACE_EVENT_COPY_INSTANT0("v8", mm.c_str(), TRACE_EVENT_SCOPE_THREAD); TRACE_EVENT_COPY_INSTANT0("v8", mm.c_str(), TRACE_EVENT_SCOPE_THREAD);
TRACE_EVENT_COPY_INSTANT1("v8", mm.c_str(), TRACE_EVENT_SCOPE_THREAD, TRACE_EVENT_COPY_INSTANT1("v8", mm.c_str(), TRACE_EVENT_SCOPE_THREAD,
mm.c_str(), mm.c_str()); mm.c_str(), mmm.c_str());
TRACE_EVENT_COPY_INSTANT2("v8", mm.c_str(), TRACE_EVENT_SCOPE_THREAD, TRACE_EVENT_COPY_INSTANT2("v8", mm.c_str(), TRACE_EVENT_SCOPE_THREAD,
mm.c_str(), mm.c_str(), mm.c_str(), mm.c_str()); mm.c_str(), mmm.c_str(), mm.c_str(), mmm.c_str());
mm = "CHANGED"; mm = "CHANGED";
mmm = "CHANGED";
tracing_controller.StopTracing(); tracing_controller.StopTracing();
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment