Skip to content

Commit

Permalink
Fix crash in SQLite sqlite3Fts3Incrmerge
Browse files Browse the repository at this point in the history
Backports https://www.sqlite.org/src/info/361eb2f682a303bb
and https://sqlite.org/src/info/67da31e24ebb49c4

Bug:  998494
Change-Id: Icd11546d8f504a1c6df7bc5692b917ded35ba486
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835704
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704392}
  • Loading branch information
ConradIrwin authored and Commit Bot committed Oct 9, 2019
1 parent 9710388 commit f9f8801
Show file tree
Hide file tree
Showing 13 changed files with 232 additions and 20 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -185,6 +185,7 @@ Clemens Fruhwirth <clemens@endorphin.org>
Clement Scheelfeldt Skau <clementskau@gmail.com>
Clinton Staley <clintstaley@gmail.com>
Connor Pearson <cjp822@gmail.com>
Conrad Irwin <conrad.irwin@gmail.com>
Craig Schlenter <craig.schlenter@gmail.com>
Csaba Osztrogonác <ossy.szeged@gmail.com>
Daegyu Lee <na7jun8gi@gmail.com>
Expand Down
8 changes: 4 additions & 4 deletions third_party/sqlite/amalgamation/sqlite3.c
Expand Up @@ -174314,14 +174314,14 @@ static int nodeReaderInit(NodeReader *p, const char *aNode, int nNode){
p->nNode = nNode;

/* Figure out if this is a leaf or an internal node. */
if( p->aNode[0] ){
if( aNode && aNode[0] ){
/* An internal node. */
p->iOff = 1 + sqlite3Fts3GetVarint(&p->aNode[1], &p->iChild);
}else{
p->iOff = 1;
}

return nodeReaderNext(p);
return aNode ? nodeReaderNext(p) : SQLITE_OK;
}

/*
Expand Down Expand Up @@ -174812,8 +174812,8 @@ static int fts3IncrmergeLoad(
NodeReader reader;
pNode = &pWriter->aNodeWriter[i];

rc = nodeReaderInit(&reader, pNode->block.a, pNode->block.n);
if( reader.aNode ){
if ( pNode->block.a){
rc = nodeReaderInit(&reader, pNode->block.a, pNode->block.n);
while( reader.aNode && rc==SQLITE_OK ) rc = nodeReaderNext(&reader);
blobGrowBuffer(&pNode->key, reader.term.n, &rc);
if( rc==SQLITE_OK ){
Expand Down
8 changes: 4 additions & 4 deletions third_party/sqlite/patched/ext/fts3/fts3_write.c
Expand Up @@ -3797,14 +3797,14 @@ static int nodeReaderInit(NodeReader *p, const char *aNode, int nNode){
p->nNode = nNode;

/* Figure out if this is a leaf or an internal node. */
if( p->aNode[0] ){
if( aNode && aNode[0] ){
/* An internal node. */
p->iOff = 1 + sqlite3Fts3GetVarint(&p->aNode[1], &p->iChild);
}else{
p->iOff = 1;
}

return nodeReaderNext(p);
return aNode ? nodeReaderNext(p) : SQLITE_OK;
}

/*
Expand Down Expand Up @@ -4295,8 +4295,8 @@ static int fts3IncrmergeLoad(
NodeReader reader;
pNode = &pWriter->aNodeWriter[i];

rc = nodeReaderInit(&reader, pNode->block.a, pNode->block.n);
if( reader.aNode ){
if ( pNode->block.a){
rc = nodeReaderInit(&reader, pNode->block.a, pNode->block.n);
while( reader.aNode && rc==SQLITE_OK ) rc = nodeReaderNext(&reader);
blobGrowBuffer(&pNode->key, reader.term.n, &rc);
if( rc==SQLITE_OK ){
Expand Down
58 changes: 58 additions & 0 deletions third_party/sqlite/patched/test/fts4merge5.test
@@ -0,0 +1,58 @@
# 2019 October 02
#
# The author disclaims copyright to this source code. In place of
# a legal notice, here is a blessing:
#
# May you do good and not evil.
# May you find forgiveness for yourself and forgive others.
# May you share freely, never taking more than you give.
#
#*************************************************************************
# This file implements regression tests for SQLite library. The
# focus of this script is testing the FTS4 module.
#

set testdir [file dirname $argv0]
source $testdir/tester.tcl
set testprefix fts4merge5

# If SQLITE_ENABLE_FTS3 is defined, omit this file.
ifcapable !fts3 {
finish_test
return
}

source $testdir/genesis.tcl

do_execsql_test 1.1 {
CREATE TABLE t1(docid, words);
}
fts_kjv_genesis

do_execsql_test 1.2 {
CREATE VIRTUAL TABLE x1 USING fts3;
INSERT INTO x1(x1) VALUES('nodesize=64');
INSERT INTO x1(x1) VALUES('maxpending=64');
}

do_execsql_test 1.3 {
INSERT INTO x1(docid, content) SELECT * FROM t1;
}

for {set tn 1} {1} {incr tn} {
set tc1 [db total_changes]
do_execsql_test 1.4.$tn.1 {
INSERT INTO x1(x1) VALUES('merge=1,2');
}
set tc2 [db total_changes]

if {($tc2 - $tc1)<2} break

do_execsql_test 1.4.$tn.1 {
INSERT INTO x1(x1) VALUES('integrity-check');
}
}



finish_test
1 change: 1 addition & 0 deletions third_party/sqlite/patched/test/permutations.test
Expand Up @@ -126,6 +126,7 @@ set allquicktests [test_set $alltests -exclude {
walcrash2.test e_fkey.test backup.test

fts4merge.test fts4merge2.test fts4merge4.test fts4check.test
fts4merge5.test
fts3cov.test fts3snippet.test fts3corrupt2.test fts3an.test
fts3defer.test fts4langid.test fts3sort.test fts5unicode.test

Expand Down
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Sarthak Kukreti <sarthakkukreti@chromium.org>
Date: Mon, 15 Jul 2019 17:23:45 -0700
Subject: [PATCH 1/6] Call ioctl() with the correct signature on both Android
Subject: [PATCH 1/8] Call ioctl() with the correct signature on both Android
and stock Linux.

This backports https://sqlite.org/src/info/68e12e063fe41bcd
Expand Down Expand Up @@ -32,5 +32,5 @@ index 52ef64116444..894c725b5265 100644
}; /* End of the overrideable system calls */

--
2.23.0.351.gc4317032e6-goog
2.20.1 (Apple Git-117)

@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Darwin Huang <huangdarwin@chromium.org>
Date: Wed, 17 Jul 2019 14:52:39 -0700
Subject: [PATCH 2/6] Fix Heap-Buffer-Overflow
Subject: [PATCH 2/8] Fix Heap-Buffer-Overflow

Backports https://www.sqlite.org/src/info/bd9a47a3a2997bfb

Expand Down Expand Up @@ -130,5 +130,5 @@ index 000000000000..500f2bd157cd
+finish_test
+
--
2.23.0.351.gc4317032e6-goog
2.20.1 (Apple Git-117)

@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Darwin Huang <huangdarwin@chromium.org>
Date: Wed, 17 Jul 2019 15:22:41 -0700
Subject: [PATCH 3/6] Fix ASSERT memIsValid hit
Subject: [PATCH 3/8] Fix ASSERT memIsValid hit

Backports https://www.sqlite.org/src/info/7ef7b23cbb1b9ace

Expand Down Expand Up @@ -228,5 +228,5 @@ index 000000000000..126e68751b28
+finish_test
\ No newline at end of file
--
2.23.0.351.gc4317032e6-goog
2.20.1 (Apple Git-117)

4 changes: 2 additions & 2 deletions third_party/sqlite/patches/0004-Fix-incorrect-assert.patch
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Darwin Huang <huangdarwin@chromium.org>
Date: Wed, 17 Jul 2019 15:24:25 -0700
Subject: [PATCH 4/6] Fix incorrect assert
Subject: [PATCH 4/8] Fix incorrect assert

Backports https://www.sqlite.org/src/info/59c9e73f86b89ee1

Expand Down Expand Up @@ -29,5 +29,5 @@ index c0ae8f00922e..23cb6bb3b083 100644
if( pPage->nOverflow || sz+2>pPage->nFree ){
if( pTemp ){
--
2.23.0.351.gc4317032e6-goog
2.20.1 (Apple Git-117)

@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Darwin Huang <huangdarwin@chromium.org>
Date: Tue, 23 Jul 2019 15:11:19 -0700
Subject: [PATCH 5/6] Fix bad chrome_sqlite3_free
Subject: [PATCH 5/8] Fix bad chrome_sqlite3_free

Backports https://www.sqlite.org/src/info/f60a83069168899d

Expand Down Expand Up @@ -29,5 +29,5 @@ index 23cb6bb3b083..887be63bd7d0 100644
return SQLITE_OK;
}
--
2.23.0.351.gc4317032e6-goog
2.20.1 (Apple Git-117)

@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Darwin Huang <huangdarwin@chromium.org>
Date: Wed, 25 Sep 2019 14:58:51 -0700
Subject: [PATCH 6/6] Avoid dangling schema pointer
Subject: [PATCH 6/8] Avoid dangling schema pointer

Backports https://www.sqlite.org/src/info/069c2f4c61f06211

Expand Down Expand Up @@ -107,5 +107,5 @@ index 3cfd2fa2f0ab..489a51e2e5a0 100644
+
finish_test
--
2.23.0.351.gc4317032e6-goog
2.20.1 (Apple Git-117)

@@ -0,0 +1,40 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Conrad Irwin <conrad.irwin@gmail.com>
Date: Wed, 2 Oct 2019 11:17:40 -0700
Subject: [PATCH 7/8] The nodeReaderInit() function in FTS3 may not assume that
the node is non-empty.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Backports https://www.sqlite.org/src/info/361eb2f682a303bb

Bug: 998494
---
third_party/sqlite/patched/ext/fts3/fts3_write.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/third_party/sqlite/patched/ext/fts3/fts3_write.c b/third_party/sqlite/patched/ext/fts3/fts3_write.c
index 4be6941579c5..6152c82871cf 100644
--- a/third_party/sqlite/patched/ext/fts3/fts3_write.c
+++ b/third_party/sqlite/patched/ext/fts3/fts3_write.c
@@ -3797,14 +3797,14 @@ static int nodeReaderInit(NodeReader *p, const char *aNode, int nNode){
p->nNode = nNode;

/* Figure out if this is a leaf or an internal node. */
- if( p->aNode[0] ){
+ if( aNode && aNode[0] ){
/* An internal node. */
p->iOff = 1 + sqlite3Fts3GetVarint(&p->aNode[1], &p->iChild);
}else{
p->iOff = 1;
}

- return nodeReaderNext(p);
+ return aNode ? nodeReaderNext(p) : SQLITE_OK;
}

/*
--
2.20.1 (Apple Git-117)

@@ -0,0 +1,112 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Conrad Irwin <conrad.irwin@gmail.com>
Date: Fri, 4 Oct 2019 09:30:44 -0700
Subject: [PATCH 8/8] Fix a long-standing problem in fts4 incrmental merge.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Backport https://sqlite.org/src/info/67da31e24ebb49c4

Bug: 998494
---
.../sqlite/patched/ext/fts3/fts3_write.c | 4 +-
.../sqlite/patched/test/fts4merge5.test | 58 +++++++++++++++++++
.../sqlite/patched/test/permutations.test | 1 +
3 files changed, 61 insertions(+), 2 deletions(-)
create mode 100644 third_party/sqlite/patched/test/fts4merge5.test

diff --git a/third_party/sqlite/patched/ext/fts3/fts3_write.c b/third_party/sqlite/patched/ext/fts3/fts3_write.c
index 6152c82871cf..c22711dad8c7 100644
--- a/third_party/sqlite/patched/ext/fts3/fts3_write.c
+++ b/third_party/sqlite/patched/ext/fts3/fts3_write.c
@@ -4295,8 +4295,8 @@ static int fts3IncrmergeLoad(
NodeReader reader;
pNode = &pWriter->aNodeWriter[i];

- rc = nodeReaderInit(&reader, pNode->block.a, pNode->block.n);
- if( reader.aNode ){
+ if ( pNode->block.a){
+ rc = nodeReaderInit(&reader, pNode->block.a, pNode->block.n);
while( reader.aNode && rc==SQLITE_OK ) rc = nodeReaderNext(&reader);
blobGrowBuffer(&pNode->key, reader.term.n, &rc);
if( rc==SQLITE_OK ){
diff --git a/third_party/sqlite/patched/test/fts4merge5.test b/third_party/sqlite/patched/test/fts4merge5.test
new file mode 100644
index 000000000000..1fad778b95e7
--- /dev/null
+++ b/third_party/sqlite/patched/test/fts4merge5.test
@@ -0,0 +1,58 @@
+# 2019 October 02
+#
+# The author disclaims copyright to this source code. In place of
+# a legal notice, here is a blessing:
+#
+# May you do good and not evil.
+# May you find forgiveness for yourself and forgive others.
+# May you share freely, never taking more than you give.
+#
+#*************************************************************************
+# This file implements regression tests for SQLite library. The
+# focus of this script is testing the FTS4 module.
+#
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+set testprefix fts4merge5
+
+# If SQLITE_ENABLE_FTS3 is defined, omit this file.
+ifcapable !fts3 {
+ finish_test
+ return
+}
+
+source $testdir/genesis.tcl
+
+do_execsql_test 1.1 {
+ CREATE TABLE t1(docid, words);
+}
+fts_kjv_genesis
+
+do_execsql_test 1.2 {
+ CREATE VIRTUAL TABLE x1 USING fts3;
+ INSERT INTO x1(x1) VALUES('nodesize=64');
+ INSERT INTO x1(x1) VALUES('maxpending=64');
+}
+
+do_execsql_test 1.3 {
+ INSERT INTO x1(docid, content) SELECT * FROM t1;
+}
+
+for {set tn 1} {1} {incr tn} {
+ set tc1 [db total_changes]
+ do_execsql_test 1.4.$tn.1 {
+ INSERT INTO x1(x1) VALUES('merge=1,2');
+ }
+ set tc2 [db total_changes]
+
+ if {($tc2 - $tc1)<2} break
+
+ do_execsql_test 1.4.$tn.1 {
+ INSERT INTO x1(x1) VALUES('integrity-check');
+ }
+}
+
+
+
+finish_test
diff --git a/third_party/sqlite/patched/test/permutations.test b/third_party/sqlite/patched/test/permutations.test
index ac2125c73b12..717998ef3e5d 100644
--- a/third_party/sqlite/patched/test/permutations.test
+++ b/third_party/sqlite/patched/test/permutations.test
@@ -126,6 +126,7 @@ set allquicktests [test_set $alltests -exclude {
walcrash2.test e_fkey.test backup.test

fts4merge.test fts4merge2.test fts4merge4.test fts4check.test
+ fts4merge5.test
fts3cov.test fts3snippet.test fts3corrupt2.test fts3an.test
fts3defer.test fts4langid.test fts3sort.test fts5unicode.test

--
2.20.1 (Apple Git-117)

0 comments on commit f9f8801

Please sign in to comment.