New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alignment for array indexing #57053

Merged
merged 1 commit into from Dec 24, 2018

Conversation

Projects
None yet
7 participants
@nikic
Copy link
Contributor

nikic commented Dec 21, 2018

We need to reduce the alignment with the used offset. If the offset isn't known, use the element size, as this will yield the minimum possible alignment.

This handles both direct array indexing, and array repeat expressions.

Fixes #56927.

r? @nagisa

// as this will yield the lowest alignment.
let layout = self.layout.field(bx, 0);
let offset = if bx.is_const_integral(llindex) {
layout.size.checked_mul(bx.const_to_uint(llindex), bx).unwrap_or(layout.size)

This comment has been minimized.

@nikic

nikic Dec 21, 2018

Contributor

I think this checked_mul is not strictly necessary, as such cases won't reach here, but included it to be sure.

@nagisa

nagisa approved these changes Dec 21, 2018

Copy link
Contributor

nagisa left a comment

Could you also add a test case(s) for #[packed] that would exercise these specific constructs? I don’t believe we have such a test yet. codegen/packed.rs would be a good place for one.

r=me after CHECK-LABELs are added to the test.

arr: [u32; 4],
}

// CHECK: store i32 0, i32* %{{.+}}, align 16

This comment has been minimized.

@nagisa

nagisa Dec 21, 2018

Contributor

Please check for the label before the store instructions.

Fix alignment for array indexing
We need to reduce the alignment with the used offset. If the offset
isn't known, we need to reduce with the element size to support
arbitrary offsets.

@nikic nikic force-pushed the nikic:fix-gep-align branch from b208649 to 097d39d Dec 21, 2018

@nikic

This comment has been minimized.

Copy link
Contributor

nikic commented Dec 21, 2018

@nagisa I've added CHECK-LABELs and a couple of packed tests.

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Dec 21, 2018

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

📌 Commit 097d39d has been approved by nagisa

Centril added a commit to Centril/rust that referenced this pull request Dec 22, 2018

Rollup merge of rust-lang#57053 - nikic:fix-gep-align, r=nagisa
Fix alignment for array indexing

We need to reduce the alignment with the used offset. If the offset isn't known, use the element size, as this will yield the minimum possible alignment.

This handles both direct array indexing, and array repeat expressions.

Fixes rust-lang#56927.

r? @nagisa

Centril added a commit to Centril/rust that referenced this pull request Dec 23, 2018

Rollup merge of rust-lang#57053 - nikic:fix-gep-align, r=nagisa
Fix alignment for array indexing

We need to reduce the alignment with the used offset. If the offset isn't known, use the element size, as this will yield the minimum possible alignment.

This handles both direct array indexing, and array repeat expressions.

Fixes rust-lang#56927.

r? @nagisa

bors added a commit that referenced this pull request Dec 23, 2018

Auto merge of #57075 - Centril:rollup, r=Centril
Rollup of 10 pull requests

Successful merges:

 - #56188 (enum type instead of variant suggestion unification )
 - #56916 (Fix mutable references in `static mut`)
 - #56917 (Simplify MIR generation for logical operations)
 - #56953 (Mark tuple structs as live if their constructors are used)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56966 (Correct strings for raw pointer deref and array access suggestions)
 - #56999 (AST/HIR: Introduce `ExprKind::Err` for better error recovery in the front-end)
 - #57020 (Point to cause of `fn` expected return type)
 - #57032 (fix deprecation warnings in liballoc benches)
 - #57053 (Fix alignment for array indexing)

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Dec 23, 2018

Rollup merge of rust-lang#57053 - nikic:fix-gep-align, r=nagisa
Fix alignment for array indexing

We need to reduce the alignment with the used offset. If the offset isn't known, use the element size, as this will yield the minimum possible alignment.

This handles both direct array indexing, and array repeat expressions.

Fixes rust-lang#56927.

r? @nagisa

Centril added a commit to Centril/rust that referenced this pull request Dec 23, 2018

Rollup merge of rust-lang#57053 - nikic:fix-gep-align, r=nagisa
Fix alignment for array indexing

We need to reduce the alignment with the used offset. If the offset isn't known, use the element size, as this will yield the minimum possible alignment.

This handles both direct array indexing, and array repeat expressions.

Fixes rust-lang#56927.

r? @nagisa

bors added a commit that referenced this pull request Dec 23, 2018

Auto merge of #57083 - Centril:rollup, r=Centril
Rollup of 10 pull requests

Successful merges:

 - #56188 (enum type instead of variant suggestion unification )
 - #56916 (Fix mutable references in `static mut`)
 - #56917 (Simplify MIR generation for logical operations)
 - #56953 (Mark tuple structs as live if their constructors are used)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56966 (Correct strings for raw pointer deref and array access suggestions)
 - #57020 (Point to cause of `fn` expected return type)
 - #57032 (fix deprecation warnings in liballoc benches)
 - #57053 (Fix alignment for array indexing)
 - #57067 (Stabilize min_const_unsafe_fn in 1.33)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request Dec 23, 2018

Auto merge of #57083 - Centril:rollup, r=Centril
Rollup of 10 pull requests

Successful merges:

 - #56188 (enum type instead of variant suggestion unification )
 - #56916 (Fix mutable references in `static mut`)
 - #56917 (Simplify MIR generation for logical operations)
 - #56953 (Mark tuple structs as live if their constructors are used)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56966 (Correct strings for raw pointer deref and array access suggestions)
 - #57020 (Point to cause of `fn` expected return type)
 - #57032 (fix deprecation warnings in liballoc benches)
 - #57053 (Fix alignment for array indexing)
 - #57067 (Stabilize min_const_unsafe_fn in 1.33)

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Dec 23, 2018

Rollup merge of rust-lang#57053 - nikic:fix-gep-align, r=nagisa
Fix alignment for array indexing

We need to reduce the alignment with the used offset. If the offset isn't known, use the element size, as this will yield the minimum possible alignment.

This handles both direct array indexing, and array repeat expressions.

Fixes rust-lang#56927.

r? @nagisa

bors added a commit that referenced this pull request Dec 24, 2018

Auto merge of #57087 - Centril:rollup, r=Centril
Rollup of 14 pull requests

Successful merges:

 - #56188 (enum type instead of variant suggestion unification )
 - #56342 (Improve docs for collecting into `Option`s)
 - #56916 (Fix mutable references in `static mut`)
 - #56917 (Simplify MIR generation for logical operations)
 - #56939 (Pin stabilization)
 - #56953 (Mark tuple structs as live if their constructors are used)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56966 (Correct strings for raw pointer deref and array access suggestions)
 - #57020 (Point to cause of `fn` expected return type)
 - #57032 (fix deprecation warnings in liballoc benches)
 - #57053 (Fix alignment for array indexing)
 - #57062 (Fix a comment)
 - #57067 (Stabilize min_const_unsafe_fn in 1.33)
 - #57078 (Ignore two tests on s390x)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request Dec 24, 2018

Auto merge of #57087 - Centril:rollup, r=Centril
Rollup of 14 pull requests

Successful merges:

 - #56188 (enum type instead of variant suggestion unification )
 - #56342 (Improve docs for collecting into `Option`s)
 - #56916 (Fix mutable references in `static mut`)
 - #56917 (Simplify MIR generation for logical operations)
 - #56939 (Pin stabilization)
 - #56953 (Mark tuple structs as live if their constructors are used)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56966 (Correct strings for raw pointer deref and array access suggestions)
 - #57020 (Point to cause of `fn` expected return type)
 - #57032 (fix deprecation warnings in liballoc benches)
 - #57053 (Fix alignment for array indexing)
 - #57062 (Fix a comment)
 - #57067 (Stabilize min_const_unsafe_fn in 1.33)
 - #57078 (Ignore two tests on s390x)

Failed merges:

r? @ghost

@bors bors merged commit 097d39d into rust-lang:master Dec 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nagisa nagisa added beta-accepted and removed beta-nominated labels Jan 3, 2019

bors added a commit that referenced this pull request Jan 4, 2019

Auto merge of #57305 - pietroalbini:beta-rollup, r=pietroalbini
[beta] Rollup backports

Cherry-picked:

* #57053: Fix alignment for array indexing
* #57181: resolve: Fix another ICE in import validation
* #57185: resolve: Fix one more ICE in import validation
* #57282: Wf-check the output type of a function in MIR-typeck
* #55318: Ensure that Rustdoc discovers all necessary auto trait bounds
* #56838: Call poly_project_and_unify_type on types that contain inference types

Rolled up:

* #57300: [beta] Update RLS to include 100% CPU on hover bugfix
* #57301: beta: bootstrap from latest stable (1.31.1)
* #57292: [BETA] Update cargo

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment