[PATCH] D16317: [Analyzer] Fix for PR23790: bind real value returned from strcmp when modelling strcmp.

[Devin Coughlin via cfe-commits]

dcoughlin added a subscriber: dcoughlin.
dcoughlin added a comment.

As Artem notes, you can't defer to the host strcmp() -- doing so is just as unsound as using StringRef::compare() less predictable under optimization of the analyzer. I think his suggested approach is the way to go: create a symbol and constrain its range based on the result of StringRef::compare.

For tests, I would suggest:

  int lessThanZero = strcmp("aaa", "nnn");
  clang_analyzer_eval(lessThanZero < 0); // expected-warning {{TRUE}}
  clang_analyzer_eval(lessThanZero >= 0); // expected-warning {{FALSE}}
  clang_analyzer_eval(lessThanZero < -13); // expected-warning {{UNKNOWN}}
  
  int greaterThanZero = strcmp("nnn", "aaa");
  clang_analyzer_eval(greaterThanZero > 0); // expected-warning {{TRUE}}
  clang_analyzer_eval(greaterThanZero <= 0); // expected-warning {{FALSE}}
  clang_analyzer_eval(greaterThanZero > 13); // expected-warning {{UNKNOWN}}
  
  int equalToZero = strcmp("aaa", "aaa");
  clang_analyzer_eval(equalToZero == 0); // expected-warning {{TRUE}}

These show that the analyzer does assume the strongest sound postcondition and spot checks that it doesn't assume either the stronger, StringRef-implementation-specific invariant (1/-1) or an invariant from a common unoptimized memcpy() implementation ('a' - 'n' is 13).


================
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1873
@@ -1872,3 +1872,3 @@
         // Compare string 1 to string 2 the same way strcasecmp() does.
         result = s1StrRef.compare_lower(s2StrRef);
       } else {
----------------
Whatever changes you make for the case-sensitive compare should also be analogously applied to the case-insensitive compare.


http://reviews.llvm.org/D16317