Fix LLVM IR generated for C-variadic arguments #59577

dlrobertson · 2019-03-31T03:42:23Z

It is possible to create malformed LLVM IR given variadic arguments that
are aggregate types. This occurs due to improper tracking of the current
argument in the functions list of arguments.

Fixes: #58881

rust-highfive · 2019-03-31T03:42:26Z

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

dlrobertson · 2019-03-31T03:48:08Z

src/librustc_codegen_ssa/mir/block.rs

-                    )),
-                    layout: layout,
-                };
-                self.codegen_argument(&mut bx, op, &mut llargs, &fn_ty.args[i]);


codegen_argument when called given a function where is_ignore is true doesn't do anything other than add padding (which we shouldn't do here for C-variadics), so this section isn't needed.

dlrobertson · 2019-03-31T03:50:07Z

src/librustc_codegen_ssa/mir/block.rs

-                    layout: layout,
-                };
-                self.codegen_argument(&mut bx, op, &mut llargs, &fn_ty.args[i]);
+            let i = if sig.c_variadic && last_arg_idx.map(|x| i >= x).unwrap_or(false) {


If we hit the "spoofed" C-variadic argument we need to use i + 1 for each of the subsequent iterations of this loop. If we don't we'll end up using the wrong argument info.

nagisa · 2019-03-31T11:50:37Z

@bors r+

bors · 2019-03-31T11:50:39Z

📌 Commit 9095feb has been approved by nagisa

Fix LLVM IR generated for C-variadic arguments It is possible to create malformed LLVM IR given variadic arguments that are aggregate types. This occurs due to improper tracking of the current argument in the functions list of arguments. Fixes: rust-lang#58881

Centril · 2019-03-31T14:10:04Z

Failed in #59582 (comment), @bors r-

dlrobertson · 2019-03-31T15:54:23Z

Fixed the test. It now passes on the following architectures:

x86_64-unknown-linux-gnu
i686-unknown-linux-gnu
i586-unkown-linux-gnu

dlrobertson · 2019-03-31T15:57:29Z

It still fails on aarch64-unknown-linux-gnu. I'm debating adding ignore statements for certain architectures

nagisa · 2019-03-31T15:57:53Z

@bors r+

…

On Sun, Mar 31, 2019, 18:54 Dan Robertson ***@***.***> wrote: Fixed the test. It now passes on the following architectures: - x86_64-unknown-linux-gnu - i686-unknown-linux-gnu - i586-unkown-linux-gnu - aarch64-unknown-linux-gnu — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#59577 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApc0ixp-Co1ckL6yquwmMFryOZKQdc6ks5vcNpMgaJpZM4cUGRx> .

bors · 2019-03-31T15:57:54Z

📌 Commit ebed9e5fb16238dfb09bc4ed23859e391160109f has been approved by nagisa

nagisa · 2019-03-31T16:00:27Z

Ah didn't see the last message. You can have architecture specific codegen tests now as it is possible to add configurations that compile with specific flags only. You can see the optimize-attr codegen test for an example. @bors r-

bors · 2019-03-31T16:00:28Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Rollup of 4 pull requests #59584

bors · 2019-03-31T16:00:29Z

📌 Commit ebed9e5fb16238dfb09bc4ed23859e391160109f has been approved by nagisa

dlrobertson · 2019-03-31T16:02:37Z

@nagisa Made the CHECK string a bit more general and it passes with all of the previously listed architectures. Would it be better to see if it passes homu or just add ignores now for the host triples I didn't test?

Centril · 2019-03-31T16:06:28Z

r? @nagisa

nagisa · 2019-03-31T16:11:02Z

@dlrobertson It would be fine to ignore as well, but I think, as far as codegen tests are concerned, that also means compilation errors, and thus ICEs, are ignored as well.

It is possible to create malformed LLVM IR given variadic arguments that are aggregate types. This occurs due to improper tracking of the current argument in the functions list of arguments.

dlrobertson · 2019-03-31T17:50:39Z

After doing some further testing my updated version does not fail on prior versions. As a result, we'll need to use ignore for a proper regression test. I used only-x86_64. Technically the test can also be run on aarch64, but I figured it would be more maintainable if we used an only- statement instead of multiple ignore- statements. I think ignore-windows is needed because Windows on x86_64 is similar to i686.

nagisa · 2019-03-31T20:26:24Z

@bors r+

bors · 2019-03-31T20:26:25Z

📌 Commit a9d62be has been approved by nagisa

bors · 2019-03-31T20:28:07Z

⌛ Testing commit a9d62be with merge e3428db...

Fix LLVM IR generated for C-variadic arguments It is possible to create malformed LLVM IR given variadic arguments that are aggregate types. This occurs due to improper tracking of the current argument in the functions list of arguments. Fixes: #58881

bors · 2019-03-31T23:50:39Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: nagisa
Pushing e3428db to master...

rust-highfive assigned matthewjasper Mar 31, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 31, 2019

dlrobertson commented Mar 31, 2019

View reviewed changes

dlrobertson force-pushed the fix_58881 branch from 4170865 to 6d26783 Compare March 31, 2019 03:51

dlrobertson changed the title ~~Fix LLVM generated for C-variadic arguments~~ Fix LLVM IR generated for C-variadic arguments Mar 31, 2019

dlrobertson force-pushed the fix_58881 branch from 6d26783 to 9095feb Compare March 31, 2019 04:05

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 Mar 31, 2019

Centril mentioned this pull request Mar 31, 2019

Rollup of 5 pull requests #59582

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 31, 2019

dlrobertson force-pushed the fix_58881 branch from 9095feb to ebed9e5 Compare March 31, 2019 15:26

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 31, 2019

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 31, 2019

dlrobertson force-pushed the fix_58881 branch from ebed9e5 to 12d3e9f Compare March 31, 2019 16:00

rust-highfive assigned nagisa and unassigned matthewjasper Mar 31, 2019

Fix LLVM IR generated for C-variadic arguments

a9d62be

It is possible to create malformed LLVM IR given variadic arguments that are aggregate types. This occurs due to improper tracking of the current argument in the functions list of arguments.

dlrobertson force-pushed the fix_58881 branch 2 times, most recently from 2e97578 to a9d62be Compare March 31, 2019 17:47

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 31, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 31, 2019

bors merged commit a9d62be into rust-lang:master Mar 31, 2019

dlrobertson deleted the fix_58881 branch March 31, 2019 23:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix LLVM IR generated for C-variadic arguments #59577

Fix LLVM IR generated for C-variadic arguments #59577

dlrobertson commented Mar 31, 2019

rust-highfive commented Mar 31, 2019

dlrobertson Mar 31, 2019

dlrobertson Mar 31, 2019

nagisa commented Mar 31, 2019

bors commented Mar 31, 2019

Centril commented Mar 31, 2019

dlrobertson commented Mar 31, 2019 •

edited

Loading

dlrobertson commented Mar 31, 2019

nagisa commented Mar 31, 2019 via email

bors commented Mar 31, 2019

nagisa commented Mar 31, 2019 via email •

edited

Loading

bors commented Mar 31, 2019

bors commented Mar 31, 2019

dlrobertson commented Mar 31, 2019

Centril commented Mar 31, 2019

nagisa commented Mar 31, 2019 •

edited

Loading

dlrobertson commented Mar 31, 2019

nagisa commented Mar 31, 2019

bors commented Mar 31, 2019

bors commented Mar 31, 2019

bors commented Mar 31, 2019

Fix LLVM IR generated for C-variadic arguments #59577

Fix LLVM IR generated for C-variadic arguments #59577

Conversation

dlrobertson commented Mar 31, 2019

rust-highfive commented Mar 31, 2019

dlrobertson Mar 31, 2019

Choose a reason for hiding this comment

dlrobertson Mar 31, 2019

Choose a reason for hiding this comment

nagisa commented Mar 31, 2019

bors commented Mar 31, 2019

Centril commented Mar 31, 2019

dlrobertson commented Mar 31, 2019 • edited Loading

dlrobertson commented Mar 31, 2019

nagisa commented Mar 31, 2019 via email

bors commented Mar 31, 2019

nagisa commented Mar 31, 2019 via email • edited Loading

bors commented Mar 31, 2019

bors commented Mar 31, 2019

dlrobertson commented Mar 31, 2019

Centril commented Mar 31, 2019

nagisa commented Mar 31, 2019 • edited Loading

dlrobertson commented Mar 31, 2019

nagisa commented Mar 31, 2019

bors commented Mar 31, 2019

bors commented Mar 31, 2019

bors commented Mar 31, 2019

dlrobertson commented Mar 31, 2019 •

edited

Loading

nagisa commented Mar 31, 2019 via email •

edited

Loading

nagisa commented Mar 31, 2019 •

edited

Loading