Page MenuHome

Cleanup: prevent USD exports from leaking into blender binary.
ClosedPublic

Authored by LazyDodo (LazyDodo) on Sat, Jan 11, 10:38 PM.

Details

Summary

Even though we build USD as static, it still feels the need to mark its
symbols with declspec(dllexport) which means the blender binary now exports
these symbols.

now this isn't necessarily harmful there is no reason to be sloppy and
export symbols that have no business getting exported.

This patch fixes the behavior for USD which was by far the worst offender
in this area with 6137 exports. However that doesn't mean the other libs
that exhibit this behavior get a free pass, they too will be dealt with.

Before:

After:

Diff Detail

Repository
rB Blender

Event Timeline

LazyDodo (LazyDodo) edited the summary of this revision. (Show Details)Sat, Jan 11, 10:40 PM
build_files/build_environment/patches/usd.diff
122–124

Is this really only an issue on Windows? Or should we do something about this on other platforms as well?

I don't think it matters all that much on other platforms, it's always been a wild west regarding symbol exports on linux

There is a fundamentally different philosophy between the platforms:

with msvc default is : DO NOT EXPORT, if you want to export a symbol from a dynamic library, you have to explicitly request it by tagging on declspec(dllexport) . If you want to export all symbols, the compiler literally won't let you, cmake has an optional hack to work around this if you are so inclined.
with gcc had they for the longest time this approach: EXPORT ALL THE THINGS!! while hiding symbols has become more common (gcc wiki here, and USD ticket regarding it not working here ) the default afaik still is 'export all' unless you pass -fvisibility=hidden

I'd say it's up to the platform dev to decide if this is an issue for them or not.

Judging by the hacks already in place to make it build this (monolithic static) is clearly not a tested scenario for upstream, so i just stuck it in a win32 if not to bother the other platforms.

This revision is now accepted and ready to land.Tue, Jan 14, 12:44 PM