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

[hd, hdx] Move tracing macros in InsertTask<T> #3000

Merged
merged 2 commits into from
May 17, 2024

Conversation

nvidia-jomiller
Copy link
Collaborator

@nvidia-jomiller nvidia-jomiller commented Mar 11, 2024

Description of Change(s)

This PR fixes a very hard to identify issue with link time optimization (-flto) for hdx/taskController.cpp where GetRenderIndex()->InsertTask<HdxTask>() would fail with cryptic error messages for release builds of USD with address sanitizers.

Effectively, GetRenderIndex()->InsertTask<HdxPickTask>() is defined in hdx/unitTestDelegate.cpp and hdx/taskController.cpp. The linker will, understandably, optimize out the symbol(s) in hdx/taskController.o since hdx/unitTestDelegate.o defines it first. This causes a linker error since it's GetRenderIndex()->InsertTask<HdxPickTask>() has been instrumented with ASan inside of taskController.cpp

An example of the error for a release build with address sanitizers enabled:

`_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_11HdxPickTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__' referenced in section `.data.rel.local' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o: defined in discarded section `.rodata._ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_11HdxPickTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__[_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_11HdxPickTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE16TraceKeyData_596]' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o
`_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_13HdxShadowTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__' referenced in section `.data.rel.local' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o: defined in discarded section `.rodata._ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_13HdxShadowTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__[_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_13HdxShadowTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE16TraceKeyData_596]' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o
`_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_18HdxSimpleLightTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__' referenced in section `.data.rel.local' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o: defined in discarded section `.rodata._ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_18HdxSimpleLightTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__[_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_18HdxSimpleLightTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE16TraceKeyData_596]' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o
`_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_16HdxSelectionTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__' referenced in section `.data.rel.local' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o: defined in discarded section `.rodata._ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_16HdxSelectionTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__[_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_16HdxSelectionTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE16TraceKeyData_596]' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o
`_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_13HdxRenderTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__' referenced in section `.data.rel.local' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o: defined in discarded section `.rodata._ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_13HdxRenderTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE8__func__[_ZZN32pxrInternal_v0_24__pxrReserved__13HdRenderIndex10InsertTaskINS_13HdxRenderTaskEEEvPNS_15HdSceneDelegateERKNS_7SdfPathEE16TraceKeyData_596]' of pxr/imaging/hdx/CMakeFiles/hdx.dir/taskController.cpp.o

The root of the issue is caused by the HD_TRACE_FUNCTION in renderIndex.h. With this macro disabled, a release build of USD with address sanitizers will not encounter this error.

To circumvent this, the HD_TRACE_FUNCTION and HF_MALLOC_TAG_FUNCTION macros have been relocated into _TrackDelegateTask. The _TrackDelegateTask function now accepts a task creation function to create the actual HdTask . This prevents the weird link-time optimization error shown above.

Fixes Issue(s)

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

In order to prevent incorrect link time optimizations with taskController.cpp
and address sanitizer, unitTestDelegate subclasses the necessary HdxTasks
@jesschimein
Copy link
Collaborator

Filed as internal issue #USD-9436

❗ Please make sure that a signed CLA has been submitted!

@jesschimein
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

HD_TRACE_FUNCTION and HF_MALLOC_TAG_FUNCTION macros have been relocated into _TrackDelegateTask
The _TrackDelegateTask function now accepts a task creation function to create the actual HdTask
This prevents a weird link-time optimization error when building with address sanitizers
@nvidia-jomiller nvidia-jomiller changed the title [hdx] Define HdxTask subclasses for unitTestDelegate [hd, hdx] Move tracing macros in InsertTask<T> Mar 15, 2024
@jesschimein
Copy link
Collaborator

Please disregard the comment about the CLA -- we have you on file, Joshua!

@jesschimein
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pixar-oss pixar-oss merged commit 2dce784 into PixarAnimationStudios:dev May 17, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

3 participants