Skip to content

Commit

Permalink
Fix __divsi3 and __udivsi3 on thumbv6m targets
Browse files Browse the repository at this point in the history
This commit fixes a bug accidentally introduced in rust-lang#285 where some
lingering references remained to `#[cfg(thumbv6m)]` but this, since the
historical revert, was renamed to `#[cfg(thumb_1)]`. This caused on the
thumbv6m platform for the intrinsics to be accidentally omitted because
the build script didn't actually compile them but the Rust code thought
the C code was in use.

After correcting the `#[cfg]` statements the CI configuration for the
`thumb*` family of targets was all updated. The support for xargo
testing was removed from `run.sh` since it had long since bitrotted, and
the script was updated to simply build the intrinsics example to attempt
to link for each of these targets. This in turn exposed the bug locally
and allowed to confirm a fix once the `#[cfg]` statements were
corrected.

cc rust-lang/rust#60782
  • Loading branch information
alexcrichton committed May 14, 2019
1 parent e3b53f9 commit 8521530
Show file tree
Hide file tree
Showing 15 changed files with 64 additions and 95 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ rustc-dep-of-std = ['c', 'compiler-builtins', 'core']

[[example]]
name = "intrinsics"
required-features = ["c", "compiler-builtins"]
required-features = ["compiler-builtins"]

[workspace]
members = ["testcrate"]
Expand Down
20 changes: 8 additions & 12 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,14 @@ jobs:
TARGET: powerpc64-unknown-linux-gnu
powerpc64le:
TARGET: powerpc64le-unknown-linux-gnu
# thumbv6m:
# TARGET: thumbv6m-linux-eabi
# XARGO: 1
# thumbv7em:
# TARGET: thumbv7em-linux-eabi
# XARGO: 1
# thumbv7emhf:
# TARGET: thumbv7em-linux-eabihf
# XARGO: 1
# thumbv7m:
# TARGET: thumbv7m-linux-eabi
# XARGO: 1
thumbv6m:
TARGET: thumbv6m-none-eabi
thumbv7em:
TARGET: thumbv7em-none-eabi
thumbv7emhf:
TARGET: thumbv7em-none-eabihf
thumbv7m:
TARGET: thumbv7m-none-eabi
wasm32:
TARGET: wasm32-unknown-unknown
ONLY_BUILD: 1
Expand Down
10 changes: 0 additions & 10 deletions ci/docker/thumbv6m-linux-eabi/Dockerfile

This file was deleted.

7 changes: 7 additions & 0 deletions ci/docker/thumbv6m-none-eabi/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM ubuntu:18.04
RUN apt-get update && \
apt-get install -y --no-install-recommends \
gcc libc6-dev ca-certificates \
gcc-arm-none-eabi \
libnewlib-arm-none-eabi
ENV XARGO=1
10 changes: 0 additions & 10 deletions ci/docker/thumbv7em-linux-eabi/Dockerfile

This file was deleted.

10 changes: 0 additions & 10 deletions ci/docker/thumbv7em-linux-eabihf/Dockerfile

This file was deleted.

7 changes: 7 additions & 0 deletions ci/docker/thumbv7em-none-eabi/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM ubuntu:18.04
RUN apt-get update && \
apt-get install -y --no-install-recommends \
gcc libc6-dev ca-certificates \
gcc-arm-none-eabi \
libnewlib-arm-none-eabi
ENV XARGO=1
7 changes: 7 additions & 0 deletions ci/docker/thumbv7em-none-eabihf/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM ubuntu:18.04
RUN apt-get update && \
apt-get install -y --no-install-recommends \
gcc libc6-dev ca-certificates \
gcc-arm-none-eabi \
libnewlib-arm-none-eabi
ENV XARGO=1
10 changes: 0 additions & 10 deletions ci/docker/thumbv7m-linux-eabi/Dockerfile

This file was deleted.

7 changes: 7 additions & 0 deletions ci/docker/thumbv7m-none-eabi/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM ubuntu:18.04
RUN apt-get update && \
apt-get install -y --no-install-recommends \
gcc libc6-dev ca-certificates \
gcc-arm-none-eabi \
libnewlib-arm-none-eabi
ENV XARGO=1
1 change: 0 additions & 1 deletion ci/run-docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ run() {
--user $(id -u):$(id -g) \
-e CARGO_HOME=/cargo \
-e CARGO_TARGET_DIR=/target \
-e XARGO \
-v $HOME/.cargo:/cargo \
-v `pwd`/target:/target \
-v `pwd`:/checkout:ro \
Expand Down
50 changes: 12 additions & 38 deletions ci/run.sh
Original file line number Diff line number Diff line change
@@ -1,53 +1,24 @@
set -ex

# FIXME(japarix/xargo#186) this shouldn't be necessary
export RUST_TARGET_PATH=`pwd`

cargo=cargo
if [ "$XARGO" = "1" ]; then
cargo=xargo
fi

INTRINSICS_FEATURES="c"

# Some architectures like ARM apparently seem to require the `mem` feature
# enabled to successfully compile the `intrinsics` example, and... we're not
# sure why!
if [ -z "$INTRINSICS_FAILS_WITH_MEM_FEATURE" ]; then
INTRINSICS_FEATURES="$INTRINSICS_FEATURES mem"
fi

# Test our implementation
if [ "$XARGO" = "1" ]; then
run="xargo test --manifest-path testcrate/Cargo.toml --target $1"
for t in $(ls testcrate/tests); do
t=${t%.rs}

RUSTFLAGS="-C debug-assertions=no -C lto" \
CARGO_INCREMENTAL=0 \
$run --test $t --no-default-features --features 'mem c' --no-run
qemu-arm-static target/${1}/debug/$t-*
done

for t in $(ls testcrate/tests); do
t=${t%.rs}
RUSTFLAGS="-C lto" \
CARGO_INCREMENTAL=0 \
$run --test $t --no-default-features --features 'mem c' --no-run --release
qemu-arm-static target/${1}/release/$t-*
done
# FIXME: currently these tests don't work...
echo nothing to do
else
run="cargo test --manifest-path testcrate/Cargo.toml --target $1"
$run
$run --release
$run --features c
$run --features c --release
cargo build --target $1
cargo build --target $1 --release
cargo build --target $1 --features c
cargo build --target $1 --release --features c
fi

cargo build --target $1
cargo build --target $1 --release
cargo build --target $1 --features c
cargo build --target $1 --release --features c

PREFIX=$(echo $1 | sed -e 's/unknown-//')-
case $1 in
armv7-*)
Expand Down Expand Up @@ -101,8 +72,11 @@ done
rm -f $path

# Verify that we haven't drop any intrinsic/symbol
RUSTFLAGS="-C debug-assertions=no" \
$cargo build --features "$INTRINSICS_FEATURES" --target $1 --example intrinsics -v
build_intrinsics="$cargo build --target $1 -v --example intrinsics"
RUSTFLAGS="-C debug-assertions=no" $build_intrinsics
RUSTFLAGS="-C debug-assertions=no" $build_intrinsics --release
RUSTFLAGS="-C debug-assertions=no" $build_intrinsics --features c
RUSTFLAGS="-C debug-assertions=no" $build_intrinsics --features c --release

# Verify that there are no undefined symbols to `panic` within our
# implementations
Expand Down
14 changes: 13 additions & 1 deletion examples/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,14 @@ extern {}
mod intrinsics {
// trunccdfsf2
pub fn aeabi_d2f(x: f64) -> f32 {
x as f32
// This is only implemented in C currently, so only test it there.
#[cfg(feature = "c")]
return x as f32;
#[cfg(not(feature = "c"))]
{
drop(x);
0.0
}
}

// fixdfsi
Expand Down Expand Up @@ -263,6 +270,10 @@ mod intrinsics {
pub fn modti3(a: i128, b: i128) -> i128 {
a % b
}

pub fn udivsi3(a: u32, b: u32) -> u32 {
a / b
}
}

fn run() {
Expand Down Expand Up @@ -325,6 +336,7 @@ fn run() {
bb(umodti3(bb(2), bb(2)));
bb(divti3(bb(2), bb(2)));
bb(modti3(bb(2), bb(2)));
bb(udivsi3(bb(2), bb(2)));

something_with_a_dtor(&|| assert_eq!(bb(1), 1));

Expand Down
2 changes: 1 addition & 1 deletion src/int/sdiv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Divmod for i32 {}
impl Divmod for i64 {}

intrinsics! {
#[use_c_shim_if(all(target_arch = "arm", not(target_os = "ios"), not(thumbv6m)))]
#[use_c_shim_if(all(target_arch = "arm", not(target_os = "ios"), not(thumb_1)))]
#[arm_aeabi_alias = __aeabi_idiv]
pub extern "C" fn __divsi3(a: i32, b: i32) -> i32 {
a.div(b)
Expand Down
2 changes: 1 addition & 1 deletion src/int/udiv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ macro_rules! udivmod_inner {
intrinsics! {
#[use_c_shim_if(all(target_arch = "arm",
not(target_os = "ios"),
not(thumbv6m)))]
not(thumb_1)))]
#[arm_aeabi_alias = __aeabi_uidiv]
/// Returns `n / d`
pub extern "C" fn __udivsi3(n: u32, d: u32) -> u32 {
Expand Down

0 comments on commit 8521530

Please sign in to comment.