llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.01k stars 11.56k forks source link

false positive: garbage value warning #17199

Open llvmbot opened 11 years ago

llvmbot commented 11 years ago
Bugzilla Link 16825
Version unspecified
OS MacOS X
Reporter LLVM Bugzilla Contributor
CC @belkadan

Extended Description

Code:

// // main.cpp // sa-test // // Created by Michael Hecht on 8/7/13. // Copyright (c) 2013 Michael Hecht. All rights reserved. //

include

include

include

/---------------- UNCERTAINTY COEFFICIENTS ----------------/

static void zFreqCert( int nr,int nc, int rowStride, int colStride, double const tab, double constrowtot,double const* coltot,double totn, double qAlpha, // normal quantile for (1 - ef->alpha/2) double& ucr, double& e_ucr, double& l_ucr, double& u_ucr, double& urc, double& e_urc, double& l_urc, double& u_urc, double& u, double& e_u, double& l_u, double& u_u) { long i, j; double uij, ui, uj, pi, pj, pij, lpi, lpj, lpij; double uiji, uijj, uijcp, uiss, ujss, uijss;

/*--- PRELIMINARY CALCULATIONS ---*/
ui = uj = uij = uiss = ujss = uijss = uiji = uijj = uijcp = 0.;

for (i=0; i<nr; i++) { pi = rowtot[i]/totn;
    if (pi>0) { lpi=log(pi); ui+=(pi*lpi); uiss+=(pi*lpi*lpi); }
    for (j=0; j<nc; j++) {
        pj = coltot[j]/totn;     pij = tab[i*rowStride+j*colStride]/totn;

        if (pj>0) {
            lpj=log(pj); uj+=(pij*lpj); ujss+=(pij*lpj*lpj);
            if (pi>0) uijcp += (pij*lpi*lpj);
            if (pij>0) {    assert(pi>0 && pj>0);
                lpij=log(pij);
                uij += (pij*lpij);
                uijss += (pij*lpij*lpij);
                uiji += (pij*lpij*lpi);
                uijj += (pij*lpij*lpj);
            }
        }
    }
}

}

int main(int argc, const char * argv[]) {

// insert code here...
std::cout << "Hello, World!\n";
return 0;

}

Analyze target sa-test

Analyze sa-test/main.cpp cd /Users/hecht/Desktop/sa-test setenv LANG en_US.US-ASCII /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -x c++ -arch x86_64 -fmessage-length=0 -std=gnu++11 -stdlib=libc++ -Wno-trigraphs -fpascal-strings -O0 -Wno-missing-field-initializers -Wno-missing-prototypes -Wreturn-type -Wno-non-virtual-dtor -Wno-overloaded-virtual -Wno-exit-time-destructors -Wformat -Wno-missing-braces -Wparentheses -Wswitch -Wno-unused-function -Wno-unused-label -Wno-unused-parameter -Wunused-variable -Wunused-value -Wempty-body -Wuninitialized -Wno-unknown-pragmas -Wno-shadow -Wno-four-char-constants -Wno-conversion -Wconstant-conversion -Wint-conversion -Wenum-conversion -Wshorten-64-to-32 -Wno-newline-eof -Wno-c++11-extensions -DDEBUG=1 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.8 -g -fvisibility-inlines-hidden -Wno-sign-conversion -Xclang -analyzer-output=plist-multi-file -Xclang -analyzer-checker -Xclang security.insecureAPI.UncheckedReturn -Xclang -analyzer-checker -Xclang security.insecureAPI.getpw -Xclang -analyzer-checker -Xclang security.insecureAPI.gets -Xclang -analyzer-checker -Xclang security.insecureAPI.mkstemp -Xclang -analyzer-checker -Xclang security.insecureAPI.mktemp -Xclang -analyzer-disable-checker -Xclang security.insecureAPI.rand -Xclang -analyzer-disable-checker -Xclang security.insecureAPI.strcpy -Xclang -analyzer-checker -Xclang security.insecureAPI.vfork -iquote /Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/sa-test-generated-files.hmap -I/Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/sa-test-own-target-headers.hmap -I/Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/sa-test-all-target-headers.hmap -iquote /Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/sa-test-project-headers.hmap -I/Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Products/Debug/include -I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include -I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include -I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include -I/Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/DerivedSources/x86_64 -I/Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/DerivedSources -F/Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Products/Debug -MMD -MT dependencies -MF /Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/StaticAnalyzer/sa-test/sa-test/normal/x86_64/main.d --analyze /Users/hecht/Desktop/sa-test/sa-test/main.cpp -o /Users/hecht/Library/Developer/Xcode/DerivedData/sa-test-aonsrgxjbwasygdpieoztmczengq/Build/Intermediates/sa-test.build/Debug/sa-test.build/StaticAnalyzer/sa-test/sa-test/normal/x86_64/main.plist

sa-test/sa-test/main.cpp:39:28: warning: The right operand of '*' is a garbage value i 1 warning generated.

The structure of the code is this:

double lpi; ... if(pi > 0) lpi = ; ... if(pi > 0)

Not sure why this is flagged as a garbage value?
belkadan commented 11 years ago

The compiler will do something, but exactly what is pretty much random. Converting a double to a float and then loading from it means "treat this array as if it contains floats instead of doubles". This is completely incorrect code, but unfortunately the compiler won't warn about it if you put the cast in, because it assumes you know what you're doing.

You are basically doing this:

float foo(float data) { char rawData = (char *)data;

double newData[2];
char *rawNewData = (char *)newData;
memcpy(&rawNewData[0], &rawData[0], 8);
memcpy(&rawNewData[8], &rawData[8], 8);

float result;
memcpy(&result, &rawNewData[4], 4);
return result;

}

...which happens to return data[1], because copying 8 bytes from &data[0] is copying data[0] and data[1] over newData[0], and copying ((float *)newData)[1] pulls out the second half of newData[0].

Please do not do this.

llvmbot commented 11 years ago

I'm not sure what you mean. I did leave off a cast -- the last assignment should read "data = (float *)newData", but I'm not sure that will make a difference. The parameter can be treated as an array of floats or an array of doubles legitimately, I believe.

Also, if I try different returns (trying all 4 -- data[0], data[1], data[2], and data[3]), I only see the complaint with data[1] and data[3]. No complaint from data[0] or data[2].

Looking at the compiled code, the compiler is OK with this (it didn't give a warning -- I used -Wall and -Wextra).

belkadan commented 11 years ago

No, that's because you can't mix float and double in any sensible way. The upper 32 bits of a double don't make a sensible float.

llvmbot commented 11 years ago

So, would this be the same reason I get a "Undefined or garbage value returned to caller" warning from this (using checker-275 on a Mac):

float foo(float *data) { double newData[2];

newData[0] = ((double*) data)[0];
newData[1] = ((double*) data)[1];
data = newData;

return data[1];

}

or should I file a new bug?

belkadan commented 11 years ago

The analyzer doesn't model floating-point values at all right now, even for repeated checks. Definitely a limitation.

llvmbot commented 11 years ago

assigned to @tkremenek