Re: [sqlite] Help with confirming a couple of error traces

[Richard Hipp]

On 2/2/17, Shaobo He  wrote:
> May I ask that do you see null pointer deferences
> during development regularly?

Sometimes, but not too often.  We get assertion faults more.  Or just
incorrect answers.
-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Help with confirming a couple of error traces

[Shaobo He]

Thanks, Richard. I think that I fully understand what happens now. Thanks
again for your patience. May I ask that do you see null pointer deferences
during development regularly?

Shaobo

Richard Hipp 于2017年2月1日周三 下午6:52写道：

> On 2/1/17, Shaobo He  wrote:
> > Thanks for your reply. I repeated the experiment by setting
> > db->mallocFailed upon return. You are right that there is no segmentation
> > fault (there were some assertion failures: e.g, "sqlite3OomClear:
> Assertion
> > `db->lookaside.bDisable>0' failed"). Instead I got error messages saying
> > out of memory. It makes sense now. May I ask where the unwinding is done?
> > Does it mean the program stops execution at sqlite3SrcListAppend()?
> >
>
> The assertion fault is probably because you are playing games with the
> memory allocator - pretending that a fault occurred when it did not.
> You might be able to work around that by compiling with
> -DSQLITE_OMIT_LOOKASIDE.
>
> An OOM in sqlite3SrcListAppend() will likely cause the parser to abort
> at https://www.sqlite.org/src/artifact/25ccc63ae?ln=547.
>
> The SQLite parser works by extracting tokens from the input string and
> sending them one by one into the pushdown automaton that implements
> the recognizes the LALR(1) grammar.  If you break out of that loop, it
> stops the parser dead in its tracks.  After that, all the left-over
> memory allocations are cleaned up and the tokenizer returns the
> SQLITE_NOMEM error.
>
> There are other places where a prior OOM can cause processing to
> abort.  Grep for "mallocFailed" to find them.  But the tokenizer loop
> is the most likely spot.
> --
> D. Richard Hipp
> d...@sqlite.org
> ___
> sqlite-users mailing list
> sqlite-users@mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Help with confirming a couple of error traces

[Richard Hipp]

On 2/1/17, Shaobo He  wrote:
> Thanks for your reply. I repeated the experiment by setting
> db->mallocFailed upon return. You are right that there is no segmentation
> fault (there were some assertion failures: e.g, "sqlite3OomClear: Assertion
> `db->lookaside.bDisable>0' failed"). Instead I got error messages saying
> out of memory. It makes sense now. May I ask where the unwinding is done?
> Does it mean the program stops execution at sqlite3SrcListAppend()?
>

The assertion fault is probably because you are playing games with the
memory allocator - pretending that a fault occurred when it did not.
You might be able to work around that by compiling with
-DSQLITE_OMIT_LOOKASIDE.

An OOM in sqlite3SrcListAppend() will likely cause the parser to abort
at https://www.sqlite.org/src/artifact/25ccc63ae?ln=547.

The SQLite parser works by extracting tokens from the input string and
sending them one by one into the pushdown automaton that implements
the recognizes the LALR(1) grammar.  If you break out of that loop, it
stops the parser dead in its tracks.  After that, all the left-over
memory allocations are cleaned up and the tokenizer returns the
SQLITE_NOMEM error.

There are other places where a prior OOM can cause processing to
abort.  Grep for "mallocFailed" to find them.  But the tokenizer loop
is the most likely spot.
-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Help with confirming a couple of error traces

[Shaobo He]

Thanks for your reply. I repeated the experiment by setting
db->mallocFailed upon return. You are right that there is no segmentation
fault (there were some assertion failures: e.g, "sqlite3OomClear: Assertion
`db->lookaside.bDisable>0' failed"). Instead I got error messages saying
out of memory. It makes sense now. May I ask where the unwinding is done?
Does it mean the program stops execution at sqlite3SrcListAppend()?

Shaobo

Richard Hipp 于2017年2月1日周三 下午5:28写道：

> On 2/1/17, Shaobo He  wrote:
> >
> > Basically, the error trace indicate that `sqlite3SrcListAppend` can
> return
> > a null pointer under the presence of OOM and this return value can
> > propagate to somewhere in the program, resulting in a null pointer
> > deference.
>
> An OOM condition inside of sqlite3SrcListAppend() will set the
> db->mallocFailed flag, which will cause the stack to unwind, and
> thereby prevent NULL pointer dereferences.
>
> Please repeat your experiment by forcing sqlite3SrcListAppend() to
> return NULL but at the same time set db->mallocFailed.  If you
> continue hit a segfault, that is an issue.  But I'm guessing you will
> not.
>
> We do a lot of OOM testing in SQLite.  See
> https://www.sqlite.org/testing.html#oomtesting for a summary of the
> technique.  There are 829 OOM test loops in TH3 and more in the TCL
> test suite.  We do, rarely, find OOM problems in release builds, but
> because of our OOM testing procedures such findings are quite rare.
> --
> D. Richard Hipp
> d...@sqlite.org
> ___
> sqlite-users mailing list
> sqlite-users@mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Help with confirming a couple of error traces

[Richard Hipp]

On 2/1/17, Shaobo He  wrote:
>
> Basically, the error trace indicate that `sqlite3SrcListAppend` can return
> a null pointer under the presence of OOM and this return value can
> propagate to somewhere in the program, resulting in a null pointer
> deference.

An OOM condition inside of sqlite3SrcListAppend() will set the
db->mallocFailed flag, which will cause the stack to unwind, and
thereby prevent NULL pointer dereferences.

Please repeat your experiment by forcing sqlite3SrcListAppend() to
return NULL but at the same time set db->mallocFailed.  If you
continue hit a segfault, that is an issue.  But I'm guessing you will
not.

We do a lot of OOM testing in SQLite.  See
https://www.sqlite.org/testing.html#oomtesting for a summary of the
technique.  There are 829 OOM test loops in TH3 and more in the TCL
test suite.  We do, rarely, find OOM problems in release builds, but
because of our OOM testing procedures such findings are quite rare.
-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Help with confirming a couple of error traces

[Shaobo He]

Hi Richard, all,

It's so nice of you to help out. Now we understand better what we should do
to reduce the number of false positives. Thanks a lot.

I'm sorry to bother you again. But it would be great if you could provide
some feedback on the new error trace returned by our tool.

Basically, the error trace indicate that `sqlite3SrcListAppend` can return
a null pointer under the presence of OOM and this return value can
propagate to somewhere in the program, resulting in a null pointer
deference. For instance, `targetSrcList` calls `sqlite3SrcListAppend` and
returns a null pointer if its callee does it, too. `codeTriggerProgram`
calls `sqlite3Insert` with a call expression to `targetSrcList` as its
second argument, which can be a null pointer following the deduction
before. Finally, its second argument `pTabList` gets dereferenced without a
null test.

I tried to do a dummy experiment by setting the return value of
`sqlite3DbMallocRawNN` inside `sqlite3SrcListAppend` to null and ran
regression tests. A number of them failed with segmentation fault. I don't
know if this experiment is meaningful or not.

Please let me know if it makes sense. Thanks for your time and I am looking
forward to your reply.

Shaobo
Richard Hipp 于2017年1月31日周二 下午9:41写道：

> On 1/31/17, Shaobo He  wrote:
> > Hi there,
> >
> > My name is Shaobo He and I am a graduate student at University of Utah. I
> > am applying a couple of static analysis tools to C projects. The tools I
> am
> > using reports a few partial error traces about null pointer
> dereferences. I
> > was wondering if you could help me to identify whether they (described
> > below) were true bugs or just false positives. Your feedback is really
> > appreciated.
>
> They are both false-positives.
>
> >
> > 1) In function `statGet`, `sqlite3_value_blob` can return a null pointer.
> > One possible case is that `ExpandBlob(p)` returns `SQLITE_OK` and the
> > condition expression `p->n ? p->z : 0;` evaluates to null given `p->n ==
> > 0`. I tried to figure out if `p->n` can be 0 by adding an assertion
> before
> > the call site to `sqlite_value_blob` and running all regression tests. It
> > seems it cannot be for these test cases. My question is that if the case
> > described above can happen. Moreover, function `statPush` has a similar
> > error trace.
>
> The first parameter to statGet() and statPush() will always be a
> sizeof(void*)-byte blob that is in fact a pointer to an object.  So
> sqlite3_value_blob() will never return NULL there.
>
> >
> > 2) In function `walCleanupHash`, `aHash` is initialized to null and is
> > probably updated by function `walHashGet`. However, the update may not
> > happen if `walIndexPage` returns a status not equal to `SQLITE_OK`. So
> > `aHash` remains null and got dereferenced.
>
> In walCleanupHash(), the pages of the -shm file that contains the hash
> have already been allocated and initialized - otherwise
> walCleanupHash() would have never been called.  But if the -shm file
> has already been allocated and initialized, then there is no way for
> walHashGet() to fail and leave aHash uninitialized.
>
> --
> D. Richard Hipp
> d...@sqlite.org
> ___
> sqlite-users mailing list
> sqlite-users@mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Help with confirming a couple of error traces

[Richard Hipp]

On 1/31/17, Shaobo He  wrote:
> Hi there,
>
> My name is Shaobo He and I am a graduate student at University of Utah. I
> am applying a couple of static analysis tools to C projects. The tools I am
> using reports a few partial error traces about null pointer dereferences. I
> was wondering if you could help me to identify whether they (described
> below) were true bugs or just false positives. Your feedback is really
> appreciated.

They are both false-positives.

>
> 1) In function `statGet`, `sqlite3_value_blob` can return a null pointer.
> One possible case is that `ExpandBlob(p)` returns `SQLITE_OK` and the
> condition expression `p->n ? p->z : 0;` evaluates to null given `p->n ==
> 0`. I tried to figure out if `p->n` can be 0 by adding an assertion before
> the call site to `sqlite_value_blob` and running all regression tests. It
> seems it cannot be for these test cases. My question is that if the case
> described above can happen. Moreover, function `statPush` has a similar
> error trace.

The first parameter to statGet() and statPush() will always be a
sizeof(void*)-byte blob that is in fact a pointer to an object.  So
sqlite3_value_blob() will never return NULL there.

>
> 2) In function `walCleanupHash`, `aHash` is initialized to null and is
> probably updated by function `walHashGet`. However, the update may not
> happen if `walIndexPage` returns a status not equal to `SQLITE_OK`. So
> `aHash` remains null and got dereferenced.

In walCleanupHash(), the pages of the -shm file that contains the hash
have already been allocated and initialized - otherwise
walCleanupHash() would have never been called.  But if the -shm file
has already been allocated and initialized, then there is no way for
walHashGet() to fail and leave aHash uninitialized.

-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

[sqlite] Help with confirming a couple of error traces

[Shaobo He]

Hi there,

My name is Shaobo He and I am a graduate student at University of Utah. I
am applying a couple of static analysis tools to C projects. The tools I am
using reports a few partial error traces about null pointer dereferences. I
was wondering if you could help me to identify whether they (described
below) were true bugs or just false positives. Your feedback is really
appreciated.

1) In function `statGet`, `sqlite3_value_blob` can return a null pointer.
One possible case is that `ExpandBlob(p)` returns `SQLITE_OK` and the
condition expression `p->n ? p->z : 0;` evaluates to null given `p->n ==
0`. I tried to figure out if `p->n` can be 0 by adding an assertion before
the call site to `sqlite_value_blob` and running all regression tests. It
seems it cannot be for these test cases. My question is that if the case
described above can happen. Moreover, function `statPush` has a similar
error trace.

2) In function `walCleanupHash`, `aHash` is initialized to null and is
probably updated by function `walHashGet`. However, the update may not
happen if `walIndexPage` returns a status not equal to `SQLITE_OK`. So
`aHash` remains null and got dereferenced.

Thanks,
Shaobo
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users