[cfe-dev] clang static analyzer fails to find bug it found previously
Anna Zaks
ganna at apple.com
Wed Apr 3 13:33:02 PDT 2013
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.
On Apr 3, 2013, at 11:25 AM, Dennis Cote <DennisC at harding.ca> wrote:
> 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
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130403/cf217262/attachment.html>
More information about the cfe-dev
mailing list