compiler issue in msvc 2026?

[leadcoder]

osgEarth Version: master

Description of the problem:

When compiling osgEarth in RelWithDebInfo and Release with msvc 2026 (Version: 18.2.0) and running the feature_country_boundaries.earth sample in the osgEarthViewer I get a crash in OGRFeatureSource, https://github.com/gwaldron/osgearth/blob/1569a59f57396aba94c4d5dc2075926bac17e79f/src/osgEarth/OGRFeatureSource.cpp#L287

If i can trust the debugger it looks like the osg::ref_ptr constructor executed in the filterList.push_back get a nullptr and as result i get "Unhandled exception thrown: write access violation.
this was 0x10." in the OpenThreads::Atomic::operator++ method called by the osg::ref_ptr constructor

What you have tried:

• No crash in Debug (msvc2026)
• No crash msvc 2022 RelWithDebInfo and Release
• Everything works if i just push the ref_ptr without release, ie filterList.push_back(feature);
• I've tried other vector-data sources and get the same issue.

I tried check for nullptr (should not happen), ie
auto* feature_ptr = feature.release();
if (feature_ptr)
filterList.push_back(feature_ptr);

...but same problem, when the exception is raised and catched by the debugger the feature_ptr is null in the debugger? very confusing.

I have not looked at the assembler code but maybe optimization bug in the compiler?

[gwaldron]

Any difference if you just remove the .release() ? Honestly it does not need to be there.

[leadcoder]

Yes it works if i remove the release. However I don't see why the current implementation should not be valid usage of osg::ref_ptr, maybe the optimizer is to aggressive in msvc2026. As for now I don't see any other issues with msvc2026 so i hope this is the only problem.

[MCA4213]

same here, it crashes in release mode on msvc 2026 and work on debug!!!
but you saved me, removing .release() fixed the problem, thanks

[MCA4213]

same for wfs (also for tfs and XYZFeatureSource i assume), replace features.push_back(f.release()); with features.push_back(f);

[gwaldron]

How strange :)
We are happy to accept a PR getting rid of extraneous release() calls ... but it still doesn't answer the vexing question of why, and whether other OSG internals will break for the same reason ...

[leadcoder]

I can confirm that this is still an issue with the latest update of msvc2026 (18.3.0) After looking at the disassembly i'am more convinced that this is a compiler bug.

00007FF8BA6462F1  call        qword ptr [__imp_osg::Referenced::unref_nodelete (07FF8BB45C038h)]  
00007FF8BA6462F7  mov         rbx,r12  //clear rbx, r12 is zero
00007FF8BA6462FA  mov         rsi,r12  //clear rbx
00007FF8BA6462FD  mov         r14,r12  //cleared r14,  r14 is later assumed to store pointer to the feature object (that is stored in rdi)
00007FF8BA646300  jmp         osgEarth::OGR::OGRFeatureCursor::readChunk+234h (07FF8BA646314h)  //invalid path B
   272:                         {
   273:                             
   274:                             filterList.push_back( feature.release() );
00007FF8BA646302  mov         rsi,rdi  //valid path A
00007FF8BA646305  mov         r14,rdi  //rdi hold feature pointer, and its moved in r14
00007FF8BA646308  mov         rcx,rdi  
00007FF8BA64630B  call        qword ptr [__imp_osg::Referenced::unref_nodelete (07FF8BB45C038h)]  
00007FF8BA646311  mov         rbx,rdi  
00007FF8BA646314  mov         qword ptr [rbp+40h],rbx  //landing here when jmp from 00007FF8BA646300 (path B)
00007FF8BA646318  mov         qword ptr [rbp+48h],r12  
00007FF8BA64631C  mov         rdi,r12  
00007FF8BA64631F  lea         rcx,[r14+10h]  //this assume r14 hold feature pointer (to load the refcounter atomic)
00007FF8BA646323  call        qword ptr [__imp_OpenThreads::Atomic::operator++ (07FF8BB457800h)] 

Looks like there there are two paths into the "if (validateGeometry( feature->getGeometry() ))" block
The valid path A is a jump to 00007FF8BA646302 but there is another jmp at 00007FF8BA646300 (paht B) that land on 00007FF8BA646314. This is the broken path and the only one that I hit. When landing on 00007FF8BA646314 r14 is assume to store the feature pointer that is used to get the osg::Reference::_refCount atomic with lea rcx,[r14+10h] and we get a crash when calling OpenThreads::Atomic::operator++ at 00007FF8BA646323