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

SIGFPE: Arithmetical error in vehicle::thrust #45473

Closed
CAHEK7 opened this issue Nov 17, 2020 · 3 comments · Fixed by #45626
Closed

SIGFPE: Arithmetical error in vehicle::thrust #45473

CAHEK7 opened this issue Nov 17, 2020 · 3 comments · Fixed by #45626
Labels
<Crash / Freeze> Fatal bug that results in hangs or crashes. (S2 - Confirmed) Bug that's been confirmed to exist

Comments

@CAHEK7
Copy link
Contributor

CAHEK7 commented Nov 17, 2020

Describe the bug

I've got SIGFPE when applied handbrake for electrobike with empty batteries and exhausted character. (or push some random keys when running from the horde)

Steps To Reproduce

Reproduction steps are posted in the first comment.

Expected behavior

Handle integer division correctly.

Versions and configuration

  • OS: Ubuntu 18.04
  • Game Version: 0.E-7497-g48b6f14
  • Graphics version: Tiles
  • Ingame language: Russian
  • Mods loaded: bunch of, but it doesn't matter

Additional context

Backtrace:

    ./cataclysm-tiles() [0xbeb402]
    ./cataclysm-tiles() [0xbeb584]
    /lib/x86_64-linux-gnu/libc.so.6(+0x3efd0) [0x7f9778e8afd0]
    ./cataclysm-tiles(_ZN7vehicle6thrustEii+0x30f) [0x13bcaa3]
    ./cataclysm-tiles(_ZN7vehicle10gain_movesEv+0xe5) [0x13a88db]
    ./cataclysm-tiles(_ZN3map7vehmoveEv+0x112) [0xf291a6]
    ./cataclysm-tiles(_ZN4game7do_turnEv+0x86a) [0xceebfc]
    ./cataclysm-tiles(main+0x139c) [0x91f23a]
    /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7f9778e6db97]
    ./cataclysm-tiles(_start+0x29) [0x9ef539]

    Attempting to repeat stack trace using debug symbols…
    debug_write_backtrace(std::ostream&)
    ??:?
    log_crash
    crash.cpp:?
    signal_handler
    crash.cpp:?
    ??
    ??:0
    vehicle::thrust(int, int)
    ??:?
    vehicle::gain_moves()
    ??:?
    map::vehmove()
    ??:?
    game::do_turn()
    ??:?
    main
    ??:?
    ??
    ??:0
    _start
    ??:?

Disasm:

 13bca8f:   sub    $0x7d0,%eax      ; eax -= 20*100
 13bca94:   mov    $0x0,%edx
 13bca99:   cmovs  %edx,%eax        ; eax = eax < 0 ? 0 : eax
 13bca9c:   imul   $0x3e8,%eax,%eax ; eax *= 1000
 13bcaa2:   cltd
 13bcaa3:   idivl  (%rsp)           ; eax /= local_var

This is exactly this line of code:

load = 1000 * std::max( 0, std::abs( vel_inc ) - max_brake ) / accel;

I would suggest to add && accel != 0 into this condition

if( cruise_on ) {

but I'm not very familiar with the code, just performed quick investigation of what has happened.

@anothersimulacrum anothersimulacrum added (S1 - Need confirmation) Report waiting on confirmation of reproducibility <Crash / Freeze> Fatal bug that results in hangs or crashes. labels Nov 17, 2020
@CAHEK7
Copy link
Contributor Author

CAHEK7 commented Nov 21, 2020

Here is the reproduction (on the latest master 0.E-7748-g55fac0a69d).
You need to drive a couple of times with "." until you emptied out the batteries and started to decelerate and then set cruise speed less than current speed (kind of 6/26) and drive one more time.
test.zip
Mods are dda, no_npc_food, package_bionic_professions and Only_Wildlife, nothing can affect vehicle code.

@CAHEK7
Copy link
Contributor Author

CAHEK7 commented Nov 21, 2020

Also there is another special check for accel == 0 case:

// FIXME: Long-term plan is to change the fuel consumption

Seems like it's a long-lasting problem which happens there and here.
Meantime I've found out strange behavior for dual-engine transport like electrobike: when the character is out of stamina it doesn't switch to electrical motor automatically and just stops. Only if you turn off muscle engine manually, then you can start moving again. Maybe it is related, vehicle doesn't switch to only-muscle engine and accel calculation doesn't take it into account, but it seems strange because in this case you just have to decelerate to 0 ad stop. I'm just guessing and this can be wrong assumption.

@CAHEK7
Copy link
Contributor Author

CAHEK7 commented Nov 25, 2020

Ok, that happens when character is too weak (because of exhaustion or any other kind of debuff) to beat bicycle alternator, so both electrical engine and muscle engine provide zero power and thus zero acceleration and finally when we set cruise speed less than current speed we get thrusting == false and hit this case. I'll submit PR.

CAHEK7 added a commit to CAHEK7/Cataclysm-DDA that referenced this issue Nov 25, 2020
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 (S2 - Confirmed) Bug that's been confirmed to exist and removed (S1 - Need confirmation) Report waiting on confirmation of reproducibility labels Nov 25, 2020
kevingranade pushed a commit that referenced this issue Nov 25, 2020
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 #45473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Crash / Freeze> Fatal bug that results in hangs or crashes. (S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants