PixarAnimationStudios / OpenSubdiv

An Open-Source subdivision surface library.
graphics.pixar.com/opensubdiv
Other
2.9k stars 561 forks source link

Compiler Warning C5045 when compiling 3.3.3 with VS2017 (security issue) #1045

Closed langlor-autodesk closed 4 years ago

langlor-autodesk commented 5 years ago

Compiling with VS2017 produce this warning error Compiler Warning C5045 in OpenSubdiv https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5045?view=vs-2017 https://docs.microsoft.com/en-us/cpp/security/developer-guidance-speculative-execution?view=vs-2017

The error log gives information about faulty lines: 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(58): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(57) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(58) : note: feeds call on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(160): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(159) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(160) : note: feeds call on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(85): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(84) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(85) : note: feeds call on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(123): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(120) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(134) : note: feeds call on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(144): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(143) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(144) : note: feeds call on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(92): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(91) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(92) : note: feeds call on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(121): warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(120) : note: index 'i' range checked by comparison on this line 1> c:\dev\maya\artifactory\opensubdiv\ebd8bc37\opensubdiv-3.3.3.master.2018.08.23\opensubdiv\sdc\crease.cpp(121) : note: feeds call on this line

davidgyu commented 5 years ago

Interesting. Are you specifically enabling the VS2017 /Qspectre option and/or intending to use OpenSubdiv to process sensitive data which crosses a trust boundary?

There is a reported VS2017 issue on systems with the Windows 10 1809 WDK (Windows Driver Kit) installed, that causes /Qspectre to be enabled inadvertently for VS2017 projects: https://developercommunity.visualstudio.com/content/problem/348985/installing-wdk-1809-enabled-spectre-mitigation-fla.html If you aren't specifically enabling /Qspectre, it would be good to check that you are not being affected by this problem.

The lines of code flagged by those warnings are architecturally correct, but they are code sites that would be affected when directing the compiler to insert Spectre mitigation code.

We don't currently have plans to make source code changes to OpenSubdiv because of Spectre. The best practice here is to organize application code to preserve trust boundaries when handling sensitive data.

Thanks! -David

jtran56 commented 5 years ago

Filed as internal issue #OSD-254.

davidgyu commented 4 years ago

Marking this closed base on the previous comment. Thanks!

CraigHutchinson commented 1 year ago

This ticket overlaps existing #1269 and I have been blocked by this on MSVC builds under VC 2022 It is caused by /Wall + /WX options (enable C5045 + warnings as error)

This was addressed in PR https://github.com/PixarAnimationStudios/OpenSubdiv/pull/1273 but didn't get merged https://github.com/PixarAnimationStudios/OpenSubdiv/blob/8ffa2b6566be10209529d7a0d1db02a0796b160c/CMakeLists.txt#L235

CraigHutchinson commented 1 year ago

I see the initial PR was abandoned and there is a new one in flight to address this issue. Adding linkage here: https://github.com/PixarAnimationStudios/OpenSubdiv/pull/1283