BUG: fix undefined behavior flagged by UBSAN - #65831

This doesn't fix anything - it just suppresses what UBSAN is warning about. Can we properly raise when bounds are exceeded?

Can we properly raise when bounds are exceeded?

I'll be honest with you, until now I don't understand what would be the correct way to manage the overflow in this module, so I asked gemini to look at the call stack to decide on how it should be handled.

Here is its raw message on why not raise an error directly or assign an invalid value:

The choice to use wrapping arithmetic rather than immediate errors or NaT assignments is driven by the existing architecture of the pandas timestamp library and the
  requirement to maintain specific error behaviors.

  1. Preserving the "Wrap-and-Catch" Pattern
  The current implementation of tz_localize and tz_convert follows a specific pattern:
   1. Localize: Perform the arithmetic to add/subtract the timezone offset.
   2. Validate: Call check_overflows (in conversion.pyx) on the result.

  If the arithmetic overflows and wraps around (e.g., a date in 1677 wraps to 2262), the check_overflows function detects that the resulting year is physically impossible for
  that timestamp and raises a highly specific OutOfBoundsDatetime error with a formatted message (e.g., "Converting 1677-09-21 00:12:43 underflows past 1677-09-21
  00:12:43.145224193").

  2. Why not immediate errors?
  Raising the error immediately inside _get_utc_bounds would require:
   - Duplicating complex error-formatting logic into a low-level Cython function.
   - Ensuring the error message perfectly matches what users and existing tests expect.
   - Changing a design pattern that is used consistently across the tslibs module.

  3. Why not NaT?
  Assigning NaT (or NPY_NAT) would actually be a behavior change:
   - Expectation: Localization that pushes a value out of bounds is required to raise OutOfBoundsDatetime in pandas. Returning NaT would silently swallow the error.
   - Ambiguity: NaT is also used for "non-existent" times (like the hour skipped during DST). If we returned NaT for overflows, we would lose the ability to distinguish
     between a "missing hour" and a "value out of bounds."

  4. Satisfying UBSAN
  Signed integer overflow is Undefined Behavior (UB) in C. Even if it "works" on most CPUs by wrapping, compilers are allowed to optimize based on the assumption that it
  never happens, which can lead to subtle bugs.

  By using unsigned casts for wrapping, we transform Undefined Behavior into Defined Behavior. The result is identical at the bit level, but it tells the compiler (and UBSAN)
  that the wrap-around is intentional. This fixes the sanitizer warning while keeping the library's logic intact and performant.

Cython has an overflowcheck directive you can add as a wrapper that should do the right thing and raise.

In older versions of Cython you could argue this was "slow" but I think in the past few years they implemented it using the compiler built-in's, so its negligible overhead

AFAIU, we shouldn't be raising in this function. For example, in this test:

It undeflows in _get_utc_bounds on val - ppd when handling the left side

but computes correct results for the right side, and seems to rely on it, but I need to give it a better look.

I think that wrapping is unintentional and should be changed, but raising an error seems the wrong way to go.

Sounds good. Open to refactors; ideally we never have UB as it is not something we can "rely on"

Not sure if it's correct, but with the last modification I changed the behavior on overflow to the following:

• For the val ± ppd, I assign isl or isr to one of the extremas.
• for the val ± delta, I assign an status code indicating overflow/underflow. If it happens on both sides, I raise an error. If at least one of the sides remain valid, then the other side will be NPY_NAT and the valid side will be used here:

I think with cdef void you pay a performance penalty because you have to check the global Python error indicator. For high performance code I'd stick to the convention of returning an int where a non-zero value represents an error

I don't think that performance matters here. This is only called when an overflow happens on both sides and the goal is to raise a Python exception.

In the case of UNDERFLOW / OVERFLOW should we just return early?

The other side can be valid, so it's not correct to return early.

Semantics nit but I guess BE_OK isn't an error

Do we have to hard-code all of these conditions or can we not just have the built-in overflow detection? The latter would definitely be easier to maintain and in many cases is more performant than hand-rolled checks

I found a little bit convoluted and misleading to use function-like macros in Cython. I am not sure if they provide a built-in check that won't cause a Python exception.

Hmm yea that's kind of challenging to express in Cython. Any chance this macro syntax to accept variadic arguments helps? Seems like we could at the very least call the generic macros we already have in portable.h (?)

cython/cython#3264 (comment)

Not sure I understand this change - this seems to just mask an issue via a cast (?)

Wrapping on overflow is intentional in the groupby.

What UB is this avoiding? This seems like it should be a separate change / performance boost to elide a cast to object (?)

misaligned load

pandas/_libs/lib.cpython-314-x86_64-linux-gnu.so.p/pandas/_libs/lib.pyx.c:30057:30: runtime error: load of misaligned address 0x7c513cf51815 for type 'PyObject *' (aka 'struct _object *'), which requires 8 byte alignment
0x7c513cf51815: note: pointer points here
 00 00 00 00 10 a1 13  3d 71 7b 00 00 00 d8 d2  05 bf d8 d2 05 bf 01 00  00 00 02 00 00 00 00 00  00