> So isn't this an issue with the code before when we have a RANGE_EXPR > that covers the wrapping point?
No, see the reduced testcase attached in the PR. > Then index > max_index and may not catch the found element with > > /* Do we have match? */ > if (wi::cmpu (access_index, index) >= 0 > && wi::cmpu (access_index, max_index) <= 0) > return cval; > > ? We seem to be careful to convert the indices to the index type but > above use unsigned compares - isn't that the underlying issue? Not really, everything is properly unsigned at this point, so the traversal wraps around in an unsigned type. > I'm not sure how "doing from scratch" would fix things - we simply rely > on reliably detecting an element match. > > I wonder how much we can rely on the array domain type to be in sync > with the constructor field entries. At least we need to take into account the wraparound. > Also we're using offset_int here - doesn't that break when with > Ada we have an array type with domain [uint128_t_max - 1, uint128_t_max + 2] > in a type larger than int128_t? Or, since we're interpreting it > inconsistently signed/unsigned, a 128bit integer typed domain wraps > around the sign? Integer types are limited to 64 bits in GNAT. > If that's not an issue then using signed compare unconditionally is > a valid fix as well I guess. But all the types are unsigned here so this doesn't seem appealing. > But in reality 'index' should never really wrap, that is, if the domain > is of 'int' type and starts with INT_MAX then the array shall not have > more than 1 element. No? The range is [-1, 1] converted to sizetype, which is unsigned, so the low bound is UINT32_MAX and the high bound is 1. > The patch you posted isn't a real fix btw, since iff the break was > hit we either missed an index that is present or we found a gap > but then we can break. No, we cannot break if the index wraps around, that's the bug. > So I'd appreciate more explanation on how the index "wraps". See above. I can rewind the entire chain of events if need be, but this is a long one starting a long time ago with the initial blunder with PLUS_EXPR and sizetype. -- Eric Botcazou
