False-Positive clang-tidy clang-analyzer-cplusplus.NewDelete with message "Use of memory after it is freed" on deleting pointer with MACRO and MFC

[maldag]

If considering the following code

#define DELETE_POINTER(p)                   \
    do {                                    \
        delete p;                                       \
        p= nullptr;                             \
    } while(0)

This will raise a false positive error iff the delete operator is customized. This issue is solved if not using a loop construct.

#define DELETE_POINTER(p)                   \
    {                                   \
        delete p;                                       \
        p= nullptr;                             \
    }

If p is from this class type it will raise the issue:

class cPointerClass : CObject // MFC Base class
{

};

The corresponding class definition:

// CObject
_AFX_INLINE CObject::CObject()
    { }
_AFX_INLINE CObject::~CObject()
    { }
_AFX_INLINE void CObject::Serialize(CArchive&)
    { /* CObject does not serialize anything by default */ }
_AFX_INLINE void* PASCAL CObject::operator new(size_t, void* p)
    { return p; }
#ifndef _DEBUG
// _DEBUG versions in afxmem.cpp
_AFX_INLINE void PASCAL CObject::operator delete(void* p)
    { ::operator delete(p); }
_AFX_INLINE void PASCAL CObject::operator delete(void* p, void*)
    { ::operator delete(p); }
_AFX_INLINE void* PASCAL CObject::operator new(size_t nSize)
    { return ::operator new(nSize); }
// _DEBUG versions in objcore.cpp
#ifdef _AFXDLL
_AFX_INLINE void CObject::AssertValid() const
    { /* no asserts in release builds */ }
_AFX_INLINE void CObject::Dump(CDumpContext&) const
    { /* no dumping in release builds */ }
#endif //_AFXDLL
#endif //!_DEBUG

I'm considering using a release build so the operator delete will be overriden but will the use the standard delete operation.

[llvmbot]

@llvm/issue-subscribers-clang-static-analyzer

Author: Mario Linker (maldag)

If considering the following code ``` #define DELETE_POINTER(p) \ do { \ delete p; \ p= nullptr; \ } while(0) ``` This will raise a false positive error iff the delete operator is customized. This issue is solved if not using a loop construct. ``` #define DELETE_POINTER(p) \ { \ delete p; \ p= nullptr; \ } ``` If p is from this class type it will raise the issue: ``` class cPointerClass : CObject // MFC Base class { }; ``` The corresponding class definition: ``` // CObject _AFX_INLINE CObject::CObject() { } _AFX_INLINE CObject::~CObject() { } _AFX_INLINE void CObject::Serialize(CArchive&) { /* CObject does not serialize anything by default */ } _AFX_INLINE void* PASCAL CObject::operator new(size_t, void* p) { return p; } #ifndef _DEBUG // _DEBUG versions in afxmem.cpp _AFX_INLINE void PASCAL CObject::operator delete(void* p) { ::operator delete(p); } _AFX_INLINE void PASCAL CObject::operator delete(void* p, void*) { ::operator delete(p); } _AFX_INLINE void* PASCAL CObject::operator new(size_t nSize) { return ::operator new(nSize); } // _DEBUG versions in objcore.cpp #ifdef _AFXDLL _AFX_INLINE void CObject::AssertValid() const { /* no asserts in release builds */ } _AFX_INLINE void CObject::Dump(CDumpContext&) const { /* no dumping in release builds */ } #endif //_AFXDLL #endif //!_DEBUG ``` I'm considering using a release build so the operator delete will be overriden but will the use the standard delete operation.

[steakhal]

I'm sorry but I could not reproduce the issue. Could you please give us a complete reproducer in a single snippet without macros? I think that way we can investigate the issue. Thanks.

[maldag]

I double checked this issue. Using clang-tidy 14 the code will fail with attached error message. If I use clang-tidy 18 it will not fail.

class c_BaseMixIn {
    private:
        unsigned int _data=0;
    public:
        void operator delete(void* p)
        { ::operator delete(p); }
        void operator delete(void* p, void*)
        { ::operator delete(p); }
        void* operator new(size_t nSize)
        { return ::operator new(nSize); }
};

class c_BaseMixInNoCustomNewDelete {
    private:
        unsigned int _data=0;
};

/**
 * Does not work with clang-tidy 14.
 * Does work with clang-tidy 18.
 * 
 * /project/test/selftest/test.cpp:36:9: warning: Potential leak of memory pointed to by 'pmyVariable' [clang-analyzer-cplusplus.NewDeleteLeaks]
        }while(0);
               ^
    /project/test/selftest/test.cpp:32:32: note: Calling 'c_BaseMixIn::operator new'
        c_BaseMixIn* pmyVariable = new c_BaseMixIn();
                                ^~~~~~~~~~~~~~~~~
    /project/test/selftest/test.cpp:22:18: note: Memory is allocated
            { return ::operator new(nSize); }
                    ^~~~~~~~~~~~~~~~~~~~~
    /project/test/selftest/test.cpp:32:32: note: Returning from 'c_BaseMixIn::operator new'
        c_BaseMixIn* pmyVariable = new c_BaseMixIn();
                                ^~~~~~~~~~~~~~~~~
    /project/test/selftest/test.cpp:36:9: note: Potential leak of memory pointed to by 'pmyVariable'
            }while(0);
*/
TEST(ExampleTest, NewDelete1)
{
    c_BaseMixIn* pmyVariable = new c_BaseMixIn();
    do{                                     
    delete pmyVariable;                 
    pmyVariable= nullptr;               
    }while(0);
}

/**
 * Does not work with clang-tidy 14.
 * Does work with clang-tidy 18.
 * 
 * /project/test/selftest/test.cpp:62:1: warning: Potential leak of memory pointed to by 'pmyVariable' [clang-analyzer-cplusplus.NewDeleteLeaks]
    }
    ^
    /project/test/selftest/test.cpp:57:32: note: Calling 'c_BaseMixIn::operator new'
        c_BaseMixIn* pmyVariable = new c_BaseMixIn();
                                ^~~~~~~~~~~~~~~~~
    /project/test/selftest/test.cpp:22:18: note: Memory is allocated
            { return ::operator new(nSize); }
                    ^~~~~~~~~~~~~~~~~~~~~
    /project/test/selftest/test.cpp:57:32: note: Returning from 'c_BaseMixIn::operator new'
        c_BaseMixIn* pmyVariable = new c_BaseMixIn();
                                ^~~~~~~~~~~~~~~~~
    /project/test/selftest/test.cpp:62:1: note: Potential leak of memory pointed to by 'pmyVariable'
}
^
*/
TEST(ExampleTest, NewDelete2)
{
    c_BaseMixIn* pmyVariable = new c_BaseMixIn();
    {                                       
    delete pmyVariable;                 
    pmyVariable= nullptr;               
    }
}

/**
 * This will work
*/
TEST(ExampleTest, NewDelete3)
{
    c_BaseMixInNoCustomNewDelete* pmyVariable = new c_BaseMixInNoCustomNewDelete();
    do{                                     
    delete pmyVariable;                 
    pmyVariable= nullptr;               
    }while(0);
}

/**
 * This will work
*/
TEST(ExampleTest, NewDelete4)
{
    c_BaseMixInNoCustomNewDelete* pmyVariable = new c_BaseMixInNoCustomNewDelete();
    {                                       
    delete pmyVariable;                 
    pmyVariable= nullptr;               
    }
}

[maldag]

So you can close this issue as it seems to be fixed with a newer version of clang-tidy