Skip to content

Commit

Permalink
clearly define int and uint to fix unsoundness
Browse files Browse the repository at this point in the history
This fixes the gap in the language definition causing #18726 by defining
a clear bound on the maximum size for libraries to enforce.

Closes #18069
  • Loading branch information
thestinger committed Nov 19, 2014
1 parent e09d986 commit 210e059
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 20 deletions.
19 changes: 8 additions & 11 deletions src/doc/reference.md
Expand Up @@ -3557,17 +3557,14 @@ The machine types are the following:

#### Machine-dependent integer types

The Rust type `uint` [^rustuint] is an
unsigned integer type with target-machine-dependent size. Its size, in
bits, is equal to the number of bits required to hold any memory address on
the target machine.

The Rust type `int` [^rustint] is a two's complement signed integer type with
target-machine-dependent size. Its size, in bits, is equal to the size of the
rust type `uint` on the same target machine.

[^rustuint]: A Rust `uint` is analogous to a C99 `uintptr_t`.
[^rustint]: A Rust `int` is analogous to a C99 `intptr_t`.
The `uint` type is an unsigned integer type with the same number of bits as the
platform's pointer type. It can represent every memory address in the process.

The `int` type is a signed integer type with the same number of bits as the
platform's pointer type. The theoretical upper bound on object and array size
is the maximum `int` value. This ensures that `int` can be used to calculate
differences between pointers into an object or array and can address every byte
within an object along with one byte past the end.

### Textual types

Expand Down
12 changes: 6 additions & 6 deletions src/librustc_trans/trans/adt.rs
Expand Up @@ -464,17 +464,17 @@ fn ensure_struct_fits_in_address_space(ccx: &CrateContext,
scapegoat: ty::t) {
let mut offset = 0;
for &llty in fields.iter() {
// Invariant: offset < ccx.max_obj_size() <= 1<<61
// Invariant: offset < ccx.obj_size_bound() <= 1<<61
if !packed {
let type_align = machine::llalign_of_min(ccx, llty);
offset = roundup(offset, type_align);
}
// type_align is a power-of-2, so still offset < ccx.max_obj_size()
// llsize_of_alloc(ccx, llty) is also less than ccx.max_obj_size()
// type_align is a power-of-2, so still offset < ccx.obj_size_bound()
// llsize_of_alloc(ccx, llty) is also less than ccx.obj_size_bound()
// so the sum is less than 1<<62 (and therefore can't overflow).
offset += machine::llsize_of_alloc(ccx, llty);

if offset >= ccx.max_obj_size() {
if offset >= ccx.obj_size_bound() {
ccx.report_overbig_object(scapegoat);
}
}
Expand All @@ -493,11 +493,11 @@ fn ensure_enum_fits_in_address_space(ccx: &CrateContext,
let discr_size = machine::llsize_of_alloc(ccx, ll_inttype(ccx, discr));
let (field_size, field_align) = union_size_and_align(fields);

// field_align < 1<<32, discr_size <= 8, field_size < MAX_OBJ_SIZE <= 1<<61
// field_align < 1<<32, discr_size <= 8, field_size < OBJ_SIZE_BOUND <= 1<<61
// so the sum is less than 1<<62 (and can't overflow).
let total_size = roundup(discr_size, field_align) + field_size;

if total_size >= ccx.max_obj_size() {
if total_size >= ccx.obj_size_bound() {
ccx.report_overbig_object(scapegoat);
}
}
Expand Down
19 changes: 17 additions & 2 deletions src/librustc_trans/trans/context.rs
Expand Up @@ -703,8 +703,23 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
&self.local.trait_cache
}

pub fn max_obj_size(&self) -> u64 {
1<<31 /* FIXME #18069: select based on architecture */
/// Return exclusive upper bound on object size.
///
/// The theoretical maximum object size is defined as the maximum positive `int` value. This
/// ensures that the `offset` semantics remain well-defined by allowing it to correctly index
/// every address within an object along with one byte past the end, along with allowing `int`
/// to store the difference between any two pointers into an object.
///
/// The upper bound on 64-bit currently needs to be lower because LLVM uses a 64-bit integer to
/// represent object size in bits. It would need to be 1 << 61 to account for this, but is
/// currently conservatively bounded to 1 << 47 as that is enough to cover the current usable
/// address space on 64-bit ARMv8 and x86_64.
pub fn obj_size_bound(&self) -> u64 {
match self.sess().target.target.target_word_size[] {
"32" => 1 << 31,
"64" => 1 << 47,
_ => unreachable!() // error handled by config::build_target_config
}
}

pub fn report_overbig_object(&self, obj: ty::t) -> ! {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/trans/type_of.rs
Expand Up @@ -34,7 +34,7 @@ fn ensure_array_fits_in_address_space(ccx: &CrateContext,
scapegoat: ty::t) {
let esz = machine::llsize_of_alloc(ccx, llet);
match esz.checked_mul(size) {
Some(n) if n < ccx.max_obj_size() => {}
Some(n) if n < ccx.obj_size_bound() => {}
_ => { ccx.report_overbig_object(scapegoat) }
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/compile-fail/huge-enum.rs
Expand Up @@ -12,6 +12,12 @@

// FIXME: work properly with higher limits

#[cfg(target_word_size = "32")]
fn main() {
let big: Option<[u32, ..(1<<29)-1]> = None;
}

#[cfg(target_word_size = "64")]
fn main() {
let big: Option<[u32, ..(1<<45)-1]> = None;
}
21 changes: 21 additions & 0 deletions src/test/run-pass/huge-largest-array.rs
@@ -0,0 +1,21 @@
// Copyright 2014 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.

use std::mem::size_of;

#[cfg(target_word_size = "32")]
pub fn main() {
assert_eq!(size_of::<[u8, ..(1 << 31) - 1]>(), (1 << 31) - 1);
}

#[cfg(target_word_size = "64")]
pub fn main() {
assert_eq!(size_of::<[u8, ..(1 << 47) - 1]>(), (1 << 47) - 1);
}

21 comments on commit 210e059

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/int = 210e059 into auto

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/int = 210e059 merged ok, testing candidate = 133fffed

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/int = 210e059 into auto

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/int = 210e059 merged ok, testing candidate = 56d20494

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/int = 210e059 into auto

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/int = 210e059 merged ok, testing candidate = f5c1c9d8

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/int = 210e059 into auto

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/int = 210e059 merged ok, testing candidate = 879918d4

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/int = 210e059 into auto

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/int = 210e059 merged ok, testing candidate = a24b444

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 210e059 Nov 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = a24b444

Please sign in to comment.