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

[wpiutil] Use C++23 stacktrace library on Windows #6839

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

calcmogul
Copy link
Member

@calcmogul calcmogul commented Jul 16, 2024

This lets us remove the unmaintained StackWalker library and its hacky upstream_utils script.

@Gold856 reported that StackWalker gives blank stacktraces: https://discord.com/channels/176186766946992128/368993897495527424/1261940029287301150. They also reported an earlier version of this PR giving the following stacktrace instead:

D:\allwpilib\developerRobot\src\main\native\cpp\Robot.cpp(18): developerRobotCpp!Robot::RobotInit+0xB6
D:\allwpilib\wpilibc\src\main\native\cpp\TimedRobot.cpp(22): wpilibcd!frc::TimedRobot::StartCompetition+0x4F
D:\allwpilib\wpilibc\src\main\native\include\frc\RobotBase.h(36): developerRobotCpp!frc::impl::RunRobot<Robot>+0xC8
D:\allwpilib\wpilibc\src\main\native\include\frc\RobotBase.h(106): developerRobotCpp!frc::StartRobot<Robot>+0x17E
D:\allwpilib\developerRobot\src\main\native\cpp\Robot.cpp(60): developerRobotCpp!main+0xB
D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl(79): developerRobotCpp!invoke_main+0x39
D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl(288): developerRobotCpp!__scrt_common_main_seh+0x132
D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl(331): developerRobotCpp!__scrt_common_main+0xE
D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp(17): developerRobotCpp!mainCRTStartup+0xE
KERNEL32!BaseThreadInitThunk+0x1D
ntdll!RtlUserThreadStart+0x28

@calcmogul calcmogul requested review from PeterJohnson and a team as code owners July 16, 2024 00:30
// Use C++23 on Windows
nativeUtils.platformConfigs.each {
if (it.name.contains('windows')) {
it.cppCompiler.args.add("/std:c++latest")
Copy link
Member

@ThadHouse ThadHouse Jul 16, 2024

Choose a reason for hiding this comment

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

Hmm. This might be a problem. I'm not sure if mixing /std:c++latest and /std:c++20 is safe, and team programs would still be c++ 20. c++latest technically isn't ABI safe across runtime releases.

Copy link
Member Author

@calcmogul calcmogul Jul 16, 2024

Choose a reason for hiding this comment

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

From what I've read, /std:c++latest isn't guaranteed to have stable standard support. MSVC doesn't have a /std:c++23 flag yet either, tho they've had std::stacktrace support since VS 17.4 (released November 2022).

Copy link
Member

Choose a reason for hiding this comment

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

Let me see if theres a way in gradle to just set that one file to c++latest, at least for the gradle build. I'd prefer limiting the scope as much as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Does it need to be set to c++latest to use std::stacktrace?

Copy link
Member Author

Choose a reason for hiding this comment

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

@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch 3 times, most recently from ad760b0 to 0141b53 Compare July 20, 2024 07:14
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch 2 times, most recently from 5fbd5b0 to d09856e Compare August 1, 2024 23:48
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from d09856e to 757c762 Compare September 13, 2024 01:12
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from 757c762 to 0152995 Compare October 14, 2024 02:13
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch 3 times, most recently from 453c595 to 98b7368 Compare November 7, 2024 21:41
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from 98b7368 to 5afec7c Compare November 9, 2024 04:49
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from 5afec7c to ef97f6e Compare November 16, 2024 19:07
@ThadHouse
Copy link
Member

Honestly, until MSVC gets a /std:c++23 flag, this really can't be merged. We cannot ship libraries targeting /std:c++latest safely at all.

@auscompgeek auscompgeek added the state: blocked Something is blocking action. label Nov 17, 2024
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from ef97f6e to 20c9dbb Compare November 19, 2024 01:21
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from 20c9dbb to 2be6142 Compare December 22, 2024 22:48
@github-actions github-actions bot added component: ntcore NetworkTables library component: wpiutil WPI utility library component: command-based WPILib Command Based Library component: wpimath Math library component: wpinet WPI networking library component: apriltag AprilTag library labels Dec 22, 2024
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from 2be6142 to d006b67 Compare February 12, 2025 16:20
@PeterJohnson
Copy link
Member

While we're waiting for the C++23 final version to land in MSVC, if StackWalker is non-functional, we can just remove it in the meantime (and just return an empty string).

@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from d006b67 to 6e212ff Compare February 13, 2025 00:04
@calcmogul
Copy link
Member Author

calcmogul commented Feb 13, 2025

Here's the StackWalker removal PR: #7777

@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch 4 times, most recently from abbc631 to 876d410 Compare February 13, 2025 00:32
@calcmogul calcmogul force-pushed the wpiutil-use-cpp23-stacktrace-library-on-windows branch from 876d410 to b196da0 Compare February 13, 2025 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: apriltag AprilTag library component: command-based WPILib Command Based Library component: ntcore NetworkTables library component: wpimath Math library component: wpinet WPI networking library component: wpiutil WPI utility library state: blocked Something is blocking action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants