Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UsdImaing crashing in UsdImagingPointInstancerAdapter::GetScenePrimPath() if prototype is defined but not used in protoIndices #2149

Closed
csyshing opened this issue Jan 9, 2023 · 1 comment

Comments

@csyshing
Copy link

csyshing commented Jan 9, 2023

Description of Issue

Hi,

We encountered a crash in Maya when loading specific USD that PointInstancer has N prototypes but protoIndices references only a few of them (see attached USD file).

We are not able to reproduce the crash in usdview, it's possible that Maya USD and usdview are running render delegate in a different way.

The code in Maya USD was introduced in maya-usd/PR-2688, using USD API like this:

HdEngine::Execute() 
    -> renderDelegate->CommitResources()
        -> UsdImagingDelegate::GetScenePrimPath()                     // Attempt to access prim path during `CommitResources()`
            -> UsdImagingPointInstancerAdapter::GetScenePrimPath()    // USD crashes here

The gdb stack:

#0  pxrInternal_v0_22__pxrReserved__::UsdImagingPointInstancerAdapter::GetScenePrimPath (this=0x20d1d420, cachePath=..., instanceIndex=0, instancerContext=0x0) at pxr/usdImaging/usdImaging/pointInstancerAdapter.cpp:1394
#1  0x00007ffe7162f9ce in pxrInternal_v0_22__pxrReserved__::UsdImagingDelegate::GetScenePrimPath (this=0x20ba7d00, rprimId=..., instanceIndex=0, instancerContext=0x0) at pxr/usdImaging/usdImaging/delegate.cpp:2360
#2  0x00007ffe14a06cc1 in pxrInternal_v0_22__pxrReserved__::ProxyRenderDelegate::GetScenePrimPath (this=this@entry=0x212a4990, rprimId=..., instanceIndex=instanceIndex@entry=0, instancerContext=instancerContext@entry=0x0) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp:1283
#3  0x00007ffe14a08362 in pxrInternal_v0_22__pxrReserved__::ProxyRenderDelegate::GetPathInPrototype (this=this@entry=0x212a4990, id=...) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp:884
#4  0x00007ffe149e7333 in pxrInternal_v0_22__pxrReserved__::MayaUsdRPrim::<lambda()>::operator() (__closure=0x7ffc856c5628) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/mayaPrimCommon.cpp:882
#5  pxrInternal_v0_22__pxrReserved__::HdVP2TaskCommitBody<pxrInternal_v0_22__pxrReserved__::MayaUsdRPrim::_SyncSharedData(pxrInternal_v0_22__pxrReserved__::HdRprimSharedData&, pxrInternal_v0_22__pxrReserved__::HdSceneDelegate*, const HdDirtyBits*, const pxrInternal_v0_22__pxrReserved__::TfToken&, const pxrInternal_v0_22__pxrReserved__::HdRprim&, const ReprVector&, const pxrInternal_v0_22__pxrReserved__::TfToken&)::<lambda()> >::operator()(void) (this=0x7ffc856c5620) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/task_commit.h:57
#6  0x00007ffe14a15c9b in pxrInternal_v0_22__pxrReserved__::HdVP2ResourceRegistry::Commit (this=<optimized out>) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/resource_registry.h:42
#7  pxrInternal_v0_22__pxrReserved__::HdVP2RenderDelegate::CommitResources (this=0x20c07750, tracker=<optimized out>) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/render_delegate.cpp:652
#8  0x00007ffe1607e2fb in pxrInternal_v0_22__pxrReserved__::HdEngine::Execute (this=this@entry=0x212a49a8, index=0x20b218a0, tasks=tasks@entry=0x212a49e8) at pxr/imaging/hd/engine.cpp:169
#9  0x00007ffe14a0ea51 in pxrInternal_v0_22__pxrReserved__::ProxyRenderDelegate::_Execute (this=this@entry=0x212a4990, frameContext=...) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp:1181
#10 0x00007ffe14a0f981 in pxrInternal_v0_22__pxrReserved__::ProxyRenderDelegate::update (this=0x212a4990, container=..., frameContext=...) at maya-usd/lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp:1234
#11 0x00007fffa9da716f in MHWRender::THsubSceneEvaluator::update (nodes=..., this=0x213a85d0) at OpenMayaRender/Render/THsubSceneEvaluator.cpp
#12 MHWRender::THsubSceneEvaluator::update (this=0x213a85d0, nodes=...) at OpenMayaRender/Render/THsubSceneEvaluator.cpp
...

I understand it is not fair to report issue in USD when it is only reproducible in Maya, but regardless of the API usage, when looking into the crash location at line 1394 in UsdImagingPointInstancerAdapter::GetScenePrimPath():

line 1382:    // Compute the local & parent instance index.
line 1383:    VtValue indicesValue = GetInstanceIndices(_GetPrim(instancerPath),
line 1384:            instancerPath, cachePath, _GetTimeWithOffset(0.0));
....
line 1389:    VtIntArray const & indices = indicesValue.UncheckedGet<VtIntArray>();
line 1390:
line 1391:    // instanceIndex = parentIndex * indices.size() + i.
line 1392:    int parentIndex = instanceIndex / indices.size();
line 1393:    // indices[i] gives the offset into the index buffers (i.e. protoIndices).
line 1394:    int localIndex = indices[instanceIndex % indices.size()];

It seems like the code is accessing indices without checking the array boundary, in our case, indices returned from GetInstanceIndices() at line 1383 was empty due to the prototype was never used in protoIndices, thus USD crashes at line 1394.

There was a similar issue in UsdImagingInstanceAdapter::GetScenePrimPaths() and it's been fixed in this commit.
Wondering if we could apply similar fix for PointInstancer? E.g. insert a check before line 1394:

if (indices.empty() || (instanceIndex % indices.size()) >= indices.size()) {
    return SdfPath();
}

The attached USD file has one point instancer, with 4 prototypes, and protoIndices references 0, 2, and 3 but not 1.

Thanks,
Zhicheng

Steps to Reproduce

  1. Launch Maya and load 'mayaUsdPlugin'
  2. Create USD stage from the attached usda file, Maya would crash

System Information (OS, Hardware)

OS: CentOS 7.8

Package Versions

USD-22.11 / USD-dev

Build Flags

All default build flags.

USD file:
point_instancer_crash.zip

@sunyab
Copy link
Contributor

sunyab commented Jan 9, 2023

Filed as internal issue #USD-7852

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants