Fix empty layout span sizes with overflowing extent prefixes

[vera-rubin]

With 32-bit int indices, extents (65536, 65536, 0) abort during padded mapping construction with default precondition checks. Both unpadded layouts also reject required_span_size() in constant evaluation because an intermediate product overflows, although the constructor precondition concerns the complete size, which is zero, and the required span is zero.

using E = Kokkos::dextents<int, 3>;
constexpr Kokkos::layout_right::mapping<E> m(E(65536, 65536, 0));
static_assert(m.required_span_size() == 0);

We check for zero factors before multiplying in the padded representability checks and all four span observers. The regression covers both orientations and every zero position at compile time and runtime. We tested the full suite with GCC 13.3 in C++23 and the affected layout tests and reproducers in C++17, C++20 and C++23.

— ChipLab

[mhoemmen]

Thank you for your interest in mdspan and for wording to improve the quality of the reference implementation!

I think this is a reasonable reading of the Standard. Here is my analysis.

The padded layouts have specific wording to make required_span_size return 0 if the multidimensional index space is empty (see e.g., [mdspan.layout.leftpad.obs] 2.1.) Wording for the layout_left and layout_right mappings do not; they just invoke the exposition-only function fwd-prod-of-extents (see e.g., [mdspan.layout.left.obs] 1), which returns "the product" of the extents (see [mdspan.extents.expo] 6). English description suggests a mathematical interpretation of product, rather than interpretation as a C++ algorithm (which would overflow, in this case).

The constructor's precondition says that the "size of the multidimensional index space" is representable as a value of type index_type. We define the "size of a multidimensional index space" in [mdspan.overview] 1 as the "product" of what would be the extents in this case. Thus, the product of the extents must be representable as a value of type index_type. We don't say that the product of any subset of the extents must have this property.

[mhoemmen]

This looks reasonable to me. @crtrott and/or @nmm0 may want to comment. Thanks for your contribution!

[crtrott]

Hey Mark, I actually spent a bit of time reading through the standard yesterday night too, and agree with your interpretation. I scrolled through the changes, and I wondered in some places if we can do some perf optimizations.
For example we might not need to do this for unsigned integer types or? Wrap around semantics should end up with zero anyway. So we could protect these things with if constexpr (is_unsigned_v<iType>) or so. In other places I was wondering if we can do it inside the existing loop, but I think we might not because we might already overflow signed integer range before we hit the zero. Just wanted to think a bit more carefully about it.

[crtrott]

Another question: was this problem encountered in a real end-product or was this generated based on some green-field AI agent search of open source software? (I looked at the company website of the authors account). Just trying to understand the urgency of resolving the thing. I do believe it's a valid issue.

[crtrott]

To further evidence of my statement, the above code does compile without the changes in this PR, when using unsigned instead of int as the index_type for the extents. (on my Mac laptop, not tried a debug build).

[vera-rubin]

Thanks for checking the standard too. This came from our AI-assisted checks of libraries against the C++ standard, not from a failure in a shipped product, so there's no urgency on our side.

Agreed on unsigned index types: wraparound already gives zero there, which matches what you saw compiling with unsigned. I can limit the new handling to signed types with if constexpr so the unsigned path stays as it was.