From 90656441a9dfbc42b6f5f1f25abeead66012bb00 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 22 Apr 2018 18:40:54 +0200 Subject: [PATCH] Emit range metadata on calls returning scalars (fixes #50157) --- src/librustc_target/abi/mod.rs | 19 ++++++++++++++++++- src/librustc_target/lib.rs | 1 + src/librustc_trans/abi.rs | 22 ++++++++++++++++++++-- src/librustc_trans/lib.rs | 1 + src/librustc_trans/mir/block.rs | 4 ++-- src/librustc_trans/mir/place.rs | 23 +++++++---------------- src/test/codegen/call-metadata.rs | 29 +++++++++++++++++++++++++++++ 7 files changed, 78 insertions(+), 21 deletions(-) create mode 100644 src/test/codegen/call-metadata.rs diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 18dd04c0ee867..346e5667a7bab 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -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; @@ -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(&self, cx: C) -> Range { + // 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. diff --git a/src/librustc_target/lib.rs b/src/librustc_target/lib.rs index 8f4911574398b..927d5c7e15a20 100644 --- a/src/librustc_target/lib.rs +++ b/src/librustc_target/lib.rs @@ -29,6 +29,7 @@ #![feature(const_fn)] #![feature(fs_read_write)] #![feature(inclusive_range)] +#![feature(inclusive_range_fields)] #![feature(slice_patterns)] #[macro_use] diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs index 483f36afe27d6..1d0d7ec601f1f 100644 --- a/src/librustc_trans/abi.rs +++ b/src/librustc_trans/abi.rs @@ -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>> { @@ -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); @@ -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()); diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index dab01abd3353a..96a10e8b99d32 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -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)] diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 36f03605feabd..b9e272e993ea4 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -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); @@ -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 diff --git a/src/librustc_trans/mir/place.rs b/src/librustc_trans/mir/place.rs index b340d91b02708..680c7038998e1 100644 --- a/src/librustc_trans/mir/place.rs +++ b/src/librustc_trans/mir/place.rs @@ -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); } _ => {} diff --git a/src/test/codegen/call-metadata.rs b/src/test/codegen/call-metadata.rs new file mode 100644 index 0000000000000..20d42ed852dfe --- /dev/null +++ b/src/test/codegen/call-metadata.rs @@ -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 or the MIT license +// , 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 { + Some(true) +}