On 07/15/2015 08:34 PM, T?r?k Edwin wrote:
> On 07/15/2015 08:17 PM, Richard Hipp wrote:
>> On 7/15/15, T?r?k Edwin <edwin+sqlite3 at etorok.net> wrote:
>>> This might be just the test runner and not sqlite itself, I'm not sure:
>>>
>>> Time: capi2.test 25 ms
>>> =================================================================
>>> ==2330==ERROR: AddressSanitizer: heap-use-after-free on address
>>> 0x6180003b58dc at pc 0x7f5bb4894d49 bp 0x7ffd1e988d20 sp 0x7ffd1e988d18
>>> READ of size 4 at 0x6180003b58dc thread T0
>>> #0 0x7f5bb4894d48 in sqlite3SafetyCheckSickOrOk
>>> /home/edwin/skylable/sqlite/sqlite3.c:25082
>>> #1 0x7f5bb49b1174 in sqlite3Close
>>
>> I'm not able to reproduce this problem.
>>
>> Under valgrind, there are some tests in capi3c.test that deliberately
>> do use-after-free in order to test some safety mechanisms built into
>> SQLite. All such test cases have "misuse" in their names.
>>
>> The safety mechanisms implemented by sqlite3SafetyCheckSickOrOk() try
>> to validate that an "sqlite3*" pointer passed into various APIs is
>> really a valid "sqlite3*" pointer that has not been closed by looking
>> at the "magic" integer that is part of the sqlite3 object. If the
>> magic number is not correct, an error is raised. The magic number is
>> cleared whenever a database connection is closed.
>>
>> The tests in this case close a database connection, which causes the
>> sqlite3 object to be deallocated. Then they pass a pointer to that
>> now-deallocated object into some other API and verify that the error
>> is immediately detected and that the API returns SQLITE_MISUSE.
>>
>> Obviously, such things should never occur in a debugged applications.
>> No application should ever do anything that returns SQLITE_MISUSE.
>> The SQLITE_MISUSE return code is purely to alert programmers to coding
>> errors during development.
Yes, SQLITE_MISUSE is very useful (along with SQLITE_CONFIG_LOG) at catching
errors early.
>>
>> And, also obviously, since this mechanism violates memory safety rules
>> (by reading the "magic" field in a block of memory that has been
>> freed), it will raise alarms with memory debuggers.
>>
>> The test/releasetest.tcl script runs many test cases, including some
>> that make use of -fsanitize on clang and that use valgrind. Whenever
>> the test/releasetest.tcl script uses memory debuggers, it is always
>> careful to omit "misuse" tests.
>
> Thanks for the excellent explanation as always.
> Apparently GCC's and Clang's -fsanitize=address define different macros [1],
> so
> lets enhance that detection to include GCC's -fsanitize=address too
> (perhaps the function should be renamed too as its no longer clang specific):
>
> Index: src/test1.c
> ==================================================================
> --- src/test1.c
> +++ src/test1.c
> @@ -271,10 +271,13 @@
> int res = 0;
> #if defined(__has_feature)
> # if __has_feature(address_sanitizer)
> res = 1;
> # endif
> +#endif
> +#ifdef __SANITIZE_ADDRESS__
> + res = 1;
> #endif
> if( res==0 && getenv("OMIT_MISUSE")!=0 ) res = 1;
> Tcl_SetObjResult(interp, Tcl_NewIntObj(res));
> return TCL_OK;
> }
>
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56454#c5
I have run with these patches and my original configure line with GCC's address
sanitizer and 'make test' doesn't crash with use-after-free anymore:
1 errors out of 140291 tests
Failures on these tests: shell1-5.0
shell1-5.0...
Error: failed with byte E0 mismatch
In fact such detection could be added to fuzzcheck.c too (I didn't know that
you can detect presence of address sanitizer at compile time until now):
Index: test/fuzzcheck.c
==================================================================
--- test/fuzzcheck.c
+++ test/fuzzcheck.c
@@ -737,10 +737,19 @@
char *zDbName = ""; /* Appreviated name of a source database */
const char *zFailCode = 0; /* Value of the TEST_FAILURE environment
variable */
int cellSzCkFlag = 0; /* --cell-size-check */
int sqlFuzz = 0; /* True for SQL fuzz testing. False for DB fuzz
*/
int iTimeout = 120; /* Default 120-second timeout */
+
+#if defined(__has_feature)
+# if __has_feature(address_sanitizer)
+ cellSzCkFlag = 1;
+# endif
+#endif
+#ifdef __SANITIZE_ADDRESS__
+ cellSzCkFlag = 1;
+#endif
iBegin = timeOfDay();
#ifdef __unix__
signal(SIGALRM, timeoutHandler);
#endif
