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

Prevent undefined behaviour modifying bionic power #36686

Merged
merged 1 commit into from
Jan 4, 2020

Conversation

anothersimulacrum
Copy link
Member

@anothersimulacrum anothersimulacrum commented Jan 4, 2020

Summary

SUMMARY: None

Purpose of change

#34684 (comment)
I needed a way to instantly modify bionic power so that I could test that this fix worked.

Describe the solution

Add a Debug Bionic Powergen trait, which when activated prompts you for a number by which to modify bionic power.
Cast npower and power_level to int64_t, then add them and compare them with max power level to prevent integer overflow in Character::mod_bionic_power()

Describe alternatives you've considered

Leaving the UB there, like I did for the past 30 or so days.

Testing

Create a character, install enough bionic power storage to push your max power level to the max. Debug in the Debug Bionic Powergen trait, activate it and then increase your power by 2000000000 using it. Repeat, and notice that power does not overflow (where it is clamped to 0), and instead your power stays at the max power level.
If you did that without these changes and the changes in #34684 applied, it will overflow and be clamped to 0.

As it turns out, the check to prevent integer overflow when modifying
bionic power is undefined behaviour. Whoops! Let's avoid that.
@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Items: Battery / UPS Electric power management Bionics CBM (Compact Bionic Modules) labels Jan 4, 2020
@ZhilkinSerg ZhilkinSerg merged commit d0a3632 into CleverRaven:master Jan 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bionics CBM (Compact Bionic Modules) [C++] Changes (can be) made in C++. Previously named `Code` Items: Battery / UPS Electric power management [JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants