-
Notifications
You must be signed in to change notification settings - Fork 754
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
Segfault when using downcast_ref
through Layer
#453
Labels
Comments
jonhoo
added a commit
to jonhoo/tracing-timing
that referenced
this issue
Nov 26, 2019
Currently blocked on tokio-rs/tracing#453.
hawkw
added a commit
that referenced
this issue
Nov 26, 2019
These tests failed to catch issue #453 because they never actually dereference the ref created by `downcast_ref`. Therefore, if an invalid pointer is returned, the test can still pass, as long as _any_ pointer is returned. This is incorrect. This commit changes the tests to try and access data through the returned references. Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this issue
Nov 26, 2019
Currently, `Layer::downcast_raw` constructs a pointer with: ```rust &self as *const _ as *const () ``` This is wrong. The method's receiver is already `&self`, so this code constructs a reference _to_ the `&self` reference, and converts _that_ into a pointer. Since this is not the actual memory address of the layer, this is invalid. This commit changes the pointer to: ```rust self as *const _ as *const () ``` Now, the returned pointer is valid. Fixes #453 Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this issue
Nov 26, 2019
) ## Motivation Currently, `Layer::downcast_raw` constructs a pointer with: ```rust &self as *const _ as *const () ``` This is wrong. The method's receiver is already `&self`, so this code constructs a reference _to_ the `&self` reference, and converts _that_ into a pointer. Since this is not the actual memory address of the layer, this is invalid. We didn't catch this because the tests for downcasting layers are also wrong: they never actually dereference the ref created by `downcast_ref`. Therefore, if an invalid pointer is returned, the test can still pass, as long as _any_ pointer is returned. ## Solution This branch changes the pointer to: ```rust self as *const _ as *const () ``` Now, the returned pointer is valid. In addition, it changes the tests to try and access data through the returned references. The new tests fail with the current master implementation of `downcast_ref`, which is correct. Fixes #453
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Bug Report
Version
Description
The following program segfaults:
There is no
unsafe
in this code, so it should at least not segfault. I believe the program should run even correctly (as in, nounwrap
call should fail) given the documented behavior ofdowncast_ref
.The text was updated successfully, but these errors were encountered: