Skip to content
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

rustc: Add LLVM nounwind with -C panic=abort #45031

Merged
merged 1 commit into from
Oct 13, 2017

Conversation

alexcrichton
Copy link
Member

This informs LLVM that functions can't unwind, which while it should typically
have already been inferred when necessary or otherwise not impact codegen is
apparently needed on targets like ARM to avoid references to unnecessary
symbols.

Closes #44992

@rust-highfive
Copy link
Collaborator

r? @arielb1

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

@arielb1
Copy link
Contributor

arielb1 commented Oct 5, 2017

It appears that get_fn will undo your work in some situations:

if !tcx.is_foreign_item(instance_def_id) {
attributes::unwind(llfn, true);
}

I'm not sure why we have 2 places of code here, but I imagine that needs to be fixed.

@alexcrichton
Copy link
Member Author

Excellent point! I've updated and extended the test case for that as well.

// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z no-landing-pads
Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIK this test has basically always been invalid, and -C panic=abort is tested through many other tests

@arielb1
Copy link
Contributor

arielb1 commented Oct 6, 2017

Why did you remove the mir_trans_no_landing_pads tests? Is -Z no-landing-pads now just UB on panics? I don't think having a "UB on panic" flag is a good idea, even if it's an unstable flag.

r=me if you condition the nounwind flag on -C panic=abort.

@alexcrichton
Copy link
Member Author

Would you prefer I just remove the -Z no-landing-pads? The purpose of using the no_landing_pads method was to also read this from the target spec as some targets are panic-abort-by-default.

It's true that yes, -Z no-landing-pads is not sounds with -C panic=unwind.

@arielb1
Copy link
Contributor

arielb1 commented Oct 6, 2017

@alexcrichton

I know -Z no-landing-pads is an unstable feature, but I would prefer at least a warning cycle before we remove it or change it to be UB.

@alexcrichton
Copy link
Member Author

@bors: r=arielb1

@bors
Copy link
Contributor

bors commented Oct 6, 2017

📌 Commit d2348dd has been approved by arielb1

@shepmaster shepmaster added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 6, 2017
@bors
Copy link
Contributor

bors commented Oct 8, 2017

⌛ Testing commit d2348dd3d12ced0f53388e9b8899f7df58e1fea7 with merge c5cc2b0add25d809c7384c78e1915ef7156dae00...

@bors
Copy link
Contributor

bors commented Oct 9, 2017

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Oct 9, 2017

Two codegen tests failed on x86_64-pc-windows-msvc.

[01:11:22] ---- [codegen] codegen\nounwind.rs stdout ----
[01:11:22] 	
[01:11:22] error: verification with 'FileCheck' failed
[01:11:22] status: exit code: 1
[01:11:22] command: PATH=";C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.14393.0\x64;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64;C:\projects\rust\build\x86_64-pc-windows-msvc\stage0-tools\x86_64-pc-windows-msvc\release\deps;C:\projects\rust\build\x86_64-pc-windows-msvc\stage0-sysroot\lib\rustlib\x86_64-pc-windows-msvc\lib;C:\Program Files (x86)\Inno Setup 5;C:\Python27;C:\msys64\mingw64\bin;C:\msys64\usr\bin;C:\Perl\site\bin;C:\Perl\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Program Files\7-Zip;C:\Program Files\Microsoft\Web Platform Installer;C:\Tools\GitVersion;C:\Tools\PsTools;C:\Program Files\Git LFS;C:\Program Files (x86)\Subversion\bin;C:\Program Files\Microsoft SQL Server\120\Tools\Binn;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\110\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn;C:\Program Files\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn\ManagementStudio;C:\Tools\WebDriver;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.4;C:\Program Files (x86)\Microsoft Visual Studio 12.0\Common7\IDE\PrivateAssemblies;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI\wbin;C:\Ruby193\bin;C:\Tools\NUnit\bin;C:\Tools\xUnit;C:\Tools\MSpec;C:\Tools\Coverity\bin;C:\Program Files (x86)\CMake\bin;C:\go\bin;C:\Program Files\Java\jdk1.8.0\bin;C:\Python27;C:\Program Files\nodejs;C:\Program Files (x86)\iojs;C:\Program Files\iojs;C:\Users\appveyor\AppData\Roaming\npm;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files (x86)\MSBuild\14.0\Bin;C:\Tools\NuGet;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files\Microsoft DNX\Dnvm;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\130\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Apache\Maven\bin;C:\Python27\Scripts;C:\Tools\NUnit3;C:\Program Files\Mercurial;C:\Program Files\LLVM\bin;C:\Program Files\dotnet;C:\Program Files\erl8.3\bin;C:\Tools\curl\bin;C:\Program Files\Amazon\AWSCLI;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\140;C:\Program Files (x86)\Yarn\bin;C:\Program Files\Git\cmd;C:\Program Files\Git\usr\bin;C:\ProgramData\chocolatey\bin;C:\Tools\vcpkg;C:\Program Files (x86)\nodejs;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Users\appveyor\AppData\Local\Yarn\bin;C:\Users\appveyor\AppData\Roaming\npm;C:\Program Files\AppVeyor\BuildAgent;C:\projects\rust;C:\projects\rust\handle" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\llvm\\build\\bin\\FileCheck.exe" "--input-file" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\test\\codegen\\nounwind.ll" "C:\\projects\\rust\\src/test\\codegen\\nounwind.rs"
[01:11:22] stdout:
[01:11:22] ------------------------------------------
[01:11:22] 
[01:11:22] ------------------------------------------
[01:11:22] stderr:
[01:11:22] ------------------------------------------
[01:11:22] C:\projects\rust\src/test\codegen\nounwind.rs:22:11: error: expected string not found in input
[01:11:22] // CHECK: @bar() unnamed_addr #0
[01:11:22]           ^
[01:11:22] C:\projects\rust\build\x86_64-pc-windows-msvc\test\codegen\nounwind.ll:7:36: note: scanning from here
[01:11:22] define void @foo() unnamed_addr #0 {
[01:11:22]                                    ^
[01:11:22] C:\projects\rust\build\x86_64-pc-windows-msvc\test\codegen\nounwind.ll:17:14: note: possible intended match here
[01:11:22] declare void @bar() unnamed_addr #1
[01:11:22]              ^
[01:11:22] 
[01:11:22] ------------------------------------------
[01:11:22] 
[01:11:22] thread '[codegen] codegen\nounwind.rs' panicked at 'explicit panic', src\tools\compiletest\src\runtest.rs:2433:8
[01:11:22] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:11:22] 
[01:11:22] ---- [codegen] codegen\panic-abort-windows.rs stdout ----
[01:11:22] 	
[01:11:22] error: verification with 'FileCheck' failed
[01:11:22] status: exit code: 1
[01:11:22] command: PATH=";C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.14393.0\x64;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64;C:\projects\rust\build\x86_64-pc-windows-msvc\stage0-tools\x86_64-pc-windows-msvc\release\deps;C:\projects\rust\build\x86_64-pc-windows-msvc\stage0-sysroot\lib\rustlib\x86_64-pc-windows-msvc\lib;C:\Program Files (x86)\Inno Setup 5;C:\Python27;C:\msys64\mingw64\bin;C:\msys64\usr\bin;C:\Perl\site\bin;C:\Perl\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Program Files\7-Zip;C:\Program Files\Microsoft\Web Platform Installer;C:\Tools\GitVersion;C:\Tools\PsTools;C:\Program Files\Git LFS;C:\Program Files (x86)\Subversion\bin;C:\Program Files\Microsoft SQL Server\120\Tools\Binn;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\110\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn;C:\Program Files\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn\ManagementStudio;C:\Tools\WebDriver;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.4;C:\Program Files (x86)\Microsoft Visual Studio 12.0\Common7\IDE\PrivateAssemblies;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI\wbin;C:\Ruby193\bin;C:\Tools\NUnit\bin;C:\Tools\xUnit;C:\Tools\MSpec;C:\Tools\Coverity\bin;C:\Program Files (x86)\CMake\bin;C:\go\bin;C:\Program Files\Java\jdk1.8.0\bin;C:\Python27;C:\Program Files\nodejs;C:\Program Files (x86)\iojs;C:\Program Files\iojs;C:\Users\appveyor\AppData\Roaming\npm;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files (x86)\MSBuild\14.0\Bin;C:\Tools\NuGet;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files\Microsoft DNX\Dnvm;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\130\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Apache\Maven\bin;C:\Python27\Scripts;C:\Tools\NUnit3;C:\Program Files\Mercurial;C:\Program Files\LLVM\bin;C:\Program Files\dotnet;C:\Program Files\erl8.3\bin;C:\Tools\curl\bin;C:\Program Files\Amazon\AWSCLI;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\140;C:\Program Files (x86)\Yarn\bin;C:\Program Files\Git\cmd;C:\Program Files\Git\usr\bin;C:\ProgramData\chocolatey\bin;C:\Tools\vcpkg;C:\Program Files (x86)\nodejs;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Users\appveyor\AppData\Local\Yarn\bin;C:\Users\appveyor\AppData\Roaming\npm;C:\Program Files\AppVeyor\BuildAgent;C:\projects\rust;C:\projects\rust\handle" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\llvm\\build\\bin\\FileCheck.exe" "--input-file" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\test\\codegen\\panic-abort-windows.ll" "C:\\projects\\rust\\src/test\\codegen\\panic-abort-windows.rs"
[01:11:22] stdout:
[01:11:22] ------------------------------------------
[01:11:22] 
[01:11:22] ------------------------------------------
[01:11:22] stderr:
[01:11:22] ------------------------------------------
[01:11:22] C:\projects\rust\src/test\codegen\panic-abort-windows.rs:31:11: error: expected string not found in input
[01:11:22] // CHECK: Function Attrs: uwtable
[01:11:22]           ^
[01:11:22] C:\projects\rust\build\x86_64-pc-windows-msvc\test\codegen\panic-abort-windows.ll:1:1: note: scanning from here
[01:11:22] ; ModuleID = 'panic_abort_windows0-8cd878b7c8d78940dfe6697baf5b88ec.rs'
[01:11:22] ^
[01:11:22] C:\projects\rust\build\x86_64-pc-windows-msvc\test\codegen\panic-abort-windows.ll:6:3: note: possible intended match here
[01:11:22] ; Function Attrs: nounwind uwtable
[01:11:22]   ^
[01:11:22] 
[01:11:22] ------------------------------------------
[01:11:22] 
[01:11:22] thread '[codegen] codegen\panic-abort-windows.rs' panicked at 'explicit panic', src\tools\compiletest\src\runtest.rs:2433:8
[01:11:22] 
[01:11:22] 
[01:11:22] failures:
[01:11:22]     [codegen] codegen\nounwind.rs
[01:11:22]     [codegen] codegen\panic-abort-windows.rs
[01:11:22] 
[01:11:22] test result: FAILED. 42 passed; 2 failed; 9 ignored; 0 measured; 0 filtered out
[01:11:22] 
[01:11:22] thread 'main' panicked at 'Some tests failed', src\tools\compiletest\src\main.rs:323:21

@carols10cents carols10cents 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 Oct 9, 2017
This informs LLVM that functions can't unwind, which while it should typically
have already been inferred when necessary or otherwise not impact codegen is
apparently needed on targets like ARM to avoid references to unnecessary
symbols.

Closes rust-lang#44992
@alexcrichton
Copy link
Member Author

@bors: r=arielb1

@bors
Copy link
Contributor

bors commented Oct 11, 2017

📌 Commit 24cc38e has been approved by arielb1

@bors
Copy link
Contributor

bors commented Oct 13, 2017

⌛ Testing commit 24cc38e with merge 2d73972...

bors added a commit that referenced this pull request Oct 13, 2017
rustc: Add LLVM `nounwind` with `-C panic=abort`

This informs LLVM that functions can't unwind, which while it should typically
have already been inferred when necessary or otherwise not impact codegen is
apparently needed on targets like ARM to avoid references to unnecessary
symbols.

Closes #44992
@kennytm kennytm 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 Oct 13, 2017
@bors
Copy link
Contributor

bors commented Oct 13, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 2d73972 to master...

@bors bors merged commit 24cc38e into rust-lang:master Oct 13, 2017
@alexcrichton alexcrichton deleted the nounwind branch October 14, 2017 02:38
japaric added a commit to rust-embedded/cortex-m-rt that referenced this pull request Oct 19, 2017
TL;DR thanks to rust-lang/rust#45031 we no longer need this

Now with -C panic=abort all functions are marked with the `nouwind` attribute in
LLVM-IR. With this change LLVM won't generate undefined references to
`__aeabi_unwind_cpp_pr0` et al. which we don't use / supply but are required by
the AEABI standard in the case that a function that throw exceptions
exists (semantically, we never have any of those function with panic=abort but
the LLVM-IR didn't reflect this before).
adamgreig pushed a commit to rust-embedded/cortex-m that referenced this pull request Jan 12, 2022
TL;DR thanks to rust-lang/rust#45031 we no longer need this

Now with -C panic=abort all functions are marked with the `nouwind` attribute in
LLVM-IR. With this change LLVM won't generate undefined references to
`__aeabi_unwind_cpp_pr0` et al. which we don't use / supply but are required by
the AEABI standard in the case that a function that throw exceptions
exists (semantically, we never have any of those function with panic=abort but
the LLVM-IR didn't reflect this before).
reitermarkus pushed a commit to reitermarkus/cortex-m that referenced this pull request May 4, 2022
TL;DR thanks to rust-lang/rust#45031 we no longer need this

Now with -C panic=abort all functions are marked with the `nouwind` attribute in
LLVM-IR. With this change LLVM won't generate undefined references to
`__aeabi_unwind_cpp_pr0` et al. which we don't use / supply but are required by
the AEABI standard in the case that a function that throw exceptions
exists (semantically, we never have any of those function with panic=abort but
the LLVM-IR didn't reflect this before).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants