[PATCH] D38499: [BasicAA] Fix adjustToPointerSize in BasicAliasAnalysis.cpp for ptr > 64b

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D38499#887466, @hfinkel wrote:

> In https://reviews.llvm.org/D38499#887306, @efriedma wrote:
>
> > Hang on, there's a more fundamental problem here this is papering over.  If your pointers are larger than 64 bits, those pointers can have offsets larger than 64 bits.  Since BasicAA is using 64-bit integers to represent pointer offsets, the math in DecomposeGEPExpression will overflow, so you'll get incorrect results, and eventually cause a miscompile.
>
>
> Indeed; I made a similar comment in https://reviews.llvm.org/D38501. In this case, it looks like the main potential overflow comes from:
>
>   Index = GetLinearExpression(Index, IndexScale, IndexOffset, ZExtBits,
>                               SExtBits, DL, 0, AC, DT, NSW, NUW);
>   
>   // The GEP index scale ("Scale") scales C1*V+C2, yielding (C1*V+C2)*Scale.
>   // This gives us an aggregate computation of (C1*Scale)*V + C2*Scale.
>   Decomposed.OtherOffset += IndexOffset.getSExtValue() * Scale;
>   Scale *= IndexScale.getSExtValue();
>   
>
> where Scale is multiplied by `IndexScale.getSExtValue();`. We might just bail out here if IndexScale, or IndexOffset, can't be represented in 64 bits.


I'm not sure, however, that I'd hold this patch up over that issue. We'll need to audit all of BasicAA to put in the necessary checks. We should probably do that as a series of small patches, however, (so that we can bisect usefully if we mess something up).


https://reviews.llvm.org/D38499