Correct bitv build failures and a bug in all() #1

cuviper · 2014-12-10T06:55:30Z

These are the changes I mentioned on rust-lang#19216.

- Fix typos on Blocks and MutBlocks. - Use slice_to_mut() for creating blocks_mut(). - Deref the block parameter in get(). - Access nbits separately from mutating set in pop().

The old logic would be ok with *either* 0 or all 1s in the last word, because it didn't compute a proper mask for the case where nbits is an exact multiple of u32::BITS. Add mask_for_bits() to compute this properly, and use it in all(). Add all/none assertions to most of the tests. Note in particular, the all-zero bitv in test_32_elements() was incorrectly all()==true before this patch.

cuviper · 2014-12-10T07:00:15Z

FWIW, I also looked at the merge conflicts and pushed a bitv-merged branch. I think you're supposed to rebase for PRs to rust proper, but it may be helpful to look at my merge commit to see where things collided.

Gankra · 2014-12-10T13:58:06Z

Thanks! Yes I haven't been dealing with merge conflicts because I need /rust-lang/rfcs/pull/509 to be accepted first.

Gankra · 2014-12-10T13:59:09Z

src/libcollections/bit.rs

@@ -199,7 +205,7 @@ impl Bitv {
    /// Iterator over mutable refs to  the underlying blocks of data.
    fn blocks_mut(&mut self) -> MutBlocks {
        let blocks = blocks_for_bits(self.len());
-        self.storage[..blocks].iter_mut()
+        self.storage.slice_to_mut(blocks).iter_mut()


I believe this can be [mut ..blocks]

But this is basically fine

Ah, I wasn't aware of that syntax, thanks!

Correct bitv build failures and a bug in all()

Fixes rust-lang#19707. In terms of output, it currently uses the form `argument #1`, `argument #2`, etc. If anyone has any better suggestions I would be glad to consider them.

fix double "however"

…hange Change verbiage in Stack & Heap page

Without that flag, LLVM generates unaligned memory access instructions, which are not allowed on ARMv5. For example, the 'hello world' example from `cargo --new` failed with: ``` $ ./hello Hello, world! thread 'main' panicked at 'assertion failed: end <= len', src/libcollections/vec.rs:1113 note: Run with `RUST_BACKTRACE=1` for a backtrace. ``` I traced this error back to the following assembler code in `BufWriter::flush_buf`: ``` 6f44: e28d0018 add r0, sp, rust-lang#24 [...] 6f54: e280b005 add fp, r0, #5 [...] 7018: e5cd001c strb r0, [sp, rust-lang#28] 701c: e1a0082a lsr r0, sl, rust-lang#16 7020: 03a01001 moveq r1, #1 7024: e5cb0002 strb r0, [fp, #2] 7028: e1cba0b0 strh sl, [fp] ``` Note that `fp` points to `sp + 29`, so the three `str*`-instructions should fill up a 32bit - value at `sp + 28`, which is later used as the value `n` in `Ok(n) => written += n`. This doesn't work on ARMv5 as the `strh` can't write to the unaligned contents of `fp`, so the upper bits of `n` won't get cleared, leading to the assertion failure in Vec::drain. With `+strict-align`, the code works as expected.

ARMv5 needs +strict-align Without that flag, LLVM generates unaligned memory access instructions, which are not allowed on ARMv5. For example, the 'hello world' example from `cargo --new` failed with: ``` $ ./hello Hello, world! thread 'main' panicked at 'assertion failed: end <= len', src/libcollections/vec.rs:1113 note: Run with `RUST_BACKTRACE=1` for a backtrace. ``` I traced this error back to the following assembler code in `BufWriter::flush_buf`: ``` 6f44: e28d0018 add r0, sp, rust-lang#24 [...] 6f54: e280b005 add fp, r0, #5 [...] 7018: e5cd001c strb r0, [sp, rust-lang#28] 701c: e1a0082a lsr r0, sl, rust-lang#16 7020: 03a01001 moveq r1, #1 7024: e5cb0002 strb r0, [fp, #2] 7028: e1cba0b0 strh sl, [fp] ``` Note that `fp` points to `sp + 29`, so the three `str*`-instructions should fill up a 32bit - value at `sp + 28`, which is later used as the value `n` in `Ok(n) => written += n`. This doesn't work on ARMv5 as the `strh` can't write to the unaligned contents of `fp`, so the upper bits of `n` won't get cleared, leading to the assertion failure in Vec::drain. With `+strict-align`, the code works as expected.

…crichton Allow atomic operations up to 32 bits The ARMv5te platform does not have instruction-level support for atomics, however the kernel provides [user space helpers] which can be used to perform atomic operations. When linked with `libgcc`, the atomic symbols needed by Rust will be provided, rather than CPU level intrinsics. [user space helpers]: https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt 32-bit versions of these kernel level helpers were introduced in Linux Kernel 2.6.12, and 64-bit version of these kernel level helpers were introduced in Linux Kernel 3.1. I have selected 32 bit versions as std currently only requires Linux version 2.6.18 and above as far as I am aware. As this target is specifically linux and gnueabi, it is reasonable to assume the Linux Kernel and libc will be available for the target. There is a large performance penalty, as we are not using CPU level intrinsics, however this penalty is likely preferable to not having the target at all. I have used this change in a custom target (along with xargo) to build std, as well as a number of higher level crates. ## Additional information For reference, here is what a a code snippet decompiles to: ```rust use std::sync::atomic::{AtomicIsize, Ordering}; #[no_mangle] pub extern fn foo(a: &AtomicIsize) -> isize { a.fetch_add(1, Ordering::SeqCst) } ``` ``` Disassembly of section .text.foo: 00000000 <foo>: 0: e92d4800 push {fp, lr} 4: e3a01001 mov r1, #1 8: ebfffffe bl 0 <__sync_fetch_and_add_4> c: e8bd8800 pop {fp, pc} ``` Which in turn is provided by `libgcc.a`, which has code which looks like this: ``` Disassembly of section .text: 00000000 <__sync_fetch_and_add_4>: 0: e92d40f8 push {r3, r4, r5, r6, r7, lr} 4: e1a05000 mov r5, r0 8: e1a07001 mov r7, r1 c: e59f6028 ldr r6, [pc, rust-lang#40] ; 3c <__sync_fetch_and_add_4+0x3c> 10: e5954000 ldr r4, [r5] 14: e1a02005 mov r2, r5 18: e1a00004 mov r0, r4 1c: e0841007 add r1, r4, r7 20: e1a0e00f mov lr, pc 24: e12fff16 bx r6 28: e3500000 cmp r0, #0 2c: 1afffff7 bne 10 <__sync_fetch_and_add_4+0x10> 30: e1a00004 mov r0, r4 34: e8bd40f8 pop {r3, r4, r5, r6, r7, lr} 38: e12fff1e bx lr 3c: ffff0fc0 .word 0xffff0fc0 ``` Where you can see the reference to `0xffff0fc0`, which is provided by the [user space helpers].

Merge upstream changes

Building for x86_64-unknown-linux-musl currently results in an executable lacking debug information for musl libc itself. If you request a backtrace in GDB while control flow is within musl – including sycalls made by musl – the result looks like: #0 0x0000000000434b46 in __cp_end () #1 0x0000000000432dbd in __syscall_cp_c () #2 0x0000000000000000 in ?? () i.e. not very helpful. Adding --enable-debug resolves this, and --enable-optimize re-enables optimisations which default to off given the previous flag.

Add --enable-debug flag to musl CI build script Building for x86_64-unknown-linux-musl currently results in an executable lacking debug information for musl libc itself. If you request a backtrace in GDB while control flow is within musl – including sycalls made by musl – the result looks like: ``` #0 0x0000000000434b46 in __cp_end () #1 0x0000000000432dbd in __syscall_cp_c () #2 0x0000000000000000 in ?? () ``` i.e. not very helpful. Adding --enable-debug resolves this, and --enable-optimize re-enables optimisations which default to off given the previous flag.

cuviper added 3 commits December 9, 2014 21:47

bitv: correct build failures

ab734ca

- Fix typos on Blocks and MutBlocks. - Use slice_to_mut() for creating blocks_mut(). - Deref the block parameter in get(). - Access nbits separately from mutating set in pop().

bitv: reuse mask_for_bits() in grow()

0f70e1a

Gankra reviewed Dec 10, 2014
View reviewed changes

Gankra added a commit that referenced this pull request Dec 10, 2014

Merge pull request #1 from cuviper/bitv

6fe16b7

Correct bitv build failures and a bug in all()

Gankra merged commit 6fe16b7 into Gankra:bitv Dec 10, 2014

Gankra added a commit that referenced this pull request Jul 14, 2015

Merge pull request #1 from Manishearth/patch-1

9997a6e

fix double "however"

Gankra pushed a commit that referenced this pull request Dec 10, 2015

Merge pull request #1 from bhargavrpatel/master-stack-heap-verbiage-c…

968adb2

…hange Change verbiage in Stack & Heap page

Gankra pushed a commit that referenced this pull request May 9, 2018

Merge pull request #1 from rust-lang/master

5e94d54

Merge upstream changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct bitv build failures and a bug in all() #1

Correct bitv build failures and a bug in all() #1

cuviper commented Dec 10, 2014

cuviper commented Dec 10, 2014

Gankra commented Dec 10, 2014

Gankra Dec 10, 2014

Gankra Dec 10, 2014

cuviper Dec 10, 2014

Correct bitv build failures and a bug in all() #1

Correct bitv build failures and a bug in all() #1

Conversation

cuviper commented Dec 10, 2014

cuviper commented Dec 10, 2014

Gankra commented Dec 10, 2014

Gankra Dec 10, 2014

Choose a reason for hiding this comment

Gankra Dec 10, 2014

Choose a reason for hiding this comment

cuviper Dec 10, 2014

Choose a reason for hiding this comment