| From: | Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com> |
|---|---|
| To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
| Cc: | malis(at)pgrust(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19593: area(circle) silently returns Infinity instead of raising "value out of range: overflow" |
| Date: | 2026-08-04 17:10:54 |
| Message-ID: | CAB8bMit8sqxCOffgruEzHfdDPB7HEaaw2dXB4WntwrWPQp=aMg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
> Attached is one patch that applies to REL_14_STABLE through REL_18_STABLE.
Please use v2.
I in v1 copied the float_*_error_ext(struct Node *escontext) signature from
master (45cdaf366), where escontext is used for soft-error reporting via
ereturn. On REL_14 through REL_18 that path does not exist, so the
argument was unused. That was needless API mirroring.
v2 drops escontext. The helpers are plain float8-returning wrappers
around the existing noreturn float_*_error() calls. The return 0.0 is
never reached. It is only there so the compiler treats the call as an
ordinary returning call. That CFG change is what keeps gcc 13+ jump
threading from deleting the outer isinf() check in float8_mul().
A short comment in float.c spells that out.
вт, 4 авг. 2026 г. в 21:38, Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com>:
> Hi, David!
>
> You asked which part of 45cdaf366 stopped the miscompile on master.
>
> The relevant change is the float8_mul error path. Calling the noreturn
> float_overflow_error() is enough for gcc 13+ jump threading to drop the
> outer isinf() check in circle_ar() after proving both operands finite.
> Returning through a non-noreturn helper (float_overflow_error_ext) keeps
> that check alive. The geo_ops soft-error churn from 45cdaf366 is not
> needed.
>
> Attached is one patch that applies to REL_14_STABLE through REL_18_STABLE.
> It backports that float8_mul subset and adds a geometry regress for:
>
> SELECT area(circle '<(0,0),1e154>');
>
> Verified with gcc 15: unpatched REL_14 returns Infinity, patched raises
> "value out of range: overflow". Same for REL_18.
>
> REL_19 and master already have the helpers via 45cdaf366. A regress-only
> follow-up for those can be sent separately if wanted.
>
>
> вт, 4 авг. 2026 г. в 16:42, David Rowley <dgrowleyml(at)gmail(dot)com>:
>
>> On Tue, 4 Aug 2026 at 22:41, Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com>
>> wrote:
>> >
>> > I researched related past bugs and found this is already fixed in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126464 (reverse Inf
>> handling in float_widen_lhs_range / range-op-float.cc).
>> >
>> > Jakub Jelinek says: Fixed also for 15.4+, as well as backported to 14.5
>> and 13.5.
>>
>> Thanks for doing that work. I see that master isn't affected by this
>> particular issue. The changes made in 45cdaf366 must have shuffled the
>> code around enough that the bug isn't getting triggered.
>>
>> As for what to do in the meantime... I can't think of anything that's
>> not painful in some way or another.
>>
>> A few options which might be worth at least writing down:
>>
>> 1. Add a config precheck using the code you posted to the GCC bugzilla
>> as a configure test and if the bug appears, add -fno-thread-jumps to
>> CFLAGS.
>> 2. Add a volatile qualifier to the result variable in float_mul().
>> 3. Add a regression test for "SELECT area(circle '<(0,0),1e154>');"
>> and leave a comment saying the compiler is broken.
>>
>> All of these seem quite terrible...
>>
>> #1 ends up reducing pgbench -S TPS by half. (tps = 1058074 down to tps
>> = 542227 with -c 100 -j 100).
>> #2 would fix this one instance with probably minimal performance loss,
>> but there are quite a few other similar checks that would all need to
>> be edited. Also, at what point would we ever remove these?
>> Effectively, by removing them, that risks reintroducing the bug(s).
>> #3 is very likely not an option at the moment as the buildfarm would
>> hate it, but it might be an option at some point in the future, once
>> some time has gone by.
>>
>> We could perhaps do #2 then remove it and replace with #3 in some
>> number of months or years.
>>
>> Another thing that might be worth looking into is exactly which part
>> of 45cdaf366 resulted in this inadvertently getting fixed. Maybe
>> there's a realistic subset of that we can do to change the code enough
>> to not trigger the bug.
>>
>> David
>>
>
>
> --
> Regards,
> Rachitskiy Andrey
>
--
Regards,
Rachitskiy Andrey
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Keep-float8-mul-overflow-checks-alive-under-gcc-13-UNIVERSAL.patch | text/x-patch | 3.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matheus Alcantara | 2026-08-04 19:51:35 | Re: BUG #19588: Semantically equivalent DISTINCT ON query returns different result when wrapped in MATERIALIZED CTE. |
| Previous Message | Andrey Rachitskiy | 2026-08-04 16:38:49 | Re: BUG #19593: area(circle) silently returns Infinity instead of raising "value out of range: overflow" |