Skip to content

Commit

Permalink
Emit range metadata on calls returning scalars (fixes #50157)
Browse files Browse the repository at this point in the history
  • Loading branch information
nox committed Apr 26, 2018
1 parent 949010d commit 9065644
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 21 deletions.
19 changes: 18 additions & 1 deletion src/librustc_target/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use self::Primitive::*;
use spec::Target;

use std::cmp;
use std::ops::{Add, Deref, Sub, Mul, AddAssign, RangeInclusive};
use std::ops::{Add, Deref, Sub, Mul, AddAssign, Range, RangeInclusive};

pub mod call;

Expand Down Expand Up @@ -544,6 +544,23 @@ impl Scalar {
false
}
}

/// Returns the valid range as a `x..y` range.
///
/// If `x` and `y` are equal, the range is full, not empty.
pub fn valid_range_exclusive<C: HasDataLayout>(&self, cx: C) -> Range<u128> {
// For a (max) value of -1, max will be `-1 as usize`, which overflows.
// However, that is fine here (it would still represent the full range),
// i.e., if the range is everything.
let bits = self.value.size(cx).bits();
assert!(bits <= 128);
let mask = !0u128 >> (128 - bits);
let start = self.valid_range.start;
let end = self.valid_range.end;
assert_eq!(start, start & mask);
assert_eq!(end, end & mask);
start..(end.wrapping_add(1) & mask)
}
}

/// Describes how the fields of a type are located in memory.
Expand Down
1 change: 1 addition & 0 deletions src/librustc_target/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#![feature(const_fn)]
#![feature(fs_read_write)]
#![feature(inclusive_range)]
#![feature(inclusive_range_fields)]
#![feature(slice_patterns)]

#[macro_use]
Expand Down
22 changes: 20 additions & 2 deletions src/librustc_trans/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ pub trait FnTypeExt<'a, 'tcx> {
fn llvm_type(&self, cx: &CodegenCx<'a, 'tcx>) -> Type;
fn llvm_cconv(&self) -> llvm::CallConv;
fn apply_attrs_llfn(&self, llfn: ValueRef);
fn apply_attrs_callsite(&self, callsite: ValueRef);
fn apply_attrs_callsite(&self, bx: &Builder<'a, 'tcx>, callsite: ValueRef);
}

impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
Expand Down Expand Up @@ -640,7 +640,7 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
}
}

fn apply_attrs_callsite(&self, callsite: ValueRef) {
fn apply_attrs_callsite(&self, bx: &Builder<'a, 'tcx>, callsite: ValueRef) {
let mut i = 0;
let mut apply = |attrs: &ArgAttributes| {
attrs.apply_callsite(llvm::AttributePlace::Argument(i), callsite);
Expand All @@ -653,6 +653,24 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
PassMode::Indirect(ref attrs) => apply(attrs),
_ => {}
}
if let layout::Abi::Scalar(ref scalar) = self.ret.layout.abi {
// If the value is a boolean, the range is 0..2 and that ultimately
// become 0..0 when the type becomes i1, which would be rejected
// by the LLVM verifier.
match scalar.value {
layout::Int(..) if !scalar.is_bool() => {
let range = scalar.valid_range_exclusive(bx.cx);
if range.start != range.end {
// FIXME(nox): This causes very weird type errors about
// SHL operators in constants in stage 2 with LLVM 3.9.
if unsafe { llvm::LLVMRustVersionMajor() >= 4 } {
bx.range_metadata(callsite, range);
}
}
}
_ => {}
}
}
for arg in &self.args {
if arg.pad.is_some() {
apply(&ArgAttributes::new());
Expand Down
1 change: 1 addition & 0 deletions src/librustc_trans/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#![allow(unused_attributes)]
#![feature(libc)]
#![feature(quote)]
#![feature(range_contains)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
#![feature(optin_builtin_traits)]
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_trans/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
ret_bx,
llblock(this, cleanup),
cleanup_bundle);
fn_ty.apply_attrs_callsite(invokeret);
fn_ty.apply_attrs_callsite(&bx, invokeret);

if let Some((ret_dest, target)) = destination {
let ret_bx = this.build_block(target);
Expand All @@ -136,7 +136,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
}
} else {
let llret = bx.call(fn_ptr, &llargs, cleanup_bundle);
fn_ty.apply_attrs_callsite(llret);
fn_ty.apply_attrs_callsite(&bx, llret);
if this.mir[bb].is_cleanup {
// Cleanup is always the cold path. Don't inline
// drop glue. Also, when there is a deeply-nested
Expand Down
23 changes: 7 additions & 16 deletions src/librustc_trans/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,24 +91,15 @@ impl<'a, 'tcx> PlaceRef<'tcx> {
}

let scalar_load_metadata = |load, scalar: &layout::Scalar| {
let (min, max) = (scalar.valid_range.start, scalar.valid_range.end);
let max_next = max.wrapping_add(1);
let bits = scalar.value.size(bx.cx).bits();
assert!(bits <= 128);
let mask = !0u128 >> (128 - bits);
// For a (max) value of -1, max will be `-1 as usize`, which overflows.
// However, that is fine here (it would still represent the full range),
// i.e., if the range is everything. The lo==hi case would be
// rejected by the LLVM verifier (it would mean either an
// empty set, which is impossible, or the entire range of the
// type, which is pointless).
let vr = scalar.valid_range.clone();
match scalar.value {
layout::Int(..) if max_next & mask != min & mask => {
// llvm::ConstantRange can deal with ranges that wrap around,
// so an overflow on (max + 1) is fine.
bx.range_metadata(load, min..max_next);
layout::Int(..) => {
let range = scalar.valid_range_exclusive(bx.cx);
if range.start != range.end {
bx.range_metadata(load, range);
}
}
layout::Pointer if 0 < min && min < max => {
layout::Pointer if vr.start < vr.end && !vr.contains(&0) => {
bx.nonnull_metadata(load);
}
_ => {}
Expand Down
29 changes: 29 additions & 0 deletions src/test/codegen/call-metadata.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Checks that range metadata gets emitted on calls to functions returning a
// scalar value.

// compile-flags: -C no-prepopulate-passes
// min-llvm-version 4.0


#![crate_type = "lib"]

pub fn test() {
// CHECK: call i8 @some_true(), !range [[R0:![0-9]+]]
// CHECK: [[R0]] = !{i8 0, i8 3}
some_true();
}

#[no_mangle]
fn some_true() -> Option<bool> {
Some(true)
}

0 comments on commit 9065644

Please sign in to comment.