In the vast majority of cases, we hand AA precise sizes for MemoryLocations, rather than upper-bounds. BasicAA has a few checks that assume the sizes passed in are precise, which breaks down when we start passing in upper-bounds that might include conditionally-executed UB (which is what AliasSetTracker is doing right now, and which caused PR36228). The most straightforward way to fix this without penalizing AA seems to be passing an extra "I guarantee you this size is precise" bit along.
Passing this bit in a minimally invasive way appears difficult in pathological cases (read: massive sizes), and we're already sort of treating this Size as an Optional<uint64_t>, so I figured I'd wrap this up in its own type to make the properties of MemoryLocation sizes more obvious/hopefully easier to deal with.
As a natural part of this refactoring, PR36228 gets fixed.
If this lands, I will do the following cleanups (...which there's a lot of, but the majority are trivial):
- Replace all uses of MemoryLocation::UnknownSize with LocationSize::unknown() + remove MemoryLocation::UnknownSize
- Replace the various resulting Loc == LocationSize::unknown()s with the shorter, equivalent !Loc.hasValue()
- Migrate users of the implicit LocationSize(uint64_t) ctor to LocationSize::precise or LocationSize::upperBound
- Remove the LocationSize(uint64_t) ctor, which will force callers of AA APIs to reason about/be explicit about the precision of the LocationSize they're passing.
The last two bullets are the bits that I hope will prevent future bugs like this (and catch other existing ones, if any). :)
Any thoughts/comments welcome, as always.