false positive with lack of ==/!= or range checking between symbolic pointer values

[Quuxplusone]

Bugzilla Link	PR2695
Status	CONFIRMED
Importance	P normal
Reported by	Török Edwin (edwin+bugs@etorok.eu)
Reported on	2008-08-20 02:13:05 -0700
Last modified on	2017-08-13 03:06:13 -0700
Version	unspecified
Hardware	PC Linux
CC	florian_hahn@apple.com, kremenek@apple.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments	func2.bc (49368 bytes, application/octet-stream) func.bc (49332 bytes, application/octet-stream)
Blocks
Blocked by
See also

Using SVN r55039:
$ ~/llvm-svn/llvm/tools/clang/utils/ccc-analyzer -c t2.c
ANALYZE: t2.c foo
t2.c:9:11: warning: Dereference of undefined value.
                        return *max;
                               ^~~~
1 diagnostic generated.

The code is below, see attached report for execution path.
It says the while  is taken, so s1 < end and s2 < end2. Then it says that the
"if" is taken immediately on the first loop, which is impossible.
In reality the "if" is entered soonest at second loop, and max is initialized
by that time, so there is no deref of undef. value.

#include <string.h>
char foo(char *s1, char* s2)
{
        char *max;
        char *end1 = s1 +strlen(s1);
        char *end2 = s2 +strlen(s2);
        while(s1 < end1 && s2 < end2) {
                if(s1 == end1 || s2 == end2) {
                        return *max;
                }
                s1++;
                s2++;
                max=s1;
        }
        return 0;
}

[Quuxplusone]

This FP manifests because we are not (yet) tracking constraints between
symbolic pointer values.  This FP can be suppressed simply by recording a !=
relationship between s1 and end1 (and s2 and end2) because of s1 < end1.

More generally, after the call to strlen(s1), we should have the constraints:

  s1 + strlen(s1) < MAX_POINTER_VALUE
  strlen(s1) >= 0

This is because we cannot have an integer overflow here because strlen would
segfault.  This means that end1 >= s1 (but it would not be possible that end1 <
s1).

[Quuxplusone]

instcombine does this transform, which seems legal to me:
IC: ConstFold to: i8 undef from:        %tmp194 = trunc i64 undef to i8
; <i8> [#uses=1]
IC: ConstFold to: i1 undef from:        %toBool17 = icmp eq i8 undef, 0
; <i1> [#uses=1]

I guess it is not an instcombine bug, but instcombine exposes a bug introduced
by an earlier transform.

[Quuxplusone]

Attached func2.bc (49368 bytes, application/octet-stream): original function as compiled by -O4

[Quuxplusone]

Attached func.bc (49332 bytes, application/octet-stream): as func2.bc, but after running some optimizations on it by bugpoint

[Quuxplusone]

(In reply to comment #4)
> instcombine does this transform, which seems legal to me:
> IC: ConstFold to: i8 undef from:        %tmp194 = trunc i64 undef to i8
> ; <i8> [#uses=1]
> IC: ConstFold to: i1 undef from:        %toBool17 = icmp eq i8 undef, 0
> ; <i1> [#uses=1]
>
> I guess it is not an instcombine bug, but instcombine exposes a bug introduced
> by an earlier transform.
>

Sorry, these were meant for PR4313.

[Quuxplusone]

that's still in a issue. ccc-analyzer has moved to clang/tools/scan-build/libexec/ccc-analyzer