Thread: [PATCH] Fix float8 parsing of denormal values (on some platforms?)
Hi list, Back in June we had a discussion about parsing denormal floating-point values. A float8->text conversion could result in a number that can't be converted back to float8 anymore for some values. Among other things, this could break backups (though my searches didn't turn up any reports of this ever happening). The reason is that Linux strtod() sets errno=ERANGE for denormal numbers. This behavior is also explicitly allowed (implementation-defined) by the C99 standard. Further analysis was done by Jeroen Vermeulen here: http://archives.postgresql.org/pgsql-hackers/2011-06/msg01773.php I think the least invasive fix, as proposed by Jeroen, is to fail only when ERANGE is set *and* the return value is 0.0 or +/-HUGE_VAL. Reading relevant specifications, this seems to be a fairly safe assumption. That's what the attached patch does. (I also updated the float4in function, but that's not strictly necessary -- it would fail later in CHECKFLOATVAL() anyway) What I don't know is how many platforms are actually capable of parsing denormal double values. I added some regression tests, it would be interesting to see results from pgbuildfarm and potentially revert these tests later. Regards, Marti
On Wed, Dec 21, 2011 at 18:21, Marti Raudsepp <marti@juffo.org> wrote: > I think the least invasive fix, as proposed by Jeroen, is to fail only > when ERANGE is set *and* the return value is 0.0 or +/-HUGE_VAL. > Reading relevant specifications, this seems to be a fairly safe > assumption. That's what the attached patch does. Oops, now attached the patch too. Regards, Marti
Attachment
At 2011-12-21 18:24:17 +0200, marti@juffo.org wrote: > > > I think the least invasive fix, as proposed by Jeroen, is to fail only > > when ERANGE is set *and* the return value is 0.0 or +/-HUGE_VAL. > > Reading relevant specifications, this seems to be a fairly safe > > assumption. That's what the attached patch does. > > Oops, now attached the patch too. Approach seems sensible, patch applies, builds, and passes tests. Marking ready for committer and crossing fingers about buildfarm failures… -- ams
Marti Raudsepp <marti@juffo.org> writes:
> On Wed, Dec 21, 2011 at 18:21, Marti Raudsepp <marti@juffo.org> wrote:
>> I think the least invasive fix, as proposed by Jeroen, is to fail only
>> when ERANGE is set *and* the return value is 0.0 or +/-HUGE_VAL.
>> Reading relevant specifications, this seems to be a fairly safe
>> assumption. That's what the attached patch does.
> Oops, now attached the patch too.
Applied with minor revisions. Notably, after staring at the code a bit
I got uncomfortable with its assumption that pg_strncasecmp() cannot
change errno, so I fixed it to not assume that. Also, on some platforms
HUGE_VAL isn't infinity but the largest finite value, so I made the
range tests be like ">= HUGE_VAL" not just "== HUGE_VAL". I know the
man page for strtod() specifies it should return exactly HUGE_VAL for
overflow, but who's to say that <math.h> is on the same page as the
actual function?
regards, tom lane
On Wed, Feb 1, 2012 at 20:17, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Applied with minor revisions. Thanks! :) We're already seeing first buildfarm failures, on system "narwhal" using an msys/mingw compiler. http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=narwhal&dt=2012-02-02%2005%3A00%3A02 No idea which libc it uses though. The MSVC libc looks fine because "currawong" passes these tests. PS: it would be useful to have the output of 'cc -v' in buildfarm reports since the "Compiler" field may be out of date. Regards, Marti
Marti Raudsepp <marti@juffo.org> writes:
> We're already seeing first buildfarm failures, on system "narwhal"
> using an msys/mingw compiler.
Yeah. After a full day's cycle, the answer seems to be that
denormalized input works fine everywhere except:
1. mingw on Windows (even though MSVC builds work)
2. some Gentoo builds fail (knowing that platform, the phase of the moon
is probably the most predictable factor determining this).
I'm inclined at this point to remove the regression test cases, because
we have no principled way to deal with case #2. We could install
alternative "expected" files that accept the failure, but then what's
the point of having the test case at all?
It might be worth pressuring the mingw folk to get with the program,
but I'm not going to be the one to do that.
regards, tom lane
On 02/03/2012 03:49 AM, Tom Lane wrote: > Marti Raudsepp<marti@juffo.org> writes: >> We're already seeing first buildfarm failures, on system "narwhal" >> using an msys/mingw compiler. > Yeah. After a full day's cycle, the answer seems to be that > denormalized input works fine everywhere except: > > 1. mingw on Windows (even though MSVC builds work) > > 2. some Gentoo builds fail (knowing that platform, the phase of the moon > is probably the most predictable factor determining this). > > I'm inclined at this point to remove the regression test cases, because > we have no principled way to deal with case #2. We could install > alternative "expected" files that accept the failure, but then what's > the point of having the test case at all? > > It might be worth pressuring the mingw folk to get with the program, > but I'm not going to be the one to do that. > > No doubt they would tell us to upgrade the compiler. Narwhal's is horribly old. Neither of my mingw-based members is failing. cheers andrew