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

vehicle: fix sigfpe when acceleration is zero #45626

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

CAHEK7
Copy link
Contributor

@CAHEK7 CAHEK7 commented Nov 25, 2020

Summary

SUMMARY: Bugfixes "Fix sigfpe crash in vehicle::thrust"

Purpose of change

It fixes rare case when all engines produce zero power due to no fuel or exhausted character and target cruise speed is set less than current.
Fix #45473

Describe the solution

Just jump over the case where division by 0 occurs, set load = 0 inside else branch and perform all other calculations.

Describe alternatives you've considered

Maybe it should be done a few lines earlier to perform early exit: remove thrusting from if( thrusting && accel == 0 ).

Testing

Just checked that save from #45473 doesn't crash anymore and ran make check

Additional context

I just fixed exact place where the crash occurs.
The fix is kind of trivial but yet can be done in many other ways like move this check into some other place or even special handling zero-accelerated vehicles and I'm not very familiar with the codebase so I can miss something important.

It fixes rare case when all engines produce zero power due to no fuel or
exhausted character and target cruise speed is set less than current.

fixes CleverRaven#45473
@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Vehicles Vehicles, parts, mechanics & interactions labels Nov 25, 2020
@kevingranade
Copy link
Member

Thanks for finding the source, this unfortunately a pretty tricky chump of logic, so we need to be careful.

Just summarizing some stuff:
accel == 0 at that line implies thrusting == false
This means that the "sgn" of acceleration and velocity are different, meaning we're braking instead of accelerating.
accel == 0 means vehicle::acceleration() returned 0, the most obvious cause of which is that the vehicle is unfueled and/or the engines are off.
The only outcome of calculating load is to put strain on the engine, but in this scenario the engine is braking, so that won't happen anyway.

So tl;dr, looks like your change does the right thing.

@kevingranade kevingranade merged commit 9de3be8 into CleverRaven:master Nov 25, 2020
@CAHEK7 CAHEK7 deleted the fix_sigfpe_45473 branch November 25, 2020 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SIGFPE: Arithmetical error in vehicle::thrust
3 participants