[Strings] Avoid mishandling unicode in interpreter #6405
Conversation
Our interpreter implementations of `stringview_wtf16.length` and `stringview_wtf16.get_codeunit` are not unicode-aware, so they were previously incorrect in the face of multi-byte code units. As a fix, bail out of the interpretation if there is a non-ascii code point that would make our naive implementation incorrect.
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
cc @rluble |
| ;; CHECK: (func $encode (type $0) (result i32) | ||
| ;; CHECK-NEXT: (i32.const 2) | ||
| ;; CHECK-NEXT: ) |
There was a problem hiding this comment.
@kripken, is this optimization safe? Does Precompute do this even if the modified array would escape?
There was a problem hiding this comment.
Yes, this is safe. If there were effects (like a local.tee that allows the value to escape) then it would not happen.
| if (uint32_t(data->values[i].geti32()) > 127) { | ||
| return Flow(NONCONSTANT_FLOW); | ||
| } | ||
| } |
There was a problem hiding this comment.
Perhaps it makes sense to add a helper for this? Could be a templated function in src/support/string.h, or maybe a helper in this file as part of the interpreter would be better, I'm not sure.
There was a problem hiding this comment.
Since we're operating on lists of literals, I've added a helper to the interpreter.
| ;; CHECK: (func $encode (type $0) (result i32) | ||
| ;; CHECK-NEXT: (i32.const 2) | ||
| ;; CHECK-NEXT: ) |
There was a problem hiding this comment.
Yes, this is safe. If there were effects (like a local.tee that allows the value to escape) then it would not happen.
Our interpreter implementations of
stringview_wtf16.length,stringview_wtf16.get_codeunit, andstring.encode_wtf16_arrayare notunicode-aware, so they were previously incorrect in the face of multi-byte code
units. As a fix, bail out of the interpretation if there is a non-ascii code
point that would make our naive implementation incorrect.