Bug 71867 - Optimizer generates code dereferencing a null pointer
Summary: Optimizer generates code dereferencing a null pointer
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 5.3.0
: P3 normal
Target Milestone: 7.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2016-07-13 21:51 UTC by Vadim Zeitlin
Modified: 2021-12-26 04:04 UTC (History)
2 users (show)

See Also:
Host:
Target: mingw-w64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-07-14 00:00:00


Attachments
The preprocessed file (160.44 KB, application/x-zip-compressed)
2016-07-15 00:33 UTC, asmwarrior
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vadim Zeitlin 2016-07-13 21:51:03 UTC
First of all, I'd like to say that I'm reporting this bug because it looks like a rather bad problem in gcc to me, but I don't have any simple example reproducing it because I couldn't produce one even in spite of spending some time on this, so please feel free to close if you're not interested in debugging this.

The problem in question is that, according to the original bug report (see http://trac.wxwidgets.org/ticket/17483), code generated by gcc -O2 for this method (omitting parts of the class, you can see the full version at https://github.com/wxWidgets/wxWidgets/blob/v3.1.0/include/wx/rtti.h#L86):

class wxClassInfo {
public:
    ...
    bool IsKindOf(const wxClassInfo *info) const
    {
        return info != 0 &&
               ( info == this ||
                 ( m_baseInfo1 && m_baseInfo1->IsKindOf(info) ) ||
                 ( m_baseInfo2 && m_baseInfo2->IsKindOf(info) ) );
    }

private:
    const wxClassInfo       *m_baseInfo1;
    const wxClassInfo       *m_baseInfo2;
};

generates the code which crashes during run-time because m_baseInfo1->IsKindOf() call is done even when m_baseInfo1 is null. The crash doesn't happen with -O0 or even with an attribute optimize("O0") applied to just this function.

Unfortunately, extracting this class and compiling just it with -O2 doesn't show the problem, there must be something else triggering it and making the optimizer assume that the pointers can never be null (which is true for almost all classes, but not for the root class of the hierarchy, which is constructed with null base class info pointer). And, again, I tried, but I couldn't find what it was.

Rewriting the expression as a sequence of statements, as done in https://github.com/wxWidgets/wxWidgets/commit/aa3acfdd15eff1519a41b48a2babe4cba75660f9, fixes the bug, so from my point of view this particular problem is solved, but, again, I find it rather worrying if the optimizer can miscompile quite straightforward code like above, so I still wanted to report it. If you'd like to look at it, please get any version of wxWidgets prior to the commit above (e.g. 3.1.0 release) and build it under Windows. Of course, please let me know if you need any more information -- other than a simple reproducible test case which I, unfortunately, just can't make.

Thanks in advance!
Comment 1 Andrew Pinski 2016-07-14 01:37:31 UTC
Can you provide the preprocessed source?
Comment 2 Vadim Zeitlin 2016-07-14 12:45:01 UTC
I'll try to add the preprocessed code a bit later, but, FWIW, I can already say that there is no macro trickery whatsoever in this code itself.
Comment 3 asmwarrior 2016-07-15 00:33:57 UTC
Created attachment 38908 [details]
The preprocessed file

This is the command I use to generate the preprocessed file(a.cpp in zip)

g++ -E -o a.cpp -O2 -mthreads  -DHAVE_W32API_H -D__WXMSW__   -DNDEBUG    -D_UNICODE   -I..\..\lib\gcc_dll\mswu -I..\..\include  -W -Wall -DWXBUILDING -I..\..\src\tiff -I..\..\src\jpeg -I..\..\src\png -I..\..\src\zlib -I..\..\src\regex -I..\..\src\expat\lib -DwxUSE_BASE=1 -DWXMAKINGDLL   -Wno-ctor-dtor-privacy  -Wno-unused-local-typedefs -Wno-deprecated-declarations -fno-keep-inline-dllexport -g -MTgcc_mswudll\monodll_xh_sizer.o -MFgcc_mswudll\monodll_xh_sizer.o.d -MD -MP ../../src/xrc/xh_sizer.cpp

I use TDM GCC5.1 under Windows XP, I use wxWidgets 2.8.12 official release. The command is run from the folder \build\msw\.
Comment 4 asmwarrior 2016-07-15 00:35:53 UTC
It looks like the code is called from an inline function, you can see this code snippet in the prepossessed file.


inline wxObject *wxCheckDynamicCast(wxObject *obj, wxClassInfo *classInfo)
{
    return obj && obj->GetClassInfo()->IsKindOf(classInfo) ? obj : 
# 502 "..\\..\\include/wx/object.h" 3 4
                                                                  __null
# 502 "..\\..\\include/wx/object.h"
                                                                      ;
}
Comment 5 Andrew Pinski 2016-07-22 06:59:28 UTC
I don't see it at least on aarch64-elf.  But I did not check all checks.
Comment 6 Richard Biener 2016-07-22 11:23:23 UTC
Does -fno-delete-null-pointer-checks make it work?  Does -fsanitize=null trigger?
Comment 7 asmwarrior 2016-07-24 10:36:37 UTC
The -fno-delete-null-pointer-checks option exists in -O2 mode in both GCC 4.9 and GCC 5.x, but this crash issue only happens on GCC 5.x serials. So, why do you think it is the reason?

See my related discussion here: http://forums.codeblocks.org/index.php/topic,21207.msg145242.html#msg145242
Comment 8 asmwarrior 2016-08-03 03:18:53 UTC
Hi, I just build wx2.8.12 under TDM-GCC 5.1 with -fno-delete-null-pointer-checks enabled. But the bad thing is that I still see the same crash here. The whole command is below:

mingw32-make -f makefile.gcc USE_XRC=1 SHARED=1 MONOLITHIC=1 BUILD=release UNICODE=1 USE_OPENGL=1 VENDOR=cb CXXFLAGS="-Wno-unused-local-typedefs -Wno-deprecated-declarations -fno-keep-inline-dllexport  -fno-delete-null-pointer-checks -g" >log-release-no-delete-null.txt 2>&1

For some assembler code of the crash:

[debug]Stack level 0, frame at 0x22e004:
[debug] eip = 0x6d11fb05 in wxClassInfo::IsKindOf (F:\wx\wxMSW-2.8.12\include\wx\object.h:94); saved eip = 0x6cfa043d
[debug] inlined into frame 1
[debug] source language c++.
[debug] Arglist at unknown address.
[debug] Locals at unknown address, Previous frame's sp in esp
[debug]>>>>>>cb_gdb:
[debug]> disassemble 0x6d11fb05
[debug]Dump of assembler code for function wxCheckDynamicCast(wxObject*, wxClassInfo*):
[debug]   0x6d11fa70 <+0>:	push   ebp
[debug]   0x6d11fa71 <+1>:	push   edi
[debug]   0x6d11fa72 <+2>:	push   esi
[debug]   0x6d11fa73 <+3>:	push   ebx
[debug]   0x6d11fa74 <+4>:	sub    esp,0x1c
[debug]   0x6d11fa77 <+7>:	mov    ebx,DWORD PTR [esp+0x30]
[debug]   0x6d11fa7b <+11>:	mov    esi,DWORD PTR [esp+0x34]
[debug]   0x6d11fa7f <+15>:	test   ebx,ebx
[debug]   0x6d11fa81 <+17>:	je     0x6d11fb50 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+224>
[debug]   0x6d11fa87 <+23>:	mov    eax,DWORD PTR [ebx]
[debug]   0x6d11fa89 <+25>:	mov    ecx,ebx
[debug]   0x6d11fa8b <+27>:	call   DWORD PTR [eax]
[debug]   0x6d11fa8d <+29>:	test   esi,esi
[debug]   0x6d11fa8f <+31>:	mov    edx,eax
[debug]   0x6d11fa91 <+33>:	je     0x6d11fb50 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+224>
[debug]   0x6d11fa97 <+39>:	cmp    eax,esi
[debug]   0x6d11fa99 <+41>:	je     0x6d11fb3e <wxCheckDynamicCast(wxObject*, wxClassInfo*)+206>
[debug]   0x6d11fa9f <+47>:	mov    edi,DWORD PTR [eax+0xc]
[debug]   0x6d11faa2 <+50>:	test   edi,edi
[debug]   0x6d11faa4 <+52>:	je     0x6d11fb05 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+149>
[debug]   0x6d11faa6 <+54>:	cmp    esi,edi
[debug]   0x6d11faa8 <+56>:	je     0x6d11fb3e <wxCheckDynamicCast(wxObject*, wxClassInfo*)+206>
[debug]   0x6d11faae <+62>:	mov    ebp,DWORD PTR [edi+0xc]
[debug]   0x6d11fab1 <+65>:	test   ebp,ebp
[debug]   0x6d11fab3 <+67>:	je     0x6d11faed <wxCheckDynamicCast(wxObject*, wxClassInfo*)+125>
[debug]   0x6d11fab5 <+69>:	cmp    esi,ebp
[debug]   0x6d11fab7 <+71>:	je     0x6d11fb3e <wxCheckDynamicCast(wxObject*, wxClassInfo*)+206>
[debug]   0x6d11fabd <+77>:	mov    ecx,DWORD PTR [ebp+0xc]
[debug]   0x6d11fac0 <+80>:	test   ecx,ecx
[debug]   0x6d11fac2 <+82>:	je     0x6d11fad5 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+101>
[debug]   0x6d11fac4 <+84>:	mov    DWORD PTR [esp],esi
[debug]   0x6d11fac7 <+87>:	call   0x6d166c80 <wxClassInfo::IsKindOf(wxClassInfo const*) const>
[debug]   0x6d11facc <+92>:	sub    esp,0x4
[debug]   0x6d11facf <+95>:	test   al,al
[debug]   0x6d11fad1 <+97>:	mov    ecx,ebx
[debug]   0x6d11fad3 <+99>:	jne    0x6d11fb40 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+208>
[debug]   0x6d11fad5 <+101>:	mov    ecx,DWORD PTR [ebp+0x10]
[debug]   0x6d11fad8 <+104>:	test   ecx,ecx
[debug]   0x6d11fada <+106>:	je     0x6d11faed <wxCheckDynamicCast(wxObject*, wxClassInfo*)+125>
[debug]   0x6d11fadc <+108>:	mov    DWORD PTR [esp],esi
[debug]   0x6d11fadf <+111>:	call   0x6d166c80 <wxClassInfo::IsKindOf(wxClassInfo const*) const>
[debug]   0x6d11fae4 <+116>:	sub    esp,0x4
[debug]   0x6d11fae7 <+119>:	test   al,al
[debug]   0x6d11fae9 <+121>:	mov    ecx,ebx
[debug]   0x6d11faeb <+123>:	jne    0x6d11fb40 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+208>
[debug]   0x6d11faed <+125>:	mov    ecx,DWORD PTR [edi+0x10]
[debug]   0x6d11faf0 <+128>:	test   ecx,ecx
[debug]   0x6d11faf2 <+130>:	je     0x6d11fb05 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+149>
[debug]   0x6d11faf4 <+132>:	mov    DWORD PTR [esp],esi
[debug]   0x6d11faf7 <+135>:	call   0x6d166c80 <wxClassInfo::IsKindOf(wxClassInfo const*) const>
[debug]   0x6d11fafc <+140>:	sub    esp,0x4
[debug]   0x6d11faff <+143>:	test   al,al
[debug]   0x6d11fb01 <+145>:	mov    ecx,ebx
[debug]   0x6d11fb03 <+147>:	jne    0x6d11fb40 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+208>
[debug]=> 0x6d11fb05 <+149>:	mov    edx,DWORD PTR [edx+0x10]
[debug]   0x6d11fb08 <+152>:	test   edx,edx
[debug]   0x6d11fb0a <+154>:	je     0x6d11fb50 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+224>
[debug]   0x6d11fb0c <+156>:	cmp    esi,edx
[debug]   0x6d11fb0e <+158>:	je     0x6d11fb3e <wxCheckDynamicCast(wxObject*, wxClassInfo*)+206>
[debug]   0x6d11fb10 <+160>:	mov    ecx,DWORD PTR [edx+0xc]
[debug]   0x6d11fb13 <+163>:	test   ecx,ecx
[debug]   0x6d11fb15 <+165>:	je     0x6d11fb28 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+184>
[debug]   0x6d11fb17 <+167>:	mov    DWORD PTR [esp],esi
[debug]   0x6d11fb1a <+170>:	call   0x6d166c80 <wxClassInfo::IsKindOf(wxClassInfo const*) const>
[debug]   0x6d11fb1f <+175>:	sub    esp,0x4
[debug]   0x6d11fb22 <+178>:	test   al,al
[debug]   0x6d11fb24 <+180>:	mov    ecx,ebx
[debug]   0x6d11fb26 <+182>:	jne    0x6d11fb40 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+208>
[debug]   0x6d11fb28 <+184>:	mov    ecx,DWORD PTR [edx+0x10]
[debug]   0x6d11fb2b <+187>:	test   ecx,ecx
[debug]   0x6d11fb2d <+189>:	je     0x6d11fb50 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+224>
[debug]   0x6d11fb2f <+191>:	mov    DWORD PTR [esp],esi
[debug]   0x6d11fb32 <+194>:	call   0x6d166c80 <wxClassInfo::IsKindOf(wxClassInfo const*) const>
[debug]   0x6d11fb37 <+199>:	sub    esp,0x4
[debug]   0x6d11fb3a <+202>:	test   al,al
[debug]   0x6d11fb3c <+204>:	je     0x6d11fb50 <wxCheckDynamicCast(wxObject*, wxClassInfo*)+224>
[debug]   0x6d11fb3e <+206>:	mov    ecx,ebx
[debug]   0x6d11fb40 <+208>:	add    esp,0x1c
[debug]   0x6d11fb43 <+211>:	mov    eax,ecx
[debug]   0x6d11fb45 <+213>:	pop    ebx
[debug]   0x6d11fb46 <+214>:	pop    esi
[debug]   0x6d11fb47 <+215>:	pop    edi
[debug]   0x6d11fb48 <+216>:	pop    ebp
[debug]   0x6d11fb49 <+217>:	ret    
[debug]   0x6d11fb4a <+218>:	lea    esi,[esi+0x0]
[debug]   0x6d11fb50 <+224>:	add    esp,0x1c
[debug]   0x6d11fb53 <+227>:	xor    ecx,ecx
[debug]   0x6d11fb55 <+229>:	pop    ebx
[debug]   0x6d11fb56 <+230>:	mov    eax,ecx
[debug]   0x6d11fb58 <+232>:	pop    esi
[debug]   0x6d11fb59 <+233>:	pop    edi
[debug]   0x6d11fb5a <+234>:	pop    ebp
[debug]   0x6d11fb5b <+235>:	ret    
[debug]End of assembler dump.
[debug]>>>>>>cb_gdb:

Do you see the pointer is not checked?
The code is in object.h (in wxWidgets's 2.8.12 source code)

// ----------------------------------------------------------------------------
// wxClassInfo
// ----------------------------------------------------------------------------

typedef wxObject *(*wxObjectConstructorFn)(void);

class WXDLLIMPEXP_BASE wxClassInfo
{
public:
    wxClassInfo( const wxChar *className,
                 const wxClassInfo *baseInfo1,
                 const wxClassInfo *baseInfo2,
                 int size,
                 wxObjectConstructorFn ctor )
        : m_className(className)
        , m_objectSize(size)
        , m_objectConstructor(ctor)
        , m_baseInfo1(baseInfo1)
        , m_baseInfo2(baseInfo2)
        , m_next(sm_first)
        {
            sm_first = this;
            Register();
        }

    ~wxClassInfo();

    wxObject *CreateObject() const
        { return m_objectConstructor ? (*m_objectConstructor)() : 0; }
    bool IsDynamic() const { return (NULL != m_objectConstructor); }

    const wxChar       *GetClassName() const { return m_className; }
    const wxChar       *GetBaseClassName1() const
        { return m_baseInfo1 ? m_baseInfo1->GetClassName() : NULL; }
    const wxChar       *GetBaseClassName2() const
        { return m_baseInfo2 ? m_baseInfo2->GetClassName() : NULL; }
    const wxClassInfo  *GetBaseClass1() const { return m_baseInfo1; }
    const wxClassInfo  *GetBaseClass2() const { return m_baseInfo2; }
    int                 GetSize() const { return m_objectSize; }

    wxObjectConstructorFn      GetConstructor() const
        { return m_objectConstructor; }
    static const wxClassInfo  *GetFirst() { return sm_first; }
    const wxClassInfo         *GetNext() const { return m_next; }
    static wxClassInfo        *FindClass(const wxChar *className);

        // Climb upwards through inheritance hierarchy.
        // Dual inheritance is catered for.

    bool IsKindOf(const wxClassInfo *info) const
    {
        return info != 0 &&
               ( info == this ||
                 ( m_baseInfo1 && m_baseInfo1->IsKindOf(info) ) ||
                 ( m_baseInfo2 && m_baseInfo2->IsKindOf(info) ) );
    }
Comment 9 asmwarrior 2017-10-14 14:08:12 UTC
I see this crash issue again, but still it happens in another place of the wx's source code, add the

__attribute__((optimize("O0")))

To the function which cause the crash can workaround this issue. Note that the -O0 option does not cause this issue.

wxString __attribute__((optimize("O0"))) wxCommandEvent::GetString() const
{
    // This is part of the hack retrieving the event string from the control
    // itself only when/if it's really needed to avoid copying potentially huge
    // strings coming from multiline text controls. For consistency we also do
    // it for combo boxes, even though there are no real performance advantages
    // in doing this for them.
    if (m_eventType == wxEVT_TEXT && m_eventObject)
    {
#if wxUSE_TEXTCTRL
        wxTextCtrl *txt = wxDynamicCast(m_eventObject, wxTextCtrl);
        if ( txt )
            return txt->GetValue();
#endif // wxUSE_TEXTCTRL

#if wxUSE_COMBOBOX
        wxComboBox* combo = wxDynamicCast(m_eventObject, wxComboBox);
        if ( combo )
            return combo->GetValue();
#endif // wxUSE_COMBOBOX
    }

    return m_cmdString;
} 

Please see discussions here in wx-user's maillist and Code::Blocks's forum:

https://groups.google.com/d/msg/wx-users/LUxm6fUhirk/FJRFNIt6AAAJ

and 

http://forums.codeblocks.org/index.php/topic,22198.0.html

The original wx's workaround is here: https://trac.wxwidgets.org/ticket/17483
Comment 10 asmwarrior 2017-10-17 00:11:10 UTC
Is it related to pointer casting? I see this post: https://stackoverflow.com/questions/36816363/gcc-4-9-3-more-aggressive-null-pointer-check-removal

Some one reported that the Null check was removed in the following code

void someFunc(struct MyStruct *s)
{
    if (s != NULL)
    {
       cout << s->someField << endl;
       delete s;
    }
}

I see comments in that question, and some one suggest using `-fno-strict-aliasing` option.

@Vadim, dose wx use some cast? I see some macros in wx3.1's source like:

// this cast does some more checks at compile time as it uses static_cast
// internally
//
// note that it still has different semantics from dynamic_cast<> and so can't
// be replaced by it as long as there are any compilers not supporting it
#define wxDynamicCast(obj, className) \
    ((className *) wxCheckDynamicCast( \
        const_cast<wxObject *>(static_cast<const wxObject *>(\
          const_cast<className *>(static_cast<const className *>(obj)))), \
        &className::ms_classInfo))
Comment 11 Jonathan Wakely 2018-10-17 09:36:57 UTC
Does this code still get miscompiled with GCC 6 and newer?
Comment 12 asmwarrior 2018-10-17 10:54:13 UTC
Hi, (In reply to Jonathan Wakely from comment #11)
> Does this code still get miscompiled with GCC 6 and newer?

Hi, I'm now using mingw-build i686-7.2.0-release-posix-dwarf-rt_v5-rev1, which was downloaded from: https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/mingw-builds/7.2.0/threads-posix/dwarf/i686-7.2.0-release-posix-dwarf-rt_v5-rev1.7z/download

I don't see such crash bug since 2018-02-09. (See this post in C::B forum: http://forums.codeblocks.org/index.php/topic,22198.msg152596.html#msg152596)
Comment 13 Andrew Pinski 2021-12-26 04:04:48 UTC
Reporter reports it was fixed in GCC 7, There were many bug fixes between GCC 5.3 and GCC 7 so closing as fixed.