NoQ added a comment.

Sorry, i completely forgot about this one :(

I think this patch needs `lit` tests, eg. tell the analyzer to analyze a simple 
strcpy() call on any `-target` with non-8-bit chars and see if it's still 
crashes or behaves incorrectly.



================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
     if (const auto *Array = dyn_cast<ConstantArrayType>(DeclRef->getType())) {
-      uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+      auto ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
       if (const auto *String = dyn_cast<StringLiteral>(Source)) {
----------------
leanil wrote:
> NoQ wrote:
> > I suggest not using `auto` here, because it makes it harder to understand 
> > integer promotions (potential overflows or sign extensions) in the 
> > comparison.
> That's true. Should it be `CharUnits::QuantityType` then?
I think you should still `getQuantity()`, and then explicitly cast it to a type 
that's compatible with `String->getLength()`, so that there were no implicit 
integer casts around `>=`.


https://reviews.llvm.org/D43928



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to