Skip to content

Use raw pointers to avoid aliasing in str::split_at_mut #31052

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

Merged
merged 1 commit into from
Jan 21, 2016

Conversation

bluss
Copy link
Member

@bluss bluss commented Jan 20, 2016

Use raw pointers to avoid aliasing in str::split_at_mut

Introduce private function from_raw_parts_mut for str to factor out the logic.

We want to use raw pointers here instead of duplicating a &mut str, to
be on safer ground w.r.t rust aliasing rules.

This has already been fixed for slices in PR #27358, issue #27357

@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

@bors: r+ 8d71ca5

bors added a commit that referenced this pull request Jan 21, 2016
Use raw pointers to avoid aliasing in str::split_at_mut

Introduce private functions from_raw_parts, from_raw_parts_mut for str
to factor out the logic.

We want to use raw pointers here instead of duplicating a &mut str, to
be on safer ground w.r.t rust aliasing rules.

This has already been fixed for slices in PR #27358, issue #27357
@bors
Copy link
Collaborator

bors commented Jan 21, 2016

⌛ Testing commit 8d71ca5 with merge ee6a033...

@bors
Copy link
Collaborator

bors commented Jan 21, 2016

💔 Test failed - auto-win-msvc-64-opt

@bluss
Copy link
Member Author

bluss commented Jan 21, 2016

Oh wow, an interesting build failure. I have to suspect that the slice_unchecked() changes are the triggering factors in this error actually.


failures:

---- [run-pass] run-pass/sepcomp-lib-lto.rs stdout ----

error: compilation failed!
status: exit code: 1
command: PATH="x86_64-pc-windows-msvc/stage2/bin;C:\bot\slave\auto-win-msvc-64-opt\build\obj\x86_64-pc-windows-msvc\stage2\bin;C:\bot\slave\auto-win-msvc-64-opt\build\obj\x86_64-pc-windows-msvc\llvm\Release\lib;C:\mingw-w64\x86_64-4.9.1-win32-seh-rt_v3-rev1\mingw64\bin;C:\msys64\mingw64\bin;C:\msys64\usr\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\msys64\usr\bin;C:\python27;C:\python27\scripts;C:\program files (x86)\inno setup 5;C:\program files (x86)\CMake\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit;C:\Program Files\Microsoft SQL Server\110\Tools\Binn;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.0" x86_64-pc-windows-msvc/stage2/bin/rustc.exe C:/bot/slave/auto-win-msvc-64-opt/build/src/test/run-pass/sepcomp-lib-lto.rs -L x86_64-pc-windows-msvc/test/run-pass/ --target=x86_64-pc-windows-msvc -L x86_64-pc-windows-msvc/test/run-pass\sepcomp-lib-lto.stage2-x86_64-pc-windows-msvc.run-pass.libaux -o x86_64-pc-windows-msvc/test/run-pass\sepcomp-lib-lto.stage2-x86_64-pc-windows-msvc.exe --cfg rtopt -C rpath -O -L x86_64-pc-windows-msvc/rt -C lto
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
Instruction does not dominate all uses!
  %oldret10 = insertvalue %str_slice %oldret8, i64 %1209, 1
  %.sink.i.i.i = phi %str_slice [ %oldret10, %"_ZN3str6traits45str.ops..Index$LT$ops..Range$LT$usize$GT$$GT$5index20h11b34fcbe1b5d5d0JVSE.exit.i.i.i" ], [ %oldret22, %"_ZN3str6traits45str.ops..Index$LT$ops..Range$LT$usize$GT$$GT$5index20h11b34fcbe1b5d5d0JVSE.exit539.i.i.i" ]
Instruction does not dominate all uses!
  %oldret22 = insertvalue %str_slice %oldret20, i64 %1223, 1
  %.sink.i.i.i = phi %str_slice [ %oldret10, %"_ZN3str6traits45str.ops..Index$LT$ops..Range$LT$usize$GT$$GT$5index20h11b34fcbe1b5d5d0JVSE.exit.i.i.i" ], [ %oldret22, %"_ZN3str6traits45str.ops..Index$LT$ops..Range$LT$usize$GT$$GT$5index20h11b34fcbe1b5d5d0JVSE.exit539.i.i.i" ]
Instruction does not dominate all uses!
  %1213 = getelementptr inbounds i8, i8* %1194, i64 2
  %oldret20 = insertvalue %str_slice undef, i8* %1213, 0
Instruction does not dominate all uses!
  %1223 = add i64 %1195, -3
  %oldret22 = insertvalue %str_slice %oldret20, i64 %1223, 1
Instruction does not dominate all uses!
  %1199 = getelementptr inbounds i8, i8* %1194, i64 3
  %oldret8 = insertvalue %str_slice undef, i8* %1199, 0
Instruction does not dominate all uses!
  %1209 = add i64 %1195, -4
  %oldret10 = insertvalue %str_slice %oldret8, i64 %1209, 1
LLVM ERROR: Broken function found, compilation aborted!

------------------------------------------

thread '[run-pass] run-pass/sepcomp-lib-lto.rs' panicked at 'explicit panic', C:/bot/slave/auto-win-msvc-64-opt/build/src/compiletest\runtest.rs:1505


failures:
    [run-pass] run-pass/sepcomp-lib-lto.rs

Introduce private function from_raw_parts_mut for str to factor out the logic.

We want to use raw pointers here instead of duplicating a &mut str, to
be on safer ground w.r.t rust aliasing rules.
@bluss
Copy link
Member Author

bluss commented Jan 21, 2016

PR revision: I removed the changes to slice_unchecked, slice_unchecked_mut.

/// str is returned.
///
unsafe fn from_raw_parts_mut<'a>(p: *mut u8, len: usize) -> &'a mut str {
mem::transmute::<&mut [u8], &mut str>(slice::from_raw_parts_mut(p, len))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transmute -> from_utf8_unchecked?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a _mut version. I could add that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I? Always quicker to avoid having to champion new public API for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would personally prefer to stabilize from_utf8_unchecked_mut than from_raw_parts_mut, but I don't think we necessarily need the function just yet so I would be fine just leaving this as -is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good plan for the future.

@bluss
Copy link
Member Author

bluss commented Jan 21, 2016

The commit how it looked when causing the interesting error https://gist.github.com/bluss/0f0f90c0b5b7ec5789d4

Edit: Wrong version. fixed.

@alexcrichton
Copy link
Member

@bors: r+ ba9a3bc

@bors
Copy link
Collaborator

bors commented Jan 21, 2016

⌛ Testing commit ba9a3bc with merge 46dcffd...

bors added a commit that referenced this pull request Jan 21, 2016
Use raw pointers to avoid aliasing in str::split_at_mut

Introduce private function  from_raw_parts_mut for str to factor out the logic.

We want to use raw pointers here instead of duplicating a &mut str, to
be on safer ground w.r.t rust aliasing rules.

This has already been fixed for slices in PR #27358, issue #27357
@bors bors merged commit ba9a3bc into rust-lang:master Jan 21, 2016
@bluss bluss deleted the split-at-mut-str branch January 21, 2016 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants