[Bug c++/70403] New: A null pointer check removed with -O2 even with -fno-delete-null-pointers-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403
Bug ID: 70403
Summary: A null pointer check removed with -O2 even with
-fno-delete-null-pointers-checks
Product: gcc
Version: 4.9.2
Status: UNCONFIRMED
Severity: major
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: thadula at ciena dot com
Target Milestone: ---
g++, powerpc-e500-linux-gnuspe-g++, version 4.9.2
compiled with -O2 and -fno-delete-null-pointers-checks
Code (look for ==> to mark the point in the code):
static const char* fnName = "NewLs::CpSelect - ";
tbr_t tbrStart, tbrEnd, timediff;
tbrStart = tbrGet();
McId newCpId((McIdHandle)cpId);
McId newSelId((McIdHandle)selId);
McCpId* pCpId= static_cast(&newCpId);
ComplexXCon* pXCon= NULL;
CtpCpBase* pNewSel = NULL;
ComplexXCon* pNewXCon = NULL;
UniCp* pCp = NULL;
rFailureReason = LS_SUCCESS;
if (!pCpId)
{
MEMORY_LOG_MCHECK (getSwitchCoreMemoryLog(), pCpId, NULL,
"!!!ERROR!!!: " << fnName << "null CP id");
return false;
}
// Find the CP object
pCp = FindCpFromHashList(*pCpId);
if (!pCp)
{
LOGIC_SWITCH_ERRLOG_PRINTF (getSwitchCoreMemoryLog(),
"%sCannot find CP 0x%llX", fnName, cpId);
rFailureReason = END_POINT_NOT_FOUND;
return false;
}
McCpProtectionApp protApp = pCp->GetProtApp();
if (rxProtApp != protApp)
{
.
.
.
return false;
}
pXCon = pCp->GetXCon();
if (!pXCon)
{
char mcBuf[ 64 ];
pCpId->toString (mcBuf, sizeof (mcBuf));
LOGIC_SWITCH_ERRLOG_PRINTF (getSwitchCoreMemoryLog(),
"%sCannot find XCon for CP: %s", fnName, mcBuf);
rFailureReason = CROSS_CONNECT_NOT_FOUND;
return false;
}
if (!squelch)
{
if (newSelId == McId::InvalidId)
{
rFailureReason = BAD_END_POINT_ID;
return false;
}
//
// Find the CTP or CP (if it exists) and verify that it belongs to
pXCon
//
if (newSelId.IsCtpId())
{
McCtpId ctpId(newSelId);
pNewSel = FindCtpFromHashList(ctpId);
}
else if (newSelId.IsCpId())
{
McCpId newCpId((McCpIdHandle) newSelId.GetId());
pNewSel = FindCpFromHashList(newCpId);
}
==> if (pNewSel)
pNewXCon = pNewSel->GetXCon();
}
else
{
char mcBuf[ 64 ];
pCpId->toString (mcBuf, sizeof (mcBuf));
getSwitchCoreMemoryLog()->PrintRamLog ("%sSetting %s to be squelched.",
fnName, mcBuf);
}
if (squelch || (pNewSel && pNewXCon == pXCon))
{
if (pCp->GetSelectedCtpCp() == pNewSel
|| (isDefaultFromCac && pCp->GetDefaultSelector() == pNewSel))
{
.
.
.
return false; // already selected
}
// Update current selector on CP.
pCp->SetSelectedCtpCp(pNewSel);
.
.
.
}
Segmentation fault:
Program terminated with signal 11, Segmentation fault.
#0 CtpCpBase::GetXCon (this=this@entry=0x0) at
src/software/centaur/apps/core/txn/Switching/NewLS/CtpCpBase.cpp:79
79 if( mpXCon && mpXCon->IsRemoteTAP() && !mpXCon->IsRemoteTAPParent()
)
(gdb) bt
#0 CtpCpBase::GetXCon (this=this@entry=0x0) at
src/software/centaur/apps/core/txn/Switching/NewLS/CtpCpBase.cpp:79
#1 0x1149b610 in NewLogicalSwitch::CpSelect (this=0x671d0038, cpId=, selId=,
rFailureReason=@0x7019e6f0: LS_SUCCESS,
rxProtApp=rxProtApp@entry=MC_CP_PROTECTION_APP_UPSR_SNCP, squelch=true,
isDefaultFromCac=isDefaultFromCac@entry=false) at
src/software/centaur/apps/core/txn/Switching/NewLS/NewLogicalSwitch.cpp:8221
#2 0x1149bfb4 in NewLogicalSwitch::CpSelectTest (this=,
cpId=cpId@entry=2, newSelId=)
at
src/software/centaur/apps/core/txn/Switching/NewLS/NewLogicalSwitch.cpp:5869
#3 0x114c704c in NewLsDcraft::ImplementMsg (this=0x6df9b338, pObj=0x836ea7e8)
at src/software/centaur/apps/core/txn/Switching/NewLS/NewLsDcraft.cpp:711
#4 0x11cafe6c in Actor::Run (this=0x6df9b338) at
src/generic/component/Actor/Actor.cpp:929
#5 0x11bdff9c in RunnableTask::TaskFunction (this=0x6df9c970) at
src/generic/component/UtilProg/RunnableTask.cpp:35
#6 0x11593080 in LnTask::TaskEntry (pThis=0x6df9c970) at
src/generic/component/Osencap/linux/LnTask.cpp:627
#7 0x0f1ecc04 in ?? ()
#8 0x09e4a5b4 in ?? ()
(gdb) up
#1 0x1149b610 in NewLogicalSwitch::CpSelect (this=0x671d0038, cpId=, selId=,
rFailureReason=@0x7019e6f0: LS_SUCCESS,
rxProtApp=rxPr
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointers-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403
--- Comment #2 from Hadula, Tomasz ---
The whole thing is huge (compiles for almost 2 hours).
The problem is that when I attempt to make it smaller in form of a simple,
standalone app, the problem doesn't happen anymore.
I've been searching for something that dereferences pNewSel or some sort of
memcpy or memmove, but I can't find it.
Those are 2 methods used to obtain pNewSel. Hashlist is based on RogueWave
hashset:
UniCtp*
NewLogicalSwitch::FindCtpFromHashList (McCtpId ctpId)
{
UniCtp* pDummyCtp = mUniCtpPool.Get();
if (!pDummyCtp)
{
getSwitchCoreMemoryLog()->PrintRamLog (
"NewLogicalSwitch::FindCtpFromHashList: no more room in the CTP
pool");
return NULL;
}
pDummyCtp->CtpCpBase::Init(ctpId); // call base class method; need
CtpCpBase:: qualifier because UniCtp redefines Init(), hiding the base class
version
UniCtp* pCtp = mCtpHashList.find(pDummyCtp);
pDummyCtp->Destroy(); // return to mempool
return pCtp;
}
UniCp*
NewLogicalSwitch::FindCpFromHashList (McCpId cpId)
{
// Note - The prot app is not used in the find process.
//Find just compares the ids of the two CPs.
//The prot app is not part of the id.
//We just needed to pass in a dummy prot app
//along with the real id to create the dummyCp.
//
TcmInfo dummyTcmInfo;
UniCp* pDummyCp, *pUniCp;
pDummyCp = mUniCpPool.Get();
if (!pDummyCp)
{
getSwitchCoreMemoryLog()->PrintRamLog (
"NewLogicalSwitch::FindCpFromHashList: no more room in the CP
pool");
return NULL;
}
pDummyCp->Init(cpId, MC_CP_PROTECTION_APP_NONE,
MC_CP_PU_TYPE_NONE,
0, dummyTcmInfo, true, true, true, true, McId::InvalidId);
pUniCp = mCpHashList.find (pDummyCp);
// Return CP to MemPool
pDummyCp->Destroy();
return pUniCp;
}
Just a command line used to compile that particular source goes like this:
/usr/local/ciena/powerpc-e500-linux-gnuspe-20160316/bin/powerpc-e500-linux-gnuspe-g++
-Wp,-MD,/simdisk/thadula/oneos/branches/centaur/dev/main/build/centaur-cn5430/libNewLS.a/chassis/powerpc-e500/32/.NewLogicalSwitch.o.d,-MP
-g -pipe -O2 -fno-delete-null-pointer-checks -std=gnu++03 -Wall -Wextra
-Wno-unused-parameter -Wno-deprecated -Wno-unused-but-set-parameter
-Wno-unused-function -Wno-unused-parameter -fPIC
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/software/centaur/apps/core/txn/Switching/NewLS
-DCIENA_CTM -DCIENA_CENTAUR -DOSI_STACK_ENABLED -DCENTAUR_5430
-DCENTAUR_SPECIFIC -DWARM_RESTART
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardware/uio_ciena_hdpsm/lib/
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardware/uio_ciena_hdpsm/ko/
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardware/uio_ciena_hdsm/lib/
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardware/uio_ciena_hdsm/ko/
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardware/uio_ciena_sm/lib/
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardware/uio_ciena_sm/ko/
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/elf-utils
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/include
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/linxipc
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/log
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/onesies
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/subsys-elf
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/cad
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/compiler
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/apps/apis
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/apps/include
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/apps/infrastructure
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/apps/notifications
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/apps/utilities
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ntp
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/nonbranched
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/system/map
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/system/shared
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/system/shared/hal
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/ni/system/branding
-D_REENTRANT -DRW_MULTI_THREAD -DRW_POSIX_THREADS -DRWDEBUG
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/event_mgr
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/generic/osrp/osrputil
-iquote/simdisk/thadula/oneos/branches/centaur/dev/main/src/hardwa
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointers-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #4 from Hadula, Tomasz --- Created attachment 38085 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38085&action=edit output from -fdump-tree-optimized I attached dump from -fdump-tree-optimized for the affected source (NewLogicalSwitch.cpp). Please look for ::CpSelect in it
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointers-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #5 from Hadula, Tomasz --- Created attachment 38086 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38086&action=edit Original and preprocessed source generated with -E
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointer-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #6 from Hadula, Tomasz --- Created attachment 38093 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38093&action=edit Output from gcc -v -save-temps Generated by g++ -v -save-temps
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointer-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #7 from Hadula, Tomasz --- Created attachment 38094 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38094&action=edit Preprocessed source generated by gcc -v -save-temps Unfortunately I had to compress it with gzip as the file size exceeded 1000 KB and I couldn't attach it as is.
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointer-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #8 from Hadula, Tomasz --- When I compile with devirtualization disabled (i.e. with -fno-devirtualize) the null pointer check is where it was supposed to be. Any clue why?
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointer-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #9 from Hadula, Tomasz --- Created attachment 38148 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38148&action=edit Reduced testcase I reduced the size of the testcase
[Bug c++/70403] A null pointer check removed with -O2 even with -fno-delete-null-pointer-checks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70403 --- Comment #12 from Hadula, Tomasz --- > The reduced testcase doesn't have the pattern you describe in > the description. Well, of course, I couldn't verify that for sure, because the reduced testcase was so severely mutilated that I couldn't possibly use to generate a running program. However when I was looking into the output produced by -fdump-tree-optimized I could see that pNewSel_2 in in CpSelect was not tested for null. > Compiling with -fsanitize=undefined would have found that bug. I tried that earlier, but the whole executable is so heavy that I couldn't even have it started in the realistic time. So sanitizing it during runtime is the challenge that we keep working on, but not there yet. Static analysis would work better for us. Is there any option that could point out any undefined behavior and print it as a warning? (e.g. -Wundefined ? /but under 4.9.2 it is not recognized option/) Anyway thank you very much for your help. I'm so glad that it gave us opportunity to put that bug to rest eventually. Right about the time I posted the reduced testcase yesterday, we have found many more occurrences of that bug in other places in the code involving different classes, but the pattern seems to be the same. In the first preprocessed source I attached, you could find at least one more. Hard to say was the original idea behind it, but looking at the source control it seems to be there since beginning of time. It's like way over a decade old bug. I still don't understand though why - since "pCpId" was responsible for undefined behavior - why did devirtualization pick on "pNewSel" (and also in the original source: on "squelch")?
