Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Jan Nijtmans
2014-08-08 23:34 GMT+02:00 Richard Hipp>:
> Please continue to test the latest snapshots and report any issues you find.

I already reported this to sqlite-dev, but without test-case exposing the
problem. Here is the test-case, which is 100% reproducable.
And below is a minimal patch which fixes this problem.

=============== main.c ================
#include <sqlite3.h>
int main(){
        sqlite3_mutex *mutex = sqlite3_mutex_alloc(SQLITE_MUTEX_RECURSIVE);
        int rc = sqlite3_mutex_try(mutex);
        if( rc == SQLITE_OK) sqlite3_mutex_leave(mutex);
}
======================================

Compile this using any Windows compiler (MSVC, MinGW, MinGW-w64)
with the following options.e.g.:

i686-w64-mingw32-gcc -o bug.exe -DNTDDI_VERSION=NTDDI_WINBLUE
-D_WIN32_WINNT=0x0400 -DSQLITE_THREADSAFE=1 main.c sqlite3.c

Then simply run "bug.exe", it will crash immediately in the function
sqlite3_win32_is_nt(), because osGetVersionExW is a NULL pointer.

Regards,
        Jan Nijtmans

---------- Forwarded message ----------
From: Jan Nijtmans <[hidden email]>
Date: 2014-08-05 13:24 GMT+02:00
Subject: Re: SQLite trunk on win32 broken with -DSQLITE_WIN32_NO_ANSI
To: sqlite-dev <[hidden email]>


2014-08-03 9:37 GMT+02:00 Jan Nijtmans <[hidden email]>:
> Fixed here:
>   <http://www.sqlite.org/src/info/ba78265429>
>
> Thanks!

Actually, just discovered that it is still not fixed for
all cases: When SQLITE_WIN32_GETVERSIONEX
is equal to 0 (later MS SDK's), then any call to
sqlite3_win32_is_nt() will crash because the
function osGetVersionExW() is a null pointer ;-(

Suggested patch (current SQLite trunk) below.

Regards,
        Jan Nijtmans

Index: src/os_win.c
==================================================================
--- src/os_win.c
+++ src/os_win.c
@@ -413,11 +413,11 @@
 ** In order to facilitate testing on a WinNT system, the test fixture
 ** can manually set this value to 1 to emulate Win98 behavior.
 */
 #ifdef SQLITE_TEST
 LONG volatile sqlite3_os_type = 0;
-#else
+#elif SQLITE_WIN32_GETVERSIONEX
 static LONG volatile sqlite3_os_type = 0;
 #endif

 #ifndef SYSCALL
 #  define SYSCALL sqlite3_syscall_ptr
@@ -1308,10 +1308,11 @@
 /*
 ** This function determines if the machine is running a version of Windows
 ** based on the NT kernel.
 */
 int sqlite3_win32_is_nt(void){
+#if SQLITE_WIN32_GETVERSIONEX
   if( osInterlockedCompareExchange(&sqlite3_os_type, 0, 0)==0 ){
 #if defined(NTDDI_VERSION) && NTDDI_VERSION >= NTDDI_WIN8
     OSVERSIONINFOW sInfo;
     sInfo.dwOSVersionInfoSize = sizeof(sInfo);
     osGetVersionExW(&sInfo);
@@ -1322,10 +1323,13 @@
 #endif
     osInterlockedCompareExchange(&sqlite3_os_type,
         (sInfo.dwPlatformId == VER_PLATFORM_WIN32_NT) ? 2 : 1, 0);
   }
   return osInterlockedCompareExchange(&sqlite3_os_type, 2, 2)==2;
+#else
+  return 1;
+#endif
 }

 #ifdef SQLITE_WIN32_MALLOC
 /*
 ** Allocate nBytes of memory.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

KlaasV


 Jan wrote: " ** can manually set this value to 1 to emulate Win98 behavior.
*/"

Can anyone give me one good reason apart from nostalgia to support a MS system not supported by MS?


Kind regards | Cordiali saluti | Vriendelijke groeten | Freundliche Grüsse,
Klaas `Z4us` V  - OrcID 0000-0001-7190-2544
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Klaas "Z4us" V, MetaDBA at InnocentIsArt.EU
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Jan Nijtmans
In reply to this post by Jan Nijtmans
2014-08-11 13:12 GMT+02:00 Jan Nijtmans <[hidden email]>:

> And below is a minimal patch which fixes this problem.
>

Unfortunately, latest SQLite trunk, which fixes the
SQLITE_WIN32_GETVERSIONEX problem brought
back the SQLITE_WIN32_NO_ANSI problem
I reported earlier   -(

$ i686-w64-mingw32-gcc -c -DSQLITE_WIN32_NO_ANSI sqlite3.c
sqlite3.c: In function ‘sqlite3_win32_is_nt’:
sqlite3.c:33312:10: error: ‘sInfo’ undeclared (first use in this function)
         (sInfo.dwPlatformId == VER_PLATFORM_WIN32_NT) ? 2 : 1, 0);
          ^
sqlite3.c:33312:10: note: each undeclared identifier is reported only once
for each function it appears in

I could deliver a patch, but there are two fundementally
different (but both correct) approaches to handle this.

Regards,
      Jan Nijtmans
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Adam DeVita
In reply to this post by KlaasV
1a) Somebody is paying you to do it.
1b) Incremental cost (or risk) of supporting it is small compared to the
cost (or risk) of porting /upgrading

Adam


On Tue, Aug 12, 2014 at 2:46 AM, Klaas V <[hidden email]> wrote:

>
>
>  Jan wrote: " ** can manually set this value to 1 to emulate Win98
> behavior.
> */"
>
> Can anyone give me one good reason apart from nostalgia to support a MS
> system not supported by MS?
>
>
> Kind regards | Cordiali saluti | Vriendelijke groeten | Freundliche Grüsse,
> Klaas `Z4us` V  - OrcID 0000-0001-7190-2544
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

jose isaias cabrera
In reply to this post by KlaasV

"Klaas V" wrote...
>
> Jan wrote: " ** can manually set this value to 1 to emulate Win98
> behavior.
> */"
>
> Can anyone give me one good reason apart from nostalgia to support a MS
> system not supported by MS?

There are 1000's of reason$ why, but if some win98 machines are doing jobs
that are working perfectly without any need to upgrade, why upgrade them?
If things are working correctly, why break it?  I am one of those that do
not upgrade just because.

josé

_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Richard Hipp-3
On Tue, Aug 12, 2014 at 6:50 PM, jose isaias cabrera <
[hidden email]> wrote:

>
> "Klaas V" wrote...
>
>
>> Jan wrote: " ** can manually set this value to 1 to emulate Win98
>> behavior.
>> */"
>>
>> Can anyone give me one good reason apart from nostalgia to support a MS
>> system not supported by MS?
>>
>
> There are 1000's of reason$ why, but if some win98 machines are doing jobs
> that are working perfectly without any need to upgrade, why upgrade them?
> If things are working correctly, why break it?  I am one of those that do
> not upgrade just because.
>

So if your old win98 machine is working perfectly, why do you need the
latest version of SQLite?  Won't an older version of SQLite that *does*
support win98 and that has been working perfectly for all these years
suffice?

--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

jose isaias cabrera
"Richard Hipp" wrote...


> On Tue, Aug 12, 2014 at 6:50 PM, jose isaias cabrera <
> [hidden email]> wrote:
>
>>
>> "Klaas V" wrote...
>>
>>
>>> Jan wrote: " ** can manually set this value to 1 to emulate Win98
>>> behavior.
>>> */"
>>>
>>> Can anyone give me one good reason apart from nostalgia to support a MS
>>> system not supported by MS?
>>>
>>
>> There are 1000's of reason$ why, but if some win98 machines are doing
>> jobs
>> that are working perfectly without any need to upgrade, why upgrade them?
>> If things are working correctly, why break it?  I am one of those that do
>> not upgrade just because.
>>
>
> So if your old win98 machine is working perfectly, why do you need the
> latest version of SQLite?  Won't an older version of SQLite that *does*
> support win98 and that has been working perfectly for all these years
> suffice?

I don't have any win98 machine. I was just responding to the question,

>>> Can anyone give me one good reason apart from nostalgia to support a MS
>>> system not supported by MS?

But, I don't have any issues with win98. :-)

josé

_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Jan Nijtmans
In reply to this post by Jan Nijtmans
2014-08-12 10:35 GMT+02:00 Jan Nijtmans <[hidden email]>:

>
> $ i686-w64-mingw32-gcc -c -DSQLITE_WIN32_NO_ANSI sqlite3.c
> sqlite3.c: In function ‘sqlite3_win32_is_nt’:
> sqlite3.c:33312:10: error: ‘sInfo’ undeclared (first use in this function)
>
>          (sInfo.dwPlatformId == VER_PLATFORM_WIN32_NT) ? 2 : 1, 0);
>           ^
> sqlite3.c:33312:10: note: each undeclared identifier is reported only once
> for each function it appears in
>

Fixed here: <http://www.sqlite.org/src/info/6715991296>

Thanks!

Looking at the function sqlite3_win32_is_nt():
    <http://www.sqlite.org/src/artifact/1c936c7b06?ln=1321-1337>
it's getting more and more complicated, without doing
really anything: It should simply return 1 on any
currently supported platform. Looking closely,
it returns 0 on Windows RT, but who cares ....
(leaving the "why" as practice for the reader)

Best overall-solution can be found here:
    <http://www.sqlite.org/src/info/169fc47e16>
A big +1 from me for this, paying any attention
to Win95 just isn't worth the effort IMHO!

Regards,
         Jan Nijtmans
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Jan Nijtmans
2014-08-15 12:50 GMT+02:00 Jan Nijtmans <[hidden email]>:
> Looking at the function sqlite3_win32_is_nt()
> It should simply return 1 on any
> currently supported platform. Looking closely,
> it returns 0 on Windows RT, but who cares ....
> (leaving the "why" as practice for the reader)

It looks like this is on its way to being corrected:
       <www.sqlite.org/src/info/2f59e71fbf>

However, I don't think this will work on Win95/98/NT
(not that I really care ......). The reason: GetVersionExW()
will be called to check whether the kernal is NT-based,
but Win95/98/NT is not NT-based so it doesn't have
this function  ;-). Suggested solution:
    <http://www.sqlite.org/src/info/169fc47e16>

Second-best suggested solution as patch below
(based on the winrt branch)

Regards,
          Jan Nijtmans

Index: src/os_win.c
==================================================================
--- src/os_win.c
+++ src/os_win.c
@@ -1323,17 +1323,17 @@
   **       kernel.
   */
   return 1;
 #elif defined(SQLITE_WIN32_GETVERSIONEX) && SQLITE_WIN32_GETVERSIONEX
   if( osInterlockedCompareExchange(&sqlite3_os_type, 0, 0)==0 ){
-#if defined(SQLITE_WIN32_HAS_WIDE)
+#if !defined(SQLITE_WIN32_HAS_ANSI)
     OSVERSIONINFOW sInfo;
     sInfo.dwOSVersionInfoSize = sizeof(sInfo);
     osGetVersionExW(&sInfo);
     osInterlockedCompareExchange(&sqlite3_os_type,
         (sInfo.dwPlatformId == VER_PLATFORM_WIN32_NT) ? 2 : 1, 0);
-#elif defined(SQLITE_WIN32_HAS_ANSI)
+#else
     OSVERSIONINFOA sInfo;
     sInfo.dwOSVersionInfoSize = sizeof(sInfo);
     osGetVersionExA(&sInfo);
     osInterlockedCompareExchange(&sqlite3_os_type,
         (sInfo.dwPlatformId == VER_PLATFORM_WIN32_NT) ? 2 : 1, 0);
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Crash in sqlite3_mutex_try [Was: SQLite 3.8.6 beta]

Jan Nijtmans
2014-08-22 9:14 GMT+02:00 Jan Nijtmans <[hidden email]>:
> It looks like this is on its way to being corrected:
>        <www.sqlite.org/src/info/2f59e71fbf>
>
> However, I don't think this will work on Win95/98/NT

Fixed here:
     <http://www.sqlite.org/src/info/9fe0f0754c>

Thanks! No more remarks!

Regards,
       Jan Nijtmans
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users