Lists: | pgsql-hackers |
---|
From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Ancient bug in formatting.c/to_char() |
Date: | 2014-05-29 01:59:23 |
Message-ID: | CAM3SWZRJJVQ2htBpt5=SAy7z=m7wbhSEAyF5vN82NFm8gXJygw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Consider this example:
[local]/postgres=# SELECT to_char(1e9::float8,'999999999999999999999D9');
to_char
--------------------------
1000000000.0
(1 row)
[local]/postgres=# SELECT to_char(1e20::float8,'999999999999999999999D9');
to_char
------------------------
100000000000000000000
(1 row)
[local]/postgres=# SELECT to_char(1e20,'999999999999999999999D9');
to_char
--------------------------
100000000000000000000.0
(1 row)
There is access to uninitialized memory here. #define
DEBUG_TO_FROM_CHAR shows this to be the case (garbage is printed to
stdout).
Valgrind brought this to my attention. I propose the attached patch as
a fix for this issue.
The direct cause appears to be that float8_to_char() feels entitled to
truncate Number description post-digits, while that doesn't happen
within numeric_to_char(), say. This is very old code, from the
original to_char() patch back in 2000, and the interactions here are
not obvious. I think that that should be okay to not do this
truncation, since the value of MAXDOUBLEWIDTH was greatly increased in
2007 as part of a round of fixes to bugs of similar vintage. There is
still a defensive measure against over-sized input (we bail), but that
ought to be unnecessary, strictly speaking.
--
Peter Geoghegan
Attachment | Content-Type | Size |
---|---|---|
float8_to_char_fix.patch | text/x-patch | 1.7 KB |
From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | Peter Geoghegan <pg(at)heroku(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Ancient bug in formatting.c/to_char() |
Date: | 2014-05-29 10:09:43 |
Message-ID: | 53870767.8090208@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 05/29/2014 04:59 AM, Peter Geoghegan wrote:
> Consider this example:
>
> [local]/postgres=# SELECT to_char(1e9::float8,'999999999999999999999D9');
> to_char
> --------------------------
> 1000000000.0
> (1 row)
>
> [local]/postgres=# SELECT to_char(1e20::float8,'999999999999999999999D9');
> to_char
> ------------------------
> 100000000000000000000
> (1 row)
>
> [local]/postgres=# SELECT to_char(1e20,'999999999999999999999D9');
> to_char
> --------------------------
> 100000000000000000000.0
> (1 row)
>
>
> There is access to uninitialized memory here. #define
> DEBUG_TO_FROM_CHAR shows this to be the case (garbage is printed to
> stdout).
>
> Valgrind brought this to my attention. I propose the attached patch as
> a fix for this issue.
>
> The direct cause appears to be that float8_to_char() feels entitled to
> truncate Number description post-digits, while that doesn't happen
> within numeric_to_char(), say. This is very old code, from the
> original to_char() patch back in 2000, and the interactions here are
> not obvious. I think that that should be okay to not do this
> truncation, since the value of MAXDOUBLEWIDTH was greatly increased in
> 2007 as part of a round of fixes to bugs of similar vintage. There is
> still a defensive measure against over-sized input (we bail), but that
> ought to be unnecessary, strictly speaking.
postgres=# select to_char('0.1'::float8, '9D' || repeat('9', 1000));
ERROR: double precision for character conversion is too wide
Yeah, the code is pretty crufty, I'm not sure what the proper fix would
be. I wonder if psprintf would be useful.
- Heikki
From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Ancient bug in formatting.c/to_char() |
Date: | 2014-05-29 16:30:55 |
Message-ID: | CAM3SWZSdkw15k3VqmMeO98yYH=pxmDWq1RJgWC3HcCAWX0rG3A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Thu, May 29, 2014 at 3:09 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> Yeah, the code is pretty crufty, I'm not sure what the proper fix would be.
> I wonder if psprintf would be useful.
I don't know that it's worth worrying about the case you highlight. It
is certainly worth worrying about the lack of a similar fix in
float4_to_char(), though.
--
Peter Geoghegan
From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Ancient bug in formatting.c/to_char() |
Date: | 2014-07-02 23:27:13 |
Message-ID: | CAM3SWZRyKpUL7ye2mgPijLA8sxsio4=FvACRbgTS7pAazgBkAQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Where are we on this?
--
Peter Geoghegan