[Strings] Fuzz and interpret all relevant StringNew methods #6526
Conversation
| [[fallthrough]]; | ||
| } | ||
| case 2: { | ||
| // Construct an interesting WTF-8 string from parts and use string.const. |
src/wasm-interpreter.h
Outdated
| std::string str; | ||
| str += char(codePoint & 0xff); | ||
| str += char((codePoint >> 8) & 0xff); | ||
| if (codePoint > 0xffff) { | ||
| str += char((codePoint >> 16) & 0xff); | ||
| str += char((codePoint >> 24) & 0xff); | ||
| } | ||
| std::stringstream null; | ||
| if (!String::convertUTF16ToUTF8(null, str)) { | ||
| trap("invalid code point"); | ||
| } |
There was a problem hiding this comment.
This check for invalid code points is probably not behaving correctly because this is not the correct way to encode a code point as UTF-16.
To check for validity, all you have to do is check codePoint <= 0x10FFFF. After that, you can use String::writeWTF16CodePoint to get the proper encoding, then you can convert that from a string to a string literal.
edit: Note that the string literal may have two code units because the encoding of the code point may require a surrogate pair.
There was a problem hiding this comment.
Thanks! I am still very fuzzy on the UTF stuff 😄 I thought I just need to emit two 16-bit chunks here...
src/wasm-interpreter.h
Outdated
| // empty. Write it all out but the null terminator. | ||
| assert(!str.empty()); | ||
| for (size_t i = 0; i < str.size() - 1; i++) { | ||
| contents.push_back(Literal(int32_t(str[i]))); |
There was a problem hiding this comment.
For this part you need to consume two bytes for each entry in the array. The Literal(std::string_view) constructor can do this for you, though.
| (string.from_code_point | ||
| (i32.const -83) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Other test cases to add:
An isolated high surrogate: i32.const 0xD800
An isolated low surrogate: i32.const 0xDC00
A codepoint requiring a surrogate pair: 10348 (𐍈)
There was a problem hiding this comment.
Thanks, added. I also noticed a fuzz error on an unsigned case (that now works) that I also added.
|
Fuzzed for 5,000 iterations with an extra patch that emits even more strings and no issues, landing. |
2 participants
This adds fuzzing for
string.new_wtf16_arrayandstring.from_code_point. Thelatter was also missing interpreter support, which this adds.
@tlively I seem to get the wrong results for unicode here - am I using the wrong
API or using it incorrectly? The last testcase here traps in V8 on being an invalid
codePoint but
String::convertUTF16ToUTF8returns that it is valid.For comparison, related V8 code: https://chromium.googlesource.com/v8/v8/+/a19a2ff2be5b60f6df6c7c7160860ede2cb85e27%5E%21/#F3