Fix undefined behaviour in LinkedPerCore::get. - #51
Conversation
|
Is there a way to create a test that triggers the issue? Maybe with running it with Miri? |
I don't think so, as that would require running the test with a linker script for the |
| (&raw const self.0) | ||
| .expose_provenance() | ||
| .cast_signed() | ||
| .checked_add(percore_local_offset()) |
There was a problem hiding this comment.
This adds runtime overhead for each percore access.
It checks for integer overflow and NULL pointer. Based on the safety requirements neither of this should happen.
The safety requirements of the PercoreLocalOffset trait should make these unneccessary.
| // `byte_offset` on the pointer to `self.0` because the per-core copy is not part of the | ||
| // same allocation. | ||
| let percore_ptr = with_exposed_provenance::<T>( | ||
| ((&raw const self.0).expose_provenance().cast_signed() + percore_local_offset()) |
There was a problem hiding this comment.
Let me know if I'm wrong, but according to the corresponding section in Rust docs this seems incorrect.
- expose_provenance [...] adds the provenance of the pointer to a global list of 'exposed' provenances [...]
- with_exposed_provenance can be used to construct a pointer with one of these previously
'exposed' provenances
In this line we "save" the provenance of the primary core's pointer, and in the previous line create a new pointer by "retrieving" the saved provenance, but that still only describes the primary core's memory area.
I'm not sure if it's possible, but we should rather save the provenance of the pointer that was passed to percore_copy_secondary_data() and use that when constructing the secondary core's pointer here.
3 participants
Fixes #48.