[PATCH] intXshr, intXshl: return error on shift count out of range

Hi, Egor!

On Wed, 30 Sept 2026 at 18:03, Egor Ivkov <e.ivkov@arenadata.io> wrote:

Small fix for .out files to contain rightmost spaces.

Regards,
Egor Ivkov

Your patch looks good to me except the similar behavior of int2 and
int4 are never explained. In C11 and older int16 left arg bit shift
is not UB only if the result filts int32. There is no check for this
in the patch.

Regards,
Pavel Borisov
Supabase

Hello Egor and Pavel,

30.09.2026 17:28, Pavel Borisov wrote:

Small fix for .out files to contain rightmost spaces.

Regards,
Egor Ivkov

Your patch looks good to me except the similar behavior of int2 and
int4 are never explained. In C11 and older int16 left arg bit shift
is not UB only if the result filts int32. There is no check for this
in the patch.

FWIW, this was already reported several times:
[1]: /messages/by-id/48099055.9040607@dls.net
[2]: /messages/by-id/16790-c2a81c92a8776c5b@postgresql.org
[3]: /messages/by-id/17167-028026e4ca333817@postgresql.org
[4]: /messages/by-id/19672-33e7d9d1be2229ea@postgresql.org

By the way, having a RISC-V machine handy:
Linux orangepirv2 6.6.63-ky #1.0.0 SMP PREEMPT Wed Mar 12 09:04:00 CST 2025 riscv64 riscv64 riscv64 GNU/Linux
I've tried:
SELECT int4shl(1, 100);
 int4shl
---------
      16
(1 row)

Best regards,
Alexander

Alexander Lakhin <exclusion@gmail.com> writes:

By the way, having a RISC-V machine handy:
Linux orangepirv2 6.6.63-ky #1.0.0 SMP PREEMPT Wed Mar 12 09:04:00 CST 2025 riscv64 riscv64 riscv64 GNU/Linux
I've tried:
SELECT int4shl(1, 100);
 int4shl
---------
      16
(1 row)

FWIW, I don't agree with the premise of this patch, even a little bit.
To my mind, the purpose of these functions and their siblings is to
provide access to the C-level bitwise operators, which will do
whatever they do on your platform. There is no contract to restrict
them to some guaranteed-portable functionality subset, and I think
trying to do that would accomplish little except to break code that
had been working fine in the context it's used in.

We have generally taken a similar approach with respect to other
things that are platform-dependent, such as floating-point math.
FP math is more portable than it used to be thanks to IEEE 754's
achievement of world domination; but there are still discrepancies,
and we don't typically try to hide them.

There might be room for a documentation patch that adds something
like

-Bitwise shift left
+Bitwise shift left (defined to act like the C << operator)

just to clarify our intent.

regards, tom lane

On Wed, 30 Sept 2026 at 10:47, Egor Ivkov <e.ivkov@arenadata.io> wrote:

x86 handled this by applying a mask (e.g. & 31) while ARM will produce 0 and RISC-V will crash. So besides being an UB it is inconsistent across platforms.

Can you share more details about this claimed crash behaviour for
RISC-V? Going by [1]https://docs.riscv.org/reference/isa/v20260120/unpriv/rv64.html, I see:

"SLL, SRL, and SRA perform logical left, logical right, and arithmetic
right shifts on the value in register rs1 by the shift amount held in
register rs2. In RV64I, only the low 6 bits of rs2 are considered for
the shift amount."

I don't have access to RISC-V hardware, so I tried [2]https://godbolt.org/z/3MfTze6rj to see what the
compiler emits with constant inputs. I see it optimises the shift into
a constant-zero when asked to shift left by more than the type's
width. I'm unsure if that's a misoptimisation or not as it's not using
the lower 6 bits rule that I interpret from the standard.

Have you actually tested this on RISC-V? Can you share the results of:

SELECT 1::bigint << 255, 1::bigint << 63;

Does it actually crash?

David

[1]: https://docs.riscv.org/reference/isa/v20260120/unpriv/rv64.html
[2]: https://godbolt.org/z/3MfTze6rj

<div><p> </p><blockquote> By the way, having a RISC-V machine handy:<br /> Linux orangepirv2 6.6.63-ky #1.0.0 SMP PREEMPT Wed Mar 12 09:04:00 CST 2025 riscv64 riscv64 riscv64 GNU/Linux<br /> I've tried:<br /> SELECT int4shl(1, 100);<br />   int4shl<br /> ---------<br />        16<br /> (1 row)</blockquote></div><div> </div><div>Yes, my mistake, I rechecked the manual this instruction should not cause a trap.</div><div> </div><div><blockquote><div>FWIW, I don't agree with the premise of this patch, even a little bit.<br />To my mind, the purpose of these functions and their siblings is to<br />provide access to the C-level bitwise operators, which will do<br />whatever they do on your platform. There is no contract to restrict<br />them to some guaranteed-portable functionality subset, and I think<br />trying to do that would accomplish little except to break code that<br />had been working fine in the context it's used in.</div></blockquote></div><div> </div><div>Ok, I understand that compatible behavior is not a priority in this case.</div><div><br />Still an UB can be optimized and exploited by compiler in any way it decides,</div><div>so we don't have a guarantee that current platform behavior will stay.</div><div>What is a general postgres community policy about UBs?</div><div> </div><div>Regards,<br />Egor Ivkov</div>

On Thu, 1 Oct 2026 at 08:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:

FWIW, I don't agree with the premise of this patch, even a little bit.

I'm inclined to agree here. Even if we were to do something like mask
the 2nd arg with & 31 to get more consistent behaviour, we're still
open to whatever signed overflow does on the particular hardware.

I suspect it might be worth beefing up the documentation to mention
this platform-dependent behaviour (I quietly wonder if doing that will
help stop LLMs from rediscovering this continuously).

David

David Rowley <dgrowleyml@gmail.com> writes:

Have you actually tested this on RISC-V? Can you share the results of:
SELECT 1::bigint << 255, 1::bigint << 63;
Does it actually crash?

I tried this on the gcc compile farm's cfarm95 (said to be
a Banana Pi BPI-F3 with Spacemit(R) X60 chip; runs Debian trixie).
I see no crash and results consistent with the low-six-bits rule:

regression=# SELECT 1::bigint << 255, 1::bigint << 63;
?column? | ?column?
----------------------+----------------------
-9223372036854775808 | -9223372036854775808
(1 row)

regression=# SELECT 1::bigint << 64, 1::bigint << (-1);
?column? | ?column?
----------+----------------------
1 | -9223372036854775808
(1 row)

I rather imagine you can make it crash if you use ASAN configured to
dump core on whatever behavior it considers undefined ... but I
reject the notion that we should let ASAN dictate our choices here.

regards, tom lane

David Rowley <dgrowleyml@gmail.com> writes:

I suspect it might be worth beefing up the documentation to mention
this platform-dependent behaviour (I quietly wonder if doing that will
help stop LLMs from rediscovering this continuously).

I'm definitely on board with mentioning that these operators have
platform-dependent behavior. I doubt we should try to enumerate
any details.

regards, tom lane

On Thu, 1 Oct 2026 at 11:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:

David Rowley <dgrowleyml@gmail.com> writes:

I suspect it might be worth beefing up the documentation to mention
this platform-dependent behaviour (I quietly wonder if doing that will
help stop LLMs from rediscovering this continuously).

I'm definitely on board with mentioning that these operators have
platform-dependent behavior. I doubt we should try to enumerate
any details.

Here's an attempt at that.

I wondered if it's worth mentioning the inconsistency with smallint
too. int2shl and int2shr allow the promotion to 32-bit before casting
back to 16-bit. That might surprise a few people. Consider:

select 1::smallint << 16, 1::int << 32, 1::bigint << 64;;
?column? | ?column? | ?column?
----------+----------+----------
0 | 1 | 1
(1 row)

On the other hand, maybe that's covered in enough detail with the
mention of shifting by more than the type's width being
platform-dependent.

David

<div>+ i.khaprov@, pgsql-hackers@</div><div> </div><div> </div><div>David, thanks for fixing it in the docs!<br /><br />I've marked the original patch in commitfest as rejected - <a href="https://commitfest.postgresql.org/patch/7367&quot; rel="noopener noreferrer" target="_blank">https://commitfest.postgresql.org/patch/7367&lt;/a&gt;&lt;/div&gt;&lt;div&gt; &lt;/div&gt;&lt;div&gt;The rejection reason seems to be that we shouldn't change working behavior, especially when functions are intended as thin wrappers.</div><div> </div><div>Regards,<br />Egor Ivkov</div>

On Thu, 1 Oct 2026 at 13:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:

David Rowley <dgrowleyml@gmail.com> writes:

On Thu, 1 Oct 2026 at 11:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm definitely on board with mentioning that these operators have
platform-dependent behavior. I doubt we should try to enumerate
any details.

Here's an attempt at that.

Perhaps s/in terms of/in cases of/, or similar wording.
Otherwise I'm okay with it.

OK, done that way. Thanks.

David

Hi!

While I think that OP's patch is close to being correct, I also agree
with Tom and Alexander that the scope of this UB is outside of
Postgres code. So I fully agree with the final approach. Thanks to
everyone in this thread!

Regards,
Pavel Borisov