[flang-commits] [flang] [flang] Avoid signed integer overflow in GetNonNegativeExtent (PR #222207)

[Eugene Epshteyn via flang-commits]

eugeneepshteyn wrote:

Implementation details:

`GetNonNegativeExtent()` computes a dimension's extent from constant bounds as `ub - lb + 1` in `ConstantSubscript` (`int64_t`) arithmetic. When the extent does not fit — `integer(1) :: a(0_8:9223372036854775807_8)` has an extent of 2\*\*63 — that computation overflows, which is undefined behavior:

```
flang/lib/Evaluate/shape.cpp:467:39: runtime error: signed integer overflow:
9223372036854775807 + 1 cannot be represented in type 'long'
```

The wrapped result itself is intentional and has to be preserved. Storage sequences that are too large are diagnosed later, while offsets are computed, and that code tells a genuinely empty dimension from one whose extent wrapped around by cross-checking the original bounds (`IsEmptyDimension()` in `Semantics/compute-offsets.cpp`). Returning `std::nullopt` or saturating here would change the extent those diagnostics see.

So this keeps computing the same two's complement result, but with `llvm::SubOverflow()` and `llvm::AddOverflow()`, which are defined for every input. Subtracting first also covers bounds of mixed sign, where `ub - lb` can overflow before the increment — as in the `negative_to_positive_bounds_in_common` case of `Semantics/oversized-storage-sequence.f90`.

Three subroutines in that test reach this line with an extent of 2\*\*63: `zero_lower_bound_in_common`, `negative_to_positive_bounds_in_common`, and `equivalence_oversized_member`. They share the one source line, which is why the sanitizer reports it once.

Behavior is unchanged. Over a matrix of bound pairs spanning the `int64_t` extremes (196 pairs, 53 of which overflow), the new expression produces exactly the value the current code produces by wrapping around, and the sanitizer reports nothing for it. The flang semantics tests pass (1559 tests), including `oversized-storage-sequence.f90` itself, and its diagnostics are byte-identical to before.

One caveat, since I do not have a UBSAN build here: I verified the arithmetic in isolation and ran the Fortran semantics tests with a normal build, but I could not confirm that `Semantics/oversized-storage-sequence.f90` is free of *other* sanitizer findings — the issue notes there may be more than one. @DavidSpickett, would you mind re-running it with this patch?


https://github.com/llvm/llvm-project/pull/222207