Skip to content
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

RefCell::borrow_mut codegen broken on ARM #40738

Closed
krdln opened this issue Mar 22, 2017 · 4 comments
Closed

RefCell::borrow_mut codegen broken on ARM #40738

krdln opened this issue Mar 22, 2017 · 4 comments

Comments

@krdln
Copy link
Contributor

krdln commented Mar 22, 2017

I was trying to compile some programs on my armv7l machine and noticed that most programs using regex crate do panic. I've managed to narrow down the issue to a RefCell::borrow_mut call.

Test program:

use std::cell::RefCell;
fn main() {
    let cell = RefCell::new(1);
    cell.borrow_mut();
    cell.borrow_mut();
}

Versions of Rust tested (installed using rustup, the exact toolchain is nightly-armv7-unknown-linux-gnueabihf, etc.):

> rustc +stable --version
rustc 1.16.0 (30cf806ef 2017-03-10)
> rustc +beta --version
rustc 1.17.0-beta.2 (b7c276653 2017-03-20)
> rustc +nightly --version
rustc 1.17.0-nightly (cab4bff3d 2017-03-21)

This program works correctly on stable, but on beta and nightly fails with the following backtrace:

thread 'main' panicked at 'already borrowed: BorrowMutError', /checkout/src/libcore/result.rs:859
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: <core::result::Result<T, E>>::expect
             at /checkout/src/libcore/result.rs:761
   1: <core::cell::RefCell<T>>::borrow_mut
             at /checkout/src/libcore/cell.rs:736
   2: bm_test::main
             at ./bm_test.rs:5

What's quite interesting is that the error is not present when cross-compiling from x86-64 to arm.

I've checked the generated assembly, and found a suspicious difference. For this function:

pub fn bm_once(cell: &RefCell<i32>) {
    cell.borrow_mut();
}

the generated assembly on stable (rustc -O) (or when cross-compiling using any version) is:

	.section	.text._ZN9borrowmut7bm_once17h7c1ef392d62ede16E,"ax",%progbits
	.globl	borrowmut::bm_once
	.p2align	2
	.type	borrowmut::bm_once,%function
borrowmut::bm_once:
	.fnstart
	ldr	r1, [r0]
	cmp	r1, #0
	moveq	r1, #0     // on beta: mvneq r1, #0
	streq	r1, [r0]
	bxeq	lr
	.save	{r11, lr}
	push	{r11, lr}
	bl	core::result::unwrap_failed
.Lfunc_end2:
	.size	borrowmut::bm_once, .Lfunc_end2-borrowmut::bm_once
	.fnend

The wrong assembly for beta and nightly differs only by one instruction – mvneq instead of moveq.

@krdln
Copy link
Contributor Author

krdln commented Mar 22, 2017

llvm ir for the bm_once function for rustc +stable -O:

define void @_ZN9borrowmut7bm_once17h7c1ef392d62ede16E(%"core::cell::RefCell<i32>"* dereferenceable(8)) unnamed_addr #1 personality i32 (i32, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
entry-block:
  %1 = getelementptr inbounds %"core::cell::RefCell<i32>", %"core::cell::RefCell<i32>"* %0, i32 0, i32 0, i32 0, i32 0
  %2 = load i32, i32* %1, align 4, !noalias !8
  %cond.i.i.i = icmp eq i32 %2, 0
  br i1 %cond.i.i.i, label %"_ZN37_$LT$core..cell..RefCell$LT$T$GT$$GT$10borrow_mut17hd3c0713653e53ffbE.exit", label %bb4.i5.i

bb4.i5.i:                                         ; preds = %entry-block
  tail call fastcc void @_ZN4core6result13unwrap_failed17hd254683cc02cf089E(), !noalias !13
  unreachable

"_ZN37_$LT$core..cell..RefCell$LT$T$GT$$GT$10borrow_mut17hd3c0713653e53ffbE.exit": ; preds = %entry-block
  store i32 0, i32* %1, align 4
  ret void
}

On beta, the last store is different (the WRITING flag is written despite it's the end of the function):

store i32 -1, i32* %1, align 4, !noalias !8

@petrochenkov
Copy link
Contributor

#40593?

@arielb1
Copy link
Contributor

arielb1 commented Mar 22, 2017

Looks like it.

@krdln
Copy link
Contributor Author

krdln commented Mar 22, 2017

@petrochenkov Seems like the same bug, thanks! Somehow I missed that issue when searching, sorry for dupe.

@krdln krdln closed this as completed Mar 22, 2017
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

No branches or pull requests

3 participants