ebevhan added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:14063
+ if (isUnboundedArray) {
+ if (index.isUnsigned() || !index.isNegative()) {
+ const auto &ASTC = getASTContext();
----------------
This could be early return to avoid the indentation.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13981
+ bool overflow;
+ llvm::APInt product(index);
+ product += 1;
----------------
chrish_ericsson_atx wrote:
> ebevhan wrote:
> > What if index is wider than AddrBits, but the active bits are fewer? Then
> > you might miss out on triggering the overflow case in the multiplication.
> Line 13984 checks for active bits of product being less than AddrBits, which
> is the same case (since product, by definition, has same width as index). So
> I think this is covered. If I've misunderstood, please re-ping.
The overflow limit for _ovf is determined by the width of the APInt. If index
is 32 bits wide but only has 14 bits active, and AddrBits is 16, then an
umul_ovf might overflow past 16 bits but not for 32 bits since the product is
the same width as the index. Then we won't detect the overflow.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13993
+ MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+ MaxElems += 1;
+ if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())
----------------
chrish_ericsson_atx wrote:
> ebevhan wrote:
> > Though, why is the +1 here? Isn't this already the maximum number of
> > elements?
> Initial value of MaxElems is APInt::getMaxValue(AddrBits), which is the index
> of the last addressable CharUnit in the address space. Adding 1 makes it the
> total number of addressable CharUnits in the address space, which is what we
> want as the numerator for computing total number of elements of a given size
> that will fit in that address space.
>
Ah, yes. Got indexes and sizes mixed up again.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86796/new/
https://reviews.llvm.org/D86796
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
