Fix alignment for array indexing #57053

nikic · 2018-12-21T22:18:07Z

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

nikic · 2018-12-21T22:20:42Z

src/librustc_codegen_ssa/mir/place.rs

+        // 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)


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

nagisa

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.

nagisa · 2018-12-21T22:25:55Z

src/test/codegen/issue-56927.rs

+    arr: [u32; 4],
+}
+
+// CHECK: store i32 0, i32* %{{.+}}, align 16


Please check for the label before the store instructions.

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 · 2018-12-21T22:56:09Z

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

nagisa · 2018-12-21T22:58:56Z

@bors r+

bors · 2018-12-21T22:58:57Z

📌 Commit 097d39d has been approved by nagisa

src/librustc_codegen_ssa/mir/place.rs

@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

@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

@ghost

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

@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

@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

@ghost

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

@ghost

[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

rust-highfive assigned nagisa Dec 21, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 21, 2018

nikic commented Dec 21, 2018

View reviewed changes

nikic mentioned this pull request Dec 21, 2018

rustdoc crashing on linux-sparc64 due to unaligned access #56927

Closed

nikic added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 21, 2018

nagisa approved these changes Dec 21, 2018

View reviewed changes

Fix alignment for array indexing

097d39d

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 force-pushed the fix-gep-align branch from b208649 to 097d39d Compare December 21, 2018 22:52

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 21, 2018

jrtc27 reviewed Dec 21, 2018

View reviewed changes

src/librustc_codegen_ssa/mir/place.rs Show resolved Hide resolved

Centril mentioned this pull request Dec 22, 2018

Rollup of 41 pull requests #57058

Closed

Centril mentioned this pull request Dec 23, 2018

Rollup of 10 pull requests #57075

Closed

Centril mentioned this pull request Dec 23, 2018

Rollup of 10 pull requests #57083

Closed

Centril mentioned this pull request Dec 23, 2018

Rollup of 14 pull requests #57087

Merged

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

nagisa added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jan 3, 2019

pietroalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 3, 2019

pietroalbini mentioned this pull request Jan 3, 2019

[beta] Rollup backports #57305

Merged

pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 3, 2019

RalfJung mentioned this pull request Aug 1, 2023

interpret: fix alignment handling for Repeat expressions #114296

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix alignment for array indexing #57053

Fix alignment for array indexing #57053

nikic commented Dec 21, 2018

nikic Dec 21, 2018

nagisa left a comment

nagisa Dec 21, 2018

nikic commented Dec 21, 2018

nagisa commented Dec 21, 2018

bors commented Dec 21, 2018

Fix alignment for array indexing #57053

Fix alignment for array indexing #57053

Conversation

nikic commented Dec 21, 2018

nikic Dec 21, 2018

Choose a reason for hiding this comment

nagisa left a comment

Choose a reason for hiding this comment

nagisa Dec 21, 2018

Choose a reason for hiding this comment

nikic commented Dec 21, 2018

nagisa commented Dec 21, 2018

bors commented Dec 21, 2018