Page MenuHomePhabricator

[analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions.
ClosedPublic

Authored by NoQ on Oct 30 2017, 9:12 AM.

Details

Summary

An assertion failure was triggered in pr34779 by defining strcpy as void (unsigned char *, unsigned char *), while normally it is char *(char *, char *). The assertion is removed because normally we try not to crash in such cases, but simply refuse to model. For now i did not try to actually model the modified strcpy, because it requires more work (unhardcode CharTy in a lot of places around the checker), and supporting other aspects of the non-standard definition requires even more work (i.e. we try to bind the return value, need to disable this mechanism when the return type is void), and if we try to model other similar functions (are other string functions accepting unsigned chars as well?) it'd bloat quite a bit.

So for now the analyzer would not find C string errors in code that defines string functions in a non-standard manner. It simply tries to avoid crashing. To think - maybe we should add more defensive checks (eg. into CallDescription).

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ created this revision.Oct 30 2017, 9:12 AM
NoQ added inline comments.
test/Analysis/string-with-signedness.c
1 ↗(On Diff #120834)

We do have a warning for this.

xazax.hun accepted this revision.Oct 30 2017, 9:23 AM

This patch looks good to me.
I am wondering whether it would make sense to have some kind of warning (even in the frontend if not in the analyzer) for using standard functions with non-standard signatures, so users start to file bugs for the maintainers of those libraries.

This revision is now accepted and ready to land.Oct 30 2017, 9:23 AM
zaks.anna accepted this revision.Oct 30 2017, 4:00 PM
xazax.hun added inline comments.Oct 31 2017, 3:31 AM
test/Analysis/string-with-signedness.c
1 ↗(On Diff #120834)

Oh, so we already have a warning for this. Good :)

This revision was automatically updated to reflect the committed changes.