clang static analyzer fails to find bug it found previously

Clang Frontend
1

Hi,

This is my first post to this list. I hope someone here can help.

I have built LLVM and Clang from source (SVN trunk head today) using Mingw/MSYS and CMake in order to use the static analyzer to check some C code.

To test my builds I have been using the source files for AStyle as a test case. I added a unpaired malloc call to astyle_main.cpp to ensure a bug would be found. The static analyzer is functioning and detects that bug (as well as a couple of other “values stored is not read” errors).

In earlier test runs a few days ago the static analyzer found what I believe is a real bug in the AStyle source and gave me a nice memory is never released report with HTML etc which I printed. Since then I have rebuilt LLVM and Clang several times (while changing build tools etc.). Now I no longer get the same bug report for that section of code.

The code is question is fairly short so I have copied it below. It does two allocations, but doesn’t free the first allocation if the second fails. I believe the delete outBuf that I have commented out below is required for correct operation without a memory leak.

string ASConsole::getNumberFormat(int num, size_t lcid) const

{

// Compilers that don’t support C++ locales should still support this assert.

// The C locale should be set but not the C++.

// This function is not necessary if the C++ locale is set.

assert(locale().name() == “C”);

// convert num to a string

stringstream alphaNum;

alphaNum << num;

string number = alphaNum.str();

if (useAscii)

return number;

// format the number using the Windows API

if (lcid == 0)

lcid = LOCALE_USER_DEFAULT;

int outSize = ::GetNumberFormat(lcid, 0, number.c_str(), NULL, NULL, 0);

char* outBuf = new(nothrow) char[outSize];

if (outBuf == NULL)

return number;

::GetNumberFormat(lcid, 0, number.c_str(), NULL, outBuf, outSize);

// remove the decimal

string formattedNum(outBuf);

int decSize = ::GetLocaleInfo(lcid, LOCALE_SDECIMAL, NULL, 0);

char* decBuf = new(nothrow) char[decSize];

if (decBuf == NULL) {

// delete outBuf;

return number;

}

::GetLocaleInfo(lcid, LOCALE_SDECIMAL, decBuf, decSize);

size_t i = formattedNum.rfind(decBuf);

if (i != string::npos)

formattedNum.erase(i);

if (!formattedNum.length())

formattedNum = “0”;

delete outBuf;

delete decBuf;

return formattedNum;

}

One possibility is that the code is not compiled at all during the static analyzer build. This function inside a #ifdef _WIN32/#endif pair. I don’t know why the definition would have changed, but I suspect it has.

To check I preprocessed the file and I can see that _WIN32 is defined, at least by g++.

$ g++ -E -dM …/…/src/astyle_main.cpp | grep -i “_win32”

#define VER_PLATFORM_WIN32_NT 2

#define FACILITY_WIN32 7

#define _WIN32_WINNT WINVER

#define LHashValOfName(l,n) LHashValOfNameSys(SYS_WIN32,l,n)

#define __GTHREAD_HIDE_WIN32API 1

#define SERVICE_WIN32_OWN_PROCESS 16

#define WIN32 1

#define _WIN32 1

#define SERVICE_WIN32_SHARE_PROCESS 32

#define WIN32 1

#define SERVICE_WIN32 (SERVICE_WIN32_OWN_PROCESS|SERVICE_WIN32_SHARE_PROCESS)

#define __WIN32 1

#define RPC_WIN32

#define VER_PLATFORM_WIN32_WINDOWS 1

#define SERVICE_TYPE_ALL (SERVICE_WIN32|SERVICE_ADAPTER|SERVICE_DRIVER|SERVICE_INTERACTIVE_PROCESS)

#define HRESULT_FROM_WIN32(x) (x?((HRESULT)(((x)&0x0000FFFF)|(FACILITY_WIN32<<16)|0x80000000)):0)

#define GCC_GTHR_WIN32_H

#define VER_PLATFORM_WIN32s 0

So I repeated with clang++ in case the wrong compiler is being used for the build, but it defines _WIN32 as well

$ clang++ -E -dM …/…/src/astyle_main.cpp | grep -i “win32”

#define FACILITY_WIN32 7

#define GCC_GTHR_WIN32_H

#define HRESULT_FROM_WIN32(x) (x?((HRESULT)(((x)&0x0000FFFF)|(FACILITY_WIN32<<16)|0x80000000)):0)

#define LHashValOfName(l,n) LHashValOfNameSys(SYS_WIN32,l,n)

#define SERVICE_TYPE_ALL (SERVICE_WIN32|SERVICE_ADAPTER|SERVICE_DRIVER|SERVICE_INTERACTIVE_PROCESS)

#define SERVICE_WIN32 (SERVICE_WIN32_OWN_PROCESS|SERVICE_WIN32_SHARE_PROCESS)

#define SERVICE_WIN32_OWN_PROCESS 16

#define SERVICE_WIN32_SHARE_PROCESS 32

#define VER_PLATFORM_WIN32_NT 2

#define VER_PLATFORM_WIN32_WINDOWS 1

#define VER_PLATFORM_WIN32s 0

#define WIN32 1

#define _WIN32 1

#define _WIN32_WINNT WINVER

#define __GTHREAD_HIDE_WIN32API 1

#define RPC_WIN32

#define __WIN32 1

#define WIN32 1

So my question is why has the static analyzer stopped detecting the memory leak bug it found previously? Does anyone have any ideas or suggestions?

Thanks.

Dennis Cote

2

Dennis,

The reported issue had disappeared because we had just disabled the cplusplus.NewDelete checker (in r178529), which Anton worked on. We felt it was not ready for its prime time yet. That checker is in active development now. You can enable it manually though scan-build, but it's alpha for now. It's great to hear that it found the bug!

Anna.

3

Hi Anna,

Thanks for the confirmation. I saw that the cplusplus.NewDelete checker was shown as alpha in the scan-build help, but I didn’t think it would have been released and then made alpha later. I though one of the other checker must have detected the bug.

So my next question is how do I manually enable it? Since I am using Cmake, and it hard coded the compiler paths, I have had to set the compilers to ccc-analyzer and c+±analyzer.

The “–enable-checker alpha.cplusplus.NewDelete” option can be passed to scan-build, but it does not seem to work. I get the same error reports with and without that option.

I tried passing the option as a CFLAG to c+±analyzer but that fails with a “no such file” error.

Any other ideas?

Dennis Cote

The reported issue had disappeared because we had just disabled the cplusplus.NewDelete checker (in r178529), which Anton worked on. We felt it was not ready for its prime time yet. That checker is in active development now. You can enable it manually though scan-build, but it’s alpha for now.

4

I've just realized that the checker's package changed after the commit, so it's now called alpha.cplusplus.NewDelete.

Anna.