aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Florin Malita <fmalita@chromium.org>2018-06-14 18:41:39 +0000
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2018-06-14 18:41:50 +0000
commit0510edeebf76b8a9753acc06afde8b09a8f00a54 (patch)
treedbfb32906b2e6dc632860e574ccfd29682032222
parent28f5dd8a4c8aa053f417bcf7f1da94daa8915ca9 (diff)
Revert "[skjson] Size-constrained input API"
This reverts commit fc792b8718cc30e9da62c9559b23c1baac3166bb. Reason for revert: New ASAN, Android failures. Original change's description: > [skjson] Size-constrained input API > > Pass an explicit input size instead of requiring a C string. > > Thanks to mtklein's clever trick, this has no measurable perf impact. > > Change-Id: I64f210a9f653a78b05ab6b58fa34479504aa35ff > Reviewed-on: https://skia-review.googlesource.com/134940 > Reviewed-by: Mike Klein <mtklein@google.com> > Commit-Queue: Florin Malita <fmalita@chromium.org> TBR=mtklein@google.com,fmalita@chromium.org Change-Id: Ic0b52398b1ce6f64c781debb858829cb64bbae58 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/135022 Reviewed-by: Florin Malita <fmalita@chromium.org> Commit-Queue: Florin Malita <fmalita@chromium.org>
-rw-r--r--modules/skjson/include/SkJSON.h2
-rw-r--r--modules/skjson/src/FuzzSkJSON.cpp10
-rw-r--r--modules/skjson/src/SkJSON.cpp48
-rw-r--r--modules/skjson/src/SkJSONBench.cpp18
-rw-r--r--modules/skjson/src/SkJSONTest.cpp13
5 files changed, 41 insertions, 50 deletions
diff --git a/modules/skjson/include/SkJSON.h b/modules/skjson/include/SkJSON.h
index 1b46c4a100..99da29de72 100644
--- a/modules/skjson/include/SkJSON.h
+++ b/modules/skjson/include/SkJSON.h
@@ -326,7 +326,7 @@ private:
class DOM final : public SkNoncopyable {
public:
- DOM(const char*, size_t);
+ explicit DOM(const char*);
const Value& root() const { return fRoot; }
diff --git a/modules/skjson/src/FuzzSkJSON.cpp b/modules/skjson/src/FuzzSkJSON.cpp
index 2e971ce249..ce33cc1299 100644
--- a/modules/skjson/src/FuzzSkJSON.cpp
+++ b/modules/skjson/src/FuzzSkJSON.cpp
@@ -5,12 +5,20 @@
* found in the LICENSE file.
*/
+#include "SkAutoMalloc.h"
#include "SkData.h"
#include "SkJSON.h"
#include "SkStream.h"
void FuzzSkJSON(sk_sp<SkData> bytes) {
- skjson::DOM dom(static_cast<const char*>(bytes->data()), bytes->size());
+ // TODO: add a size + len skjson::DOM factory?
+ SkAutoMalloc data(bytes->size() + 1);
+ auto* c_str = static_cast<char*>(data.get());
+
+ memcpy(c_str, bytes->data(), bytes->size());
+ c_str[bytes->size()] = '\0';
+
+ skjson::DOM dom(c_str);
SkDynamicMemoryWStream wstream;
dom.write(&wstream);
}
diff --git a/modules/skjson/src/SkJSON.cpp b/modules/skjson/src/SkJSON.cpp
index 64f1302a7e..bf176c5c5c 100644
--- a/modules/skjson/src/SkJSON.cpp
+++ b/modules/skjson/src/SkJSON.cpp
@@ -11,12 +11,12 @@
#include "SkString.h"
#include <cmath>
-#include <tuple>
#include <vector>
namespace skjson {
-// #define SK_JSON_REPORT_ERRORS
+//#define SK_JSON_REPORT_ERRORS
+
static_assert( sizeof(Value) == 8, "");
static_assert(alignof(Value) == 8, "");
@@ -236,21 +236,7 @@ public:
fScopeStack.reserve(kScopeStackReserve);
}
- const Value parse(const char* p, size_t size) {
- if (!size) {
- this->error(NullValue(), p, "invalid empty input");
- }
-
- const char* p_stop = p + size - 1;
-
- // We're only checking for end-of-stream on object/array close('}',']'),
- // so we must trim any whitespace from the buffer tail.
- while (p_stop > p && is_ws(*p_stop)) --p_stop;
-
- if (*p_stop != '}' && *p_stop != ']') {
- return this->error(NullValue(), p_stop, "invalid top-level value");
- }
-
+ const Value parse(const char* p) {
p = skip_ws(p);
switch (*p) {
@@ -354,17 +340,17 @@ public:
pop_common:
SkASSERT(*p == '}' || *p == ']');
+ ++p;
+
if (fScopeStack.empty()) {
SkASSERT(fValueStack.size() == 1);
- // Success condition: parsed the top level element and reached the stop token.
- return p == p_stop
+ // Stop condition: parsed the top level element and there is no trailing garbage.
+ return *skip_ws(p) == '\0'
? fValueStack.front()
- : this->error(NullValue(), p + 1, "trailing root garbage");
+ : this->error(NullValue(), p, "trailing root garbage");
}
- ++p;
-
goto match_post_value;
match_array:
@@ -391,20 +377,19 @@ public:
return NullValue();
}
- std::tuple<const char*, const SkString> getError() const {
- return std::make_tuple(fErrorToken, fErrorMessage);
+ const SkString& getError() const {
+ return fError;
}
private:
- SkArenaAlloc& fAlloc;
+ SkArenaAlloc& fAlloc;
static constexpr size_t kValueStackReserve = 256;
static constexpr size_t kScopeStackReserve = 128;
std::vector<Value > fValueStack;
std::vector<intptr_t> fScopeStack;
- const char* fErrorToken = nullptr;
- SkString fErrorMessage;
+ SkString fError;
template <typename VectorT>
void popScopeAsVec(size_t scope_start) {
@@ -505,8 +490,9 @@ private:
template <typename T>
T error(T&& ret_val, const char* p, const char* msg) {
#if defined(SK_JSON_REPORT_ERRORS)
- fErrorToken = p;
- fErrorMessage.set(msg);
+ static constexpr size_t kMaxContext = 128;
+ fError = SkStringPrintf("%s: >", msg);
+ fError.append(p, std::min(strlen(p), kMaxContext));
#endif
return ret_val;
}
@@ -732,11 +718,11 @@ SkString Value::toString() const {
static constexpr size_t kMinChunkSize = 4096;
-DOM::DOM(const char* data, size_t size)
+DOM::DOM(const char* c_str)
: fAlloc(kMinChunkSize) {
DOMParser parser(fAlloc);
- fRoot = parser.parse(data, size);
+ fRoot = parser.parse(c_str);
}
void DOM::write(SkWStream* stream) const {
diff --git a/modules/skjson/src/SkJSONBench.cpp b/modules/skjson/src/SkJSONBench.cpp
index b9c3664306..4be3b972ab 100644
--- a/modules/skjson/src/SkJSONBench.cpp
+++ b/modules/skjson/src/SkJSONBench.cpp
@@ -25,21 +25,27 @@ protected:
bool isSuitableFor(Backend backend) override { return backend == kNonRendering_Backend; }
void onPerCanvasPreDraw(SkCanvas*) override {
- fData = SkData::MakeFromFileName(kBenchFile);
- if (!fData) {
+ if (auto stream = SkStream::MakeFromFile(kBenchFile)) {
+ SkASSERT(stream->hasLength());
+ fCStringData = SkData::MakeUninitialized(stream->getLength() + 1);
+ auto* data8 = reinterpret_cast<uint8_t*>(fCStringData->writable_data());
+ SkAssertResult(stream->read(data8, stream->getLength()) == stream->getLength());
+ data8[stream->getLength()] = '\0';
+
+ } else {
SkDebugf("!! Could not open bench file: %s\n", kBenchFile);
}
}
void onPerCanvasPostDraw(SkCanvas*) override {
- fData = nullptr;
+ fCStringData = nullptr;
}
void onDraw(int loops, SkCanvas*) override {
- if (!fData) return;
+ if (!fCStringData) return;
for (int i = 0; i < loops; i++) {
- skjson::DOM dom(static_cast<const char*>(fData->data()), fData->size());
+ skjson::DOM dom(static_cast<const char*>(fCStringData->data()));
if (dom.root().is<skjson::NullValue>()) {
SkDebugf("!! Parsing failed.\n");
return;
@@ -48,7 +54,7 @@ protected:
}
private:
- sk_sp<SkData> fData;
+ sk_sp<SkData> fCStringData;
using INHERITED = Benchmark;
};
diff --git a/modules/skjson/src/SkJSONTest.cpp b/modules/skjson/src/SkJSONTest.cpp
index 91ae497cf0..6d4338cbbb 100644
--- a/modules/skjson/src/SkJSONTest.cpp
+++ b/modules/skjson/src/SkJSONTest.cpp
@@ -22,14 +22,8 @@ DEF_TEST(SkJSON_Parse, reporter) {
{ "" , nullptr },
{ "[" , nullptr },
{ "]" , nullptr },
- { "[[]" , nullptr },
- { "[]]" , nullptr },
- { "[]f" , nullptr },
{ "{" , nullptr },
{ "}" , nullptr },
- { "{{}" , nullptr },
- { "{}}" , nullptr },
- { "{}f" , nullptr },
{ "{]" , nullptr },
{ "[}" , nullptr },
{ "1" , nullptr },
@@ -57,9 +51,7 @@ DEF_TEST(SkJSON_Parse, reporter) {
{ "{ \"k\" : null \"k\" : 1 }", nullptr },
- { "[]" , "[]" },
{ " \n\r\t [ \n\r\t ] \n\r\t " , "[]" },
- { "[[]]" , "[[]]" },
{ "[ null ]" , "[null]" },
{ "[ true ]" , "[true]" },
{ "[ false ]" , "[false]" },
@@ -74,7 +66,6 @@ DEF_TEST(SkJSON_Parse, reporter) {
{ "[ \"123456789\" ]" , "[\"123456789\"]" },
{ "[ null , true, false,0,12.8 ]", "[null,true,false,0,12.8]" },
- { "{}" , "{}" },
{ " \n\r\t { \n\r\t } \n\r\t " , "{}" },
{ "{ \"k\" : null }" , "{\"k\":null}" },
{ "{ \"k1\" : null, \"k2 \":0 }", "{\"k1\":null,\"k2 \":0}" },
@@ -98,7 +89,7 @@ DEF_TEST(SkJSON_Parse, reporter) {
};
for (const auto& tst : g_tests) {
- DOM dom(tst.in, strlen(tst.in));
+ DOM dom(tst.in);
const auto success = !dom.root().is<NullValue>();
REPORTER_ASSERT(reporter, success == (tst.out != nullptr));
if (!success) continue;
@@ -162,7 +153,7 @@ DEF_TEST(SkJSON_DOM_visit, reporter) {
\"k8\": { \"kk1\": 2, \"kk2\": false, \"kk1\": \"baz\" } \n\
}";
- DOM dom(json, strlen(json));
+ DOM dom(json);
const auto& jroot = dom.root().as<ObjectValue>();
REPORTER_ASSERT(reporter, jroot.is<ObjectValue>());