[S] Change in dart/sdk[main]: [vm/ffi]: Pass RTLD_DEEPBIND on Linux when loading code assets
Simon Binder (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Daco Harkes
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daco Harkes (Gerrit)
Daco Harkes added 4 comments![]( "Open in Gerrit")
Hm, our sanitizer tests start failing immediately.
char** error = nullptr,nit: I believe output parameters should come last, even if there are optional parameters. (You can consider doing overloading and add a `LoadDynamicLibrary` which doesn't take any optional params.)
Related details
- Simon Binder
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Binder (Gerrit)
Simon Binder added 4 comments![]( "Open in Gerrit")
Hm, our sanitizer tests start failing immediately.
From the logs:
> stderr : ==20892==You are trying to dlopen a
/b/s/w/ir/out/ReleaseMSANX64/libffi_test_functions.so shared library
with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime
(see https://github.com/google/sanitizers/issues/611
for details). If you want to run
/b/s/w/ir/out/ReleaseMSANX64/libffi_test_functions.so library under
sanitizers please remove RTLD_DEEPBIND from dlopen flags.
So do we need to make this optional after all? :D
Or do you know if there's say a preprocessor define we could use in the VM to not use this flag when compiling with sanitizers?
nit: I believe output parameters should come last, even if there are optional parameters. (You can consider doing overloading and add a `LoadDynamicLibrary` which doesn't take any optional params.)
Done
first = false;Simon Bindernit: unrelated change?
Reverted
Related details
- Daco Harkes
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daco Harkes (Gerrit)
Daco Harkes added 1 comment![]( "Open in Gerrit")
Simon BinderHm, our sanitizer tests start failing immediately.
From the logs:
> stderr : ==20892==You are trying to dlopen a
/b/s/w/ir/out/ReleaseMSANX64/libffi_test_functions.so shared library
with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime
(see https://github.com/google/sanitizers/issues/611
for details). If you want to run
/b/s/w/ir/out/ReleaseMSANX64/libffi_test_functions.so library under
sanitizers please remove RTLD_DEEPBIND from dlopen flags.
So do we need to make this optional after all? :D
Or do you know if there's say a preprocessor define we could use in the VM to not use this flag when compiling with sanitizers?
Yes, we have defines `USING_ADDRESS_SANITIZER` and friends.
But, it feels off to change the resolution logic based on sanitizers.
If we would make it configurable in the build hook protocol, then we'd be able to error in the build hook if we build asan/tsan/msan mode. It then puts the responsibility on the hook author to chose whether a package does not support ASAN.
Though, I still find it weird, because as a hook author, you don't know how many Linux embedders there are going to be (let's say someone comes up with something based on Dart that is not Flutter), and those embedders might change the libraries they import. So how would you program in a way that avoids conflicting with potential libraries.
As the hook author, you'd only want to use the deepbind for Flutter, and not for Dart standalone. So, you'd need to know which embedder you are running on. Which is kind of the opposite of the design goal of the hooks.
Let me loop in some other folks and discuss with them to see what their opinions are.
Related details
- Simon Binder
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daco Harkes (Gerrit)
Daco Harkes abandoned this change![]( "Open in Gerrit")
Related details
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |