Page MenuHomePhabricator

[analyzer] Correctly measure array size in security.insecureAPI.strcpy

Authored by leanil on Mar 1 2018, 12:19 AM.



This will handle those platforms that don't have 8-bit chars.
This is a follow up fix to review D41384, which has been committed since.

Diff Detail

Event Timeline

leanil created this revision.Mar 1 2018, 12:19 AM
leanil updated this revision to Diff 136474.Mar 1 2018, 12:28 AM

getQuantity() returns a signed type

xazax.hun accepted this revision.Mar 1 2018, 5:48 AM


This revision is now accepted and ready to land.Mar 1 2018, 5:48 AM
NoQ added inline comments.Mar 1 2018, 9:20 AM

I suggest not using auto here, because it makes it harder to understand integer promotions (potential overflows or sign extensions) in the comparison.

leanil added inline comments.Mar 2 2018, 6:22 AM

That's true. Should it be CharUnits::QuantityType then?

NoQ added a comment.May 4 2018, 5:51 PM

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.


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 >=.