/rust

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

Avoid some bounds checks in binary_heap::{PeekMut,Hole} #58123

Open
wants to merge 1 commit into
base: master
from
+8 −3

Conversation

Reviewers
No reviews
Assignees

@sfackler sfackler

Labels
S-waiting-on-review
Projects
None yet
Milestone
No milestone
3 participants
@lnicola @rust-highfive @sfackler
@lnicola
Copy link
Contributor

lnicola commented Feb 3, 2019

Fixes #58121.

@rust-highfive rust-highfive assigned sfackler Feb 3, 2019

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Feb 3, 2019

r? @sfackler

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

@rust-highfive rust-highfive added the S-waiting-on-review label Feb 3, 2019

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Feb 3, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
travis_time:end:015f1616:start=1549211516335855140,finish=1549211517361644809,duration=1025789669
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:04:43]    Compiling rustc-demangle v0.1.10
[00:04:44] error[E0308]: mismatched types
[00:04:44]    --> src/liballoc/collections/binary_heap.rs:876:36
[00:04:44]     |
[00:04:44] 876 |             elt: ManuallyDrop::new(elt),
[00:04:44]     |                                    ^^^ expected type parameter, found &T
[00:04:44]     = note: expected type `T`
[00:04:44]                found type `&T`
[00:04:44] 
[00:04:46] error: aborting due to previous error
---
[00:04:46] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:04:46] expected success, got: exit code: 101
[00:04:46] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:04:46] Build completed unsuccessfully in 0:00:37
[00:04:46] make: *** [all] Error 1
[00:04:46] Makefile:18: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:099616bc
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sun Feb  3 16:36:54 UTC 2019
---
travis_time:end:101f963a:start=1549211815336947275,finish=1549211815343399426,duration=6452151
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2cdaf37c
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0da015a6
travis_time:start:0da015a6
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:106eb4a1
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@lnicola lnicola force-pushed the lnicola:binary-heap-no-bounds-checks branch from ed675f9 to ca7c8a9 Feb 3, 2019

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Feb 3, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
travis_time:end:15a31ec2:start=1549212458035764223,finish=1549212458959316648,duration=923552425
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:04:38]    Compiling rustc-demangle v0.1.10
[00:04:42] error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
[00:04:42]    --> src/liballoc/collections/binary_heap.rs:262:11
[00:04:42]     |
[00:04:42] 262 |         { self.heap.data.get_unchecked_mut(0) }
[00:04:42]     |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
[00:04:42]     |
[00:04:42]     = note: consult the function's documentation for information on how to avoid undefined behavior
[00:04:42] error: aborting due to previous error
[00:04:42] 
[00:04:42] For more information about this error, try `rustc --explain E0133`.
[00:04:42] error: Could not compile `alloc`.
[00:04:42] error: Could not compile `alloc`.
[00:04:42] 
[00:04:42] To learn more, run the command again with --verbose.
[00:04:42] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:04:42] expected success, got: exit code: 101
[00:04:42] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:04:42] Build completed unsuccessfully in 0:00:42
[00:04:42] Makefile:18: recipe for target 'all' failed
[00:04:42] make: *** [all] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0f08d30b
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sun Feb  3 16:52:32 UTC 2019

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@lnicola lnicola force-pushed the lnicola:binary-heap-no-bounds-checks branch from ca7c8a9 to 1b29a03 Feb 3, 2019

@lnicola lnicola force-pushed the lnicola:binary-heap-no-bounds-checks branch from 1b29a03 to ea72066 Feb 3, 2019

@lnicola

This comment has been minimized.

Sign in to view
Copy link
Contributor

lnicola commented Feb 3, 2019 •
edited

test code

use std::collections::BinaryHeap;

#[no_mangle]
pub extern fn is_opt(heap: &mut BinaryHeap<u32>) -> bool {
    if let Some(mut peek) = heap.peek_mut() {
        *peek = 1;
        true
    } else {
        false
    }
}

before

	.text
	.file	"peekmut.71651wce-cgu.0"
	.section	.text._ZN4core3ptr18real_drop_in_place17hb6242b27b5771784E,"ax",@progbits
	.p2align	4, 0x90
	.type	_ZN4core3ptr18real_drop_in_place17hb6242b27b5771784E,@function
_ZN4core3ptr18real_drop_in_place17hb6242b27b5771784E:
	.cfi_startproc
	pushq	%rax
	.cfi_def_cfa_offset 16
	cmpb	$0, 8(%rdi)
	je	.LBB0_12
	movq	(%rdi), %rax
	movq	16(%rax), %r9
	testq	%r9, %r9
	je	.LBB0_13
	movq	(%rax), %rax
	movl	(%rax), %r8d
	cmpq	$1, %r9
	jne	.LBB0_4
	xorl	%esi, %esi
	jmp	.LBB0_11
.LBB0_4:
	movl	$1, %edi
	xorl	%ecx, %ecx
	.p2align	4, 0x90
.LBB0_5:
	leaq	1(%rdi), %rsi
	cmpq	%r9, %rsi
	jae	.LBB0_8
	movl	(%rax,%rdi,4), %edx
	cmpl	4(%rax,%rdi,4), %edx
	ja	.LBB0_8
	movq	%rsi, %rdi
.LBB0_8:
	movq	%rdi, %rsi
	movl	(%rax,%rdi,4), %edi
	cmpl	%edi, %r8d
	jae	.LBB0_9
	movl	%edi, (%rax,%rcx,4)
	leaq	(%rsi,%rsi), %rdi
	addq	$1, %rdi
	movq	%rsi, %rcx
	cmpq	%r9, %rdi
	jb	.LBB0_5
	jmp	.LBB0_11
.LBB0_9:
	movq	%rcx, %rsi
.LBB0_11:
	movl	%r8d, (%rax,%rsi,4)
.LBB0_12:
	popq	%rax
	.cfi_def_cfa_offset 8
	retq
.LBB0_13:
	.cfi_def_cfa_offset 16
	leaq	.L__unnamed_1(%rip), %rdi
	xorl	%esi, %esi
	xorl	%edx, %edx
	callq	*_ZN4core9panicking18panic_bounds_check17h59684c930baf7d8bE@GOTPCREL(%rip)
	ud2
.Lfunc_end0:
	.size	_ZN4core3ptr18real_drop_in_place17hb6242b27b5771784E, .Lfunc_end0-_ZN4core3ptr18real_drop_in_place17hb6242b27b5771784E
	.cfi_endproc

	.section	.text.is_opt,"ax",@progbits
	.globl	is_opt
	.p2align	4, 0x90
	.type	is_opt,@function
is_opt:
.Lfunc_begin0:
	.cfi_startproc
	.cfi_personality 155, DW.ref.rust_eh_personality
	.cfi_lsda 27, .Lexception0
	subq	$24, %rsp
	.cfi_def_cfa_offset 32
	cmpq	$0, 16(%rdi)
	je	.LBB1_18
	movq	%rdi, 8(%rsp)
	movb	$1, 16(%rsp)
	cmpq	$0, 16(%rdi)
	je	.LBB1_2
	movq	(%rdi), %rax
	movl	$1, (%rax)
	movq	16(%rdi), %r9
	testq	%r9, %r9
	je	.LBB1_6
	movq	(%rdi), %rax
	movl	(%rax), %r8d
	cmpq	$1, %r9
	jne	.LBB1_9
	xorl	%esi, %esi
	jmp	.LBB1_16
.LBB1_18:
	xorl	%ecx, %ecx
	movl	%ecx, %eax
	addq	$24, %rsp
	.cfi_def_cfa_offset 8
	retq
.LBB1_9:
	.cfi_def_cfa_offset 32
	movl	$1, %edi
	xorl	%ecx, %ecx
	.p2align	4, 0x90
.LBB1_10:
	leaq	1(%rdi), %rsi
	cmpq	%r9, %rsi
	jae	.LBB1_13
	movl	(%rax,%rdi,4), %edx
	cmpl	4(%rax,%rdi,4), %edx
	ja	.LBB1_13
	movq	%rsi, %rdi
.LBB1_13:
	movq	%rdi, %rsi
	movl	(%rax,%rdi,4), %edi
	cmpl	%edi, %r8d
	jae	.LBB1_14
	movl	%edi, (%rax,%rcx,4)
	leaq	(%rsi,%rsi), %rdi
	addq	$1, %rdi
	movq	%rsi, %rcx
	cmpq	%r9, %rdi
	jb	.LBB1_10
	jmp	.LBB1_16
.LBB1_14:
	movq	%rcx, %rsi
.LBB1_16:
	movl	%r8d, (%rax,%rsi,4)
	movb	$1, %cl
	movl	%ecx, %eax
	addq	$24, %rsp
	.cfi_def_cfa_offset 8
	retq
.LBB1_2:
	.cfi_def_cfa_offset 32
.Ltmp3:
	leaq	.L__unnamed_2(%rip), %rdi
	xorl	%esi, %esi
	xorl	%edx, %edx
	callq	*_ZN4core9panicking18panic_bounds_check17h59684c930baf7d8bE@GOTPCREL(%rip)
.Ltmp4:
	jmp	.LBB1_3
.LBB1_6:
.Ltmp0:
	leaq	.L__unnamed_1(%rip), %rdi
	xorl	%esi, %esi
	xorl	%edx, %edx
	callq	*_ZN4core9panicking18panic_bounds_check17h59684c930baf7d8bE@GOTPCREL(%rip)
.Ltmp1:
.LBB1_3:
	ud2
.LBB1_4:
.Ltmp2:
	ud2
	ud2
.LBB1_19:
.Ltmp5:
	leaq	8(%rsp), %rdi
	callq	_ZN4core3ptr18real_drop_in_place17hb6242b27b5771784E
	ud2
	ud2
.Lfunc_end1:
	.size	is_opt, .Lfunc_end1-is_opt
	.cfi_endproc
	.section	.gcc_except_table,"a",@progbits
	.p2align	2
GCC_except_table1:
.Lexception0:
	.byte	255
	.byte	255
	.byte	1
	.uleb128 .Lcst_end0-.Lcst_begin0
.Lcst_begin0:
	.uleb128 .Ltmp3-.Lfunc_begin0
	.uleb128 .Ltmp4-.Ltmp3
	.uleb128 .Ltmp5-.Lfunc_begin0
	.byte	0
	.uleb128 .Ltmp0-.Lfunc_begin0
	.uleb128 .Ltmp1-.Ltmp0
	.uleb128 .Ltmp2-.Lfunc_begin0
	.byte	0
	.uleb128 .Ltmp1-.Lfunc_begin0
	.uleb128 .Lfunc_end1-.Ltmp1
	.byte	0
	.byte	0
.Lcst_end0:
	.p2align	2

	.type	str.0,@object
	.section	.rodata.str.0,"a",@progbits
	.p2align	4
str.0:
	.ascii	"/rustc/8a57831a4b7dfa960110599748f3b7382ae28237/src/libcore/slice/mod.rs"
	.size	str.0, 72

	.type	.L__unnamed_2,@object
	.section	.data.rel.ro..L__unnamed_2,"aw",@progbits
	.p2align	3
.L__unnamed_2:
	.quad	str.0
	.quad	72
	.long	2541
	.long	14
	.size	.L__unnamed_2, 24

	.type	str.1,@object
	.section	.rodata.str.1,"a",@progbits
	.p2align	4
str.1:
	.ascii	"/rustc/8a57831a4b7dfa960110599748f3b7382ae28237/src/liballoc/collections/binary_heap.rs"
	.size	str.1, 87

	.type	.L__unnamed_1,@object
	.section	.data.rel.ro..L__unnamed_1,"aw",@progbits
	.p2align	3
.L__unnamed_1:
	.quad	str.1
	.quad	87
	.long	868
	.long	30
	.size	.L__unnamed_1, 24

	.hidden	DW.ref.rust_eh_personality
	.weak	DW.ref.rust_eh_personality
	.section	.data.DW.ref.rust_eh_personality,"aGw",@progbits,DW.ref.rust_eh_personality,comdat
	.p2align	3
	.type	DW.ref.rust_eh_personality,@object
	.size	DW.ref.rust_eh_personality, 8
DW.ref.rust_eh_personality:
	.quad	rust_eh_personality

	.section	".note.GNU-stack","",@progbits

after

	.text
	.file	"peekmut.71651wce-cgu.0"
	.section	.text.is_opt,"ax",@progbits
	.globl	is_opt
	.p2align	4, 0x90
	.type	is_opt,@function
is_opt:
	.cfi_startproc
	cmpq	$0, 16(%rdi)
	je	.LBB0_1
	movq	(%rdi), %rax
	movl	$1, (%rax)
	movq	16(%rdi), %r9
	movb	$1, %al
	cmpq	$2, %r9
	jb	.LBB0_11
	movq	(%rdi), %r10
	movl	(%r10), %r8d
	movl	$1, %esi
	xorl	%edx, %edx
	.p2align	4, 0x90
.LBB0_4:
	leaq	1(%rsi), %rdi
	cmpq	%r9, %rdi
	jae	.LBB0_7
	movl	(%r10,%rsi,4), %ecx
	cmpl	4(%r10,%rsi,4), %ecx
	ja	.LBB0_7
	movq	%rdi, %rsi
.LBB0_7:
	movq	%rsi, %rdi
	movl	(%r10,%rsi,4), %esi
	cmpl	%esi, %r8d
	jae	.LBB0_8
	movl	%esi, (%r10,%rdx,4)
	leaq	(%rdi,%rdi), %rsi
	addq	$1, %rsi
	movq	%rdi, %rdx
	cmpq	%r9, %rsi
	jb	.LBB0_4
	jmp	.LBB0_10
.LBB0_1:
	xorl	%eax, %eax
	retq
.LBB0_8:
	movq	%rdx, %rdi
.LBB0_10:
	movl	%r8d, (%r10,%rdi,4)
.LBB0_11:
	retq
.Lfunc_end0:
	.size	is_opt, .Lfunc_end0-is_opt
	.cfi_endproc


	.section	".note.GNU-stack","",@progbits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment