dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm] Fix crash in Script::GetLine().

Browse Source 
There's a case where String::SubString() can be called with a negative
length in the original code: if the desired line is not the first line
and is empty.

Since last_char_idx doesn't get updated by either a '\r' or \n',
last_char_idx will be the position before the last line terminator,
line_start_idx will be the start of the line, and thus the calculated
substring length will be either -1 (for '\n' or '\r' line terminators)
or -2 (for '\r\n' line terminators).

Refactor the code into two loops: the first to look for the line,
looping on a possible start index, and then if found, the second to find
the end index starting from the start index. This ensures that, when the
line is found, the the end index is >= the start index and thus the
substring call will succeed.

Fixes https://github.com/dart-lang/sdk/issues/44263

TEST=Edited vm/cc/Script to also verify Script::GetLine() no longer
crashes on such input.

Fixed: 44263
Change-Id: Ifca9d5d4984c24a6c40bd64f3ceaca06c7197444
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/173965
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
This commit is contained in:
Tess Strickland
2020-11-26 09:42:35 +00:00
committed by commit-bot@chromium.org
parent dcd5a8f005
commit f085fd0f00
2 changed files with 120 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/object.cc
+24 -21
View File
@@ -11331,33 +11331,36 @@ StringPtr Script::GetLine(intptr_t line_number, Heap::Space space) const {
if (src.IsNull()) {
return Symbols::OptimizedOut().raw();
}
intptr_t relative_line_number = line_number - line_offset();
intptr_t target_line = line_number - line_offset();
intptr_t current_line = 1;
intptr_t line_start_idx = -1;
intptr_t last_char_idx = -1;
for (intptr_t ix = 0;
(ix < src.Length()) && (current_line <= relative_line_number); ix++) {
if ((current_line == relative_line_number) && (line_start_idx < 0)) {
line_start_idx = ix;
intptr_t start = 0;
// First find the right line, if present...
for (; start < src.Length(); start++) {
if (current_line == target_line) {
break;
}
if (src.CharAt(ix) == '\n') {
const uint16_t c = src.CharAt(start);
// Only count '\r' as a line terminator if not followed by a '\n'.
if (c == '\n' || (c == '\r' && (start + 1 >= src.Length() ||
src.CharAt(start + 1) != '\n'))) {
current_line++;
} else if (src.CharAt(ix) == '\r') {
if ((ix + 1 != src.Length()) && (src.CharAt(ix + 1) != '\n')) {
current_line++;
}
} else {
last_char_idx = ix;
}
}
// Guarantee that returned string is never NULL.
if (line_start_idx >= 0) {
return String::SubString(src, line_start_idx,
last_char_idx - line_start_idx + 1, space);
} else {
return Symbols::Empty().raw();
if (current_line == target_line) {
// ... and then find its end, excluding any line terminator.
intptr_t end = start;
for (; end < src.Length(); end++) {
const uint16_t c = src.CharAt(end);
if (c == '\n' || c == '\r') {
break;
}
}
// Return the contents of the line.
return String::SubString(src, start, end - start, space);
}
// Not found, so return the empty string.
return Symbols::Empty().raw();
}
StringPtr Script::GetSnippet(TokenPosition from, TokenPosition to) const {
runtime/vm/object_test.cc
+96 -16
View File
@@ -2354,24 +2354,104 @@ ISOLATE_UNIT_TEST_CASE(ExternalTypedData) {
}
}
ISOLATE_UNIT_TEST_CASE(Script) {
const char* url_chars = "builtin:test-case";
const char* source_chars = "This will not compile.";
const String& url = String::Handle(String::New(url_chars));
const String& source = String::Handle(String::New(source_chars));
const Script& script = Script::Handle(Script::New(url, source));
static void CheckLinesWithOffset(Zone* zone, const intptr_t offset) {
const char* url_chars = "";
// Eight lines, mix of \n, \r, \r\n line terminators, lines 3, 4, 7, and 8
// are non-empty. Ends with a \r as a double-check that the \r followed by
// \n check doesn't go out of bounds.
const char* source_chars = "\n\nxyz\nabc\r\n\n\r\ndef\rghi\r";
const String& url = String::Handle(zone, String::New(url_chars));
const String& source = String::Handle(zone, String::New(source_chars));
const Script& script = Script::Handle(zone, Script::New(url, source));
EXPECT(!script.IsNull());
EXPECT(script.IsScript());
String& str = String::Handle(script.url());
EXPECT_EQ(17, str.Length());
EXPECT_EQ('b', str.CharAt(0));
EXPECT_EQ(':', str.CharAt(7));
EXPECT_EQ('e', str.CharAt(16));
str = script.Source();
EXPECT_EQ(22, str.Length());
EXPECT_EQ('T', str.CharAt(0));
EXPECT_EQ('n', str.CharAt(10));
EXPECT_EQ('.', str.CharAt(21));
script.SetLocationOffset(offset, 10);
auto& str = String::Handle(zone);
str = script.GetLine(offset + 1);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(offset + 2);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(offset + 3);
EXPECT_STREQ("xyz", str.ToCString());
str = script.GetLine(offset + 4);
EXPECT_STREQ("abc", str.ToCString());
str = script.GetLine(offset + 5);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(offset + 6);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(offset + 7);
EXPECT_STREQ("def", str.ToCString());
str = script.GetLine(offset + 8);
EXPECT_STREQ("ghi", str.ToCString());
// Lines not in the range of (1-based) line indices in the source should
// return the empty string.
str = script.GetLine(-500);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(0);
EXPECT_STREQ("", str.ToCString());
if (offset > 0) {
str = script.GetLine(1); // Absolute, not relative to offset.
EXPECT_STREQ("", str.ToCString());
}
if (offset > 2) {
str = script.GetLine(3); // Absolute, not relative to offset.
EXPECT_STREQ("", str.ToCString());
}
str = script.GetLine(offset);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(offset + 9);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(offset + 10000);
EXPECT_STREQ("", str.ToCString());
}
ISOLATE_UNIT_TEST_CASE(Script) {
{
const char* url_chars = "builtin:test-case";
const char* source_chars = "This will not compile.";
const String& url = String::Handle(String::New(url_chars));
const String& source = String::Handle(String::New(source_chars));
const Script& script = Script::Handle(Script::New(url, source));
EXPECT(!script.IsNull());
EXPECT(script.IsScript());
String& str = String::Handle(script.url());
EXPECT_EQ(17, str.Length());
EXPECT_EQ('b', str.CharAt(0));
EXPECT_EQ(':', str.CharAt(7));
EXPECT_EQ('e', str.CharAt(16));
str = script.Source();
EXPECT_EQ(22, str.Length());
EXPECT_EQ('T', str.CharAt(0));
EXPECT_EQ('n', str.CharAt(10));
EXPECT_EQ('.', str.CharAt(21));
}
CheckLinesWithOffset(Z, 0);
CheckLinesWithOffset(Z, 500);
CheckLinesWithOffset(Z, 10000);
{
const char* url_chars = "";
// Single line, no terminators.
const char* source_chars = "abc";
const String& url = String::Handle(String::New(url_chars));
const String& source = String::Handle(String::New(source_chars));
const Script& script = Script::Handle(Script::New(url, source));
EXPECT(!script.IsNull());
EXPECT(script.IsScript());
auto& str = String::Handle(Z);
str = script.GetLine(1);
EXPECT_STREQ("abc", str.ToCString());
// Lines not in the source should return the empty string.
str = script.GetLine(-500);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(0);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(2);
EXPECT_STREQ("", str.ToCString());
str = script.GetLine(10000);
EXPECT_STREQ("", str.ToCString());
}
TransitionVMToNative transition(thread);
const char* kScript = "main() {}";