Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Detlef Golze
Hi,

the following looks like an incorrect use of SQLITE_DEFAULT_CACHE_SIZE (amalgamation 3.17 starting at line 37963):

/*
 * This is cache size used in the calculation of the initial size of the
 * Win32-specific heap.  It cannot be negative.
 */
#ifndef SQLITE_WIN32_CACHE_SIZE
#  if SQLITE_DEFAULT_CACHE_SIZE>=0
#    define SQLITE_WIN32_CACHE_SIZE (SQLITE_DEFAULT_CACHE_SIZE)
#  else
#    define SQLITE_WIN32_CACHE_SIZE (-(SQLITE_DEFAULT_CACHE_SIZE))
#  endif
#endif

/*
 * The initial size of the Win32-specific heap.  This value may be zero.
 */
#ifndef SQLITE_WIN32_HEAP_INIT_SIZE
#  define SQLITE_WIN32_HEAP_INIT_SIZE ((SQLITE_WIN32_CACHE_SIZE) * \
                                       (SQLITE_DEFAULT_PAGE_SIZE) + 4194304)
#endif

SQLITE_DEFAULT_CACHE_SIZE is defined as page size if positive or Kbyte if negative. I came here because I got an integer overflow in the definition of SQLITE_WIN32_HEAP_INIT_SIZE. I did not look for further consequences.

Thank you,
Detlef.

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

Re: Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Joe Mistachkin-3

Detlef Golze wrote:
>
> SQLITE_DEFAULT_CACHE_SIZE is defined as page size if positive or
> Kbyte if negative.
>

The SQLITE_DEFAULT_CACHE_SIZE value is always measured in pages.

>
> I came here because I got an integer overflow in the definition
> of SQLITE_WIN32_HEAP_INIT_SIZE.
>

What values are you using for the SQLITE_DEFAULT_CACHE_SIZE and
SQLITE_DEFAULT_PAGE_SIZE defines?

--
Joe Mistachkin @ https://urn.to/r/mistachkin

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

Re: Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Detlef Golze
> The SQLITE_DEFAULT_CACHE_SIZE value is always measured in pages.

http://sqlite.org/compile.html#default_cache_size
http://sqlite.org/pragma.html#pragma_cache_size

My interpretation is that the option to provide a negative value has been introduced to allow specification of cache size limits independent of used page size. And it has been used like that:

Changes carried forward from version 3.12.0 (2016-03-29):

    Potentially Disruptive Change:
    The SQLITE_DEFAULT_PAGE_SIZE is increased from 1024 to 4096. The SQLITE_DEFAULT_CACHE_SIZE is changed from 2000 to -2000 so the same amount of cache memory is used by default. See the application note on the version 3.12.0 page size change for further information.


Am I missing something?

Best regards,
Detlef.

-----Original Message-----
From: sqlite-users [mailto:[hidden email]] On Behalf Of Joe Mistachkin
Sent: Monday, February 20, 2017 10:02 AM
To: 'SQLite mailing list'
Subject: Re: [sqlite] Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE


Detlef Golze wrote:
>
> SQLITE_DEFAULT_CACHE_SIZE is defined as page size if positive or
> Kbyte if negative.
>

The SQLITE_DEFAULT_CACHE_SIZE value is always measured in pages.

>
> I came here because I got an integer overflow in the definition
> of SQLITE_WIN32_HEAP_INIT_SIZE.
>

What values are you using for the SQLITE_DEFAULT_CACHE_SIZE and
SQLITE_DEFAULT_PAGE_SIZE defines?

--
Joe Mistachkin @ https://urn.to/r/mistachkin

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

Re: Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Joe Mistachkin-3

Detlef Golze wrote:
>
> Changes carried forward from version 3.12.0 (2016-03-29):
>

Ah, right.  I was reading the comments in the source code.

I've checked-in some changes that should prevent integer
overflows when very large values are used for the
SQLITE_DEFAULT_CACHE_SIZE and/or SQLITE_DEFAULT_PAGE_SIZE
defines.

--
Joe Mistachkin @ https://urn.to/r/mistachkin

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

Re: Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Detlef Golze
Hello Joe,

thank you for your attention, but your change does not fully address my issue.

This code still assumes that SQLITE_WIN32_CACHE_SIZE is measured in pages:

#ifndef SQLITE_WIN32_HEAP_INIT_SIZE
#  define SQLITE_WIN32_HEAP_INIT_SIZE   ((SQLITE_WIN32_CACHE_SIZE) * \
                                         (SQLITE_DEFAULT_PAGE_SIZE) + \
                                         (SQLITE_WIN32_HEAP_INIT_EXTRA))
#endif

But it is not:

#ifndef SQLITE_WIN32_CACHE_SIZE
#  if SQLITE_DEFAULT_CACHE_SIZE>=0
#    define SQLITE_WIN32_CACHE_SIZE     (SQLITE_DEFAULT_CACHE_SIZE)
#  else
#    define SQLITE_WIN32_CACHE_SIZE     (-(SQLITE_DEFAULT_CACHE_SIZE))
#  endif
#endif

If we go to the else branch, the SQLITE_WIN32_CACHE_SIZE will be KByte rather than pages. Your change may solve a potential overflow, but not the wrong calculation of SQLITE_WIN32_HEAP_INIT_SIZE if SQLITE_DEFAULT_PAGE_SIZE is specified as negative value (Kbyte).

Thanks,
Detlef.

-----Original Message-----
From: sqlite-users [mailto:[hidden email]] On Behalf Of Joe Mistachkin
Sent: Monday, February 20, 2017 8:25 PM
To: 'SQLite mailing list'
Subject: Re: [sqlite] Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE


Detlef Golze wrote:
>
> Changes carried forward from version 3.12.0 (2016-03-29):
>

Ah, right.  I was reading the comments in the source code.

I've checked-in some changes that should prevent integer
overflows when very large values are used for the
SQLITE_DEFAULT_CACHE_SIZE and/or SQLITE_DEFAULT_PAGE_SIZE
defines.

--
Joe Mistachkin @ https://urn.to/r/mistachkin

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

Re: Bug? Incorrect use of SQLITE_DEFAULT_CACHE_SIZE

Joe Mistachkin-3

Detlef Golze wrote:
>
> This code still assumes that SQLITE_WIN32_CACHE_SIZE is measured in pages:

>

The value of SQLITE_WIN32_HEAP_INIT_SIZE is an initial estimate of the
eventual heap size.  It does not have to be an exact figure.  Using either
KiB or pages normally ends up giving a fairly reasonable value.

Alternatively, it can be overridden at compile-time.

--
Joe Mistachkin @ https://urn.to/r/mistachkin

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users