router: fix 64bit overflow clamping#126
Conversation
sanpar-qcom
commented
Apr 20, 2026
sanpar-qcom
commented
- add logic to avoid clamp overflow to router
- add regression test for high-range address
| // make sure end address does not exceed size | ||
| auto remap_end = (tranges[idx].remap ? 0 : tranges[idx].base) + tranges[idx].size; | ||
| if(tranges[idx].size && dmi_data.get_end_address() >= remap_end) | ||
| using addr_t = decltype(address); |
There was a problem hiding this comment.
address has a fixed type deaclared in line 294. No need to get the type using decltype()
There was a problem hiding this comment.
I understand that, but my intention here is to make sure we always use the same data type as address, even if someone changes the type of the variable address without knowing this part of code exists.
I admit it is a small possibility (but probably close to 0), but when it happens, all those intermediate variables naturally follow the type rather than two different types.
| auto remap_end = (tranges[idx].remap ? 0 : tranges[idx].base) + tranges[idx].size; | ||
| if(tranges[idx].size && dmi_data.get_end_address() >= remap_end) | ||
| using addr_t = decltype(address); | ||
| auto const remap_base = tranges[idx].remap ? addr_t{0} : static_cast<addr_t>(tranges[idx].base); |
There was a problem hiding this comment.
Why do you have these static_cast here and in the next line? They make reading the code difficult and are IMO not needed
There was a problem hiding this comment.
Same as before - plus another chance where tranges[].base changes its type.
Unless we have a guarantee (i.e., the type of tranges[idx].base is deduced from address), I want to make sure the final type would be the same as address - with logical assumptions that all of these types should be static-cast-able.
Combined with above, it makes me a bit hesitant to use uint64_t here. Although, if you feel strong about this, I'm happy to hardcode uint64_t here.
| auto const remap_base = tranges[idx].remap ? addr_t{0} : static_cast<addr_t>(tranges[idx].base); | ||
| auto const remap_size = static_cast<addr_t>(tranges[idx].size); | ||
| auto const remap_end = remap_base + remap_size; | ||
| auto const remap_end_overflow = remap_size > (std::numeric_limits<addr_t>::max() - remap_base); |
There was a problem hiding this comment.
This can be simplified. In C++, unsigned datatypes have a defined overflow behavior. If remap_end is less than remap base we have an overflow. This coud be simplified to auto const remap_end_overflow = remap_end < remap_base;
With this simplification you could combine it with the previous line auto const remap_end_overflow = (remap_base + remap_size) < remap_base;
2 participants
