Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

false positive: garbage value warning #16824

Open Quuxplusone opened 11 years ago

Quuxplusone commented 11 years ago
Bugzilla Link PR16825
Status NEW
Importance P normal
Reported by Michael Hecht (michael.hecht@jmp.com)
Reported on 2013-08-07 12:17:32 -0700
Last modified on 2013-08-30 19:06:57 -0700
Version unspecified
Hardware Macintosh MacOS X
CC jrose@belkadan.com, llvm-bugs@lists.llvm.org, michael.morrell@intel.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Code:

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

#include <iostream>
#include <cassert>
#include <cmath>

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

static void zFreqCert(
        int nr,int nc,
        int rowStride, int colStride,
        double const *tab,
        double const*rowtot,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 = <some value>;
...
if(pi > 0)
   <use lpi>

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

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

Quuxplusone 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?
Quuxplusone 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.
Quuxplusone 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).
Quuxplusone 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.