-
Notifications
You must be signed in to change notification settings - Fork 602
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
bazel: fix sanitizers #24948
bazel: fix sanitizers #24948
Conversation
Needed for the sanitizers
The global configuration set a value on a singleton in boost, which causes use after free when the global configuration is torn down, but the singleton then touches the ostream on it's teardown.
} | ||
} | ||
std::ofstream _out; | ||
static inline std::optional<std::ofstream> _out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this was my doing. An example I saw had some deregister code in the desctructor which I omitted, which probably avoided the UAF.
So in this case _out
outlives the problematic access? Because the config object itself is destroyed (I think on every test) but the the static outlives that, until static destruction time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in this case _out outlives the problematic access?
Yeah, the destructor for the unit test singleton runs before this from my testing and the sanitizer is happy too. I've not fully loaded the standard on destructor ordering into ram here, so this still could be sketchy. However it does seem the XML files are still being created properly, so I think we're good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think on every test
The documentation seems to disagree with that, I think everything we're playing with here are singletons, but the global configuration singleton is destroyed before the unit test stuff, which is why we need that to run.
Backports Required
Release Notes