Horton

Re: Yet another fast GiST build

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, Justin Pryzby <pryzby@telsasoft.com>, Darafei Komяpa Praliaskouski <me@komzpa.net>, Thomas Munro <thomas.munro@gmail.com>, Michael Paquier <michael@paquier.xyz>, Alexander Korotkov <a.korotkov@postgrespro.ru>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2020-10-06T14:29:42Z
Lists: pgsql-hackers 
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> You get the same error with:
> select (float8 '1e+300')::float4;
> float.c:1204:11: runtime error: 1e+300 is outside the range of 
> representable values of type 'float'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior float.c:1204:11 in

> It boils down to casting a C double to float, when the value doesn't fit 
> in float. I'm surprised that's undefined behavior, but I'm no C99 
> lawyer. The code in dtof() expects it to yield Inf.

I think UBSan read C99 6.3.1.5:

       [#2]  When  a double is demoted to float or a long double to
       double or float, if the value being converted is outside the
       range  of  values  that  can be represented, the behavior is
       undefined.

and stopped reading at that point, which they should not have.
If you go on to read the portions around, particularly, <fenv.h>,
you get a different picture of affairs.  If we're relying on IEEE
float semantics in other places, which we are, we're perfectly
entitled to assume that the cast will yield Inf (and a floating
point exception flag, which we ignore).  I think the "undefined"
here is just meant to say that there's no single behavior promised
across all possible C implementations.  They'd have been better to
write "implementation-defined", though.

> I'm inclined to shrug this off and say that the sanitizer is being 
> over-zealous. Is there some compiler flag we should be setting, to tell 
> it that we require specific behavior? Any other ideas?

If UBSan doesn't have a flag to tell it to assume IEEE math,
I'd say that makes it next door to worthless for our purposes.

			regards, tom lane

Commits

  1. Add sortsupport for gist_btree opclasses, for faster index builds.

  2. pageinspect: Fix relcache leak in gist_page_items().

  3. Fix test failure with wal_level=minimal.

  4. Enhance nbtree index tuple deletion.

  5. Fix portability issues in the new gist pageinspect test.

  6. Add functions to 'pageinspect' to inspect GiST indexes.

  7. Fix missing validation for the new GiST sortsupport functions.

  8. Fix compilation warning in xlog.c

  9. Set right-links during sorted GiST index build.

  10. Fix checksum calculation in the new sorting GiST build.

  11. Add support for building GiST index by sorting.