ASDenysPetrov added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1696 + const llvm::APSInt &Idx = CI->getValue(); + const uint64_t I = static_cast<uint64_t>(Idx.getExtValue()); + // Use `getZExtValue` because array extent can not be negative. ---------------- ASDenysPetrov wrote: > martong wrote: > > aaron.ballman wrote: > > > > > This `static_cast` seems to be dangerous to me, it might overflow. Can't we > > compare `Idx` directly to `Extent`? I see that `Idx` is an `APSint` and > > `Extent` is an `APInt`, but I think we should be able to handle the > > comparison on the APInt level b/c that takes care of the signedness. And > > the overflow situation should be handled as well properly with `APInt`, > > given from it's name "arbitrary precision int". In this sense I don't see > > why do we need `I` at all. > We can't get rid of `I` because we use it below anyway in `I >= > InitList->getNumInits()` and `InitList->getInit(I)`. > I couldn't find any appropriate function to compare without additional > checking for signedness or bit-width adjusting. > I'll try to improve this snippet. This is not dangerous because we check for negatives separately in `Idx < 0`, so we can be sure that `I` is positive while `I >= Extent`. Unfortunately, I didn't find any suitable way to compare `APSint` //of unknown sign and bitwidth// with //signless// `APInt` without adding another checks for sign and bitwidth conversions. In this way I prefer the currect condition `I >= Extent`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104285/new/ https://reviews.llvm.org/D104285 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
