Avoid int overflow in calling unit::from_joule and unit::from_kilojoule

CleverRaven · Feb 4, 2024 · fdba69c · fdba69c
1 parent 3c15663
commit fdba69c
Show file tree

Hide file tree

Showing 16 changed files with 40 additions and 29 deletions.
diff --git a/src/activity_handlers.cpp b/src/activity_handlers.cpp
@@ -3889,7 +3889,7 @@ void activity_handlers::spellcasting_finish( player_activity *act, Character *yo
                         you->mod_stamina( -cost );
                         break;
                     case magic_energy_type::bionic:
-                        you->mod_power_level( -units::from_kilojoule( cost ) );
+                        you->mod_power_level( -units::from_kilojoule( static_cast<std::int64_t>( cost ) ) );
                         break;
                     case magic_energy_type::hp:
                         blood_magic( you, cost );

diff --git a/src/bionics.cpp b/src/bionics.cpp
@@ -694,7 +694,7 @@ void npc::check_or_use_weapon_cbm( const bionic_id &cbm_id )
     int ammo_count = weap.ammo_remaining( this );
     const units::energy ups_drain = weap.get_gun_ups_drain();
     if( ups_drain > 0_kJ ) {
-        ammo_count = units::from_kilojoule( ammo_count ) / ups_drain;
+        ammo_count = units::from_kilojoule( static_cast<std::int64_t>( ammo_count ) ) / ups_drain;
     }
 
     // the weapon value from `weapon_value` may be different from `npc_attack_rating`
@@ -1193,7 +1193,7 @@ bool Character::activate_bionic( bionic &bio, bool eff_only, bool *close_bionics
             }
             ctr.charges = units::to_kilojoule( get_power_level() );
             int power_use = invoke_item( &ctr );
-            mod_power_level( units::from_kilojoule( -power_use ) );
+            mod_power_level( units::from_kilojoule( static_cast<std::int64_t>( -power_use ) ) );
             bio.powered = ctr.active;
         } else {
             bio.powered = g->remoteveh() != nullptr || !get_value( "remote_controlling" ).empty();

diff --git a/src/character.cpp b/src/character.cpp
@@ -7060,7 +7060,7 @@ void Character::update_stamina( int turns )
             stamina_recovery += bonus;
             bonus /= 10;
             bonus = std::max( bonus, 1 );
-            mod_power_level( units::from_kilojoule( -bonus ) );
+            mod_power_level( units::from_kilojoule( static_cast<std::int64_t>( -bonus ) ) );
         }
     }
 
@@ -9635,7 +9635,8 @@ units::energy Character::available_ups() const
 
     if( is_mounted() && mounted_creature.get()->has_flag( mon_flag_RIDEABLE_MECH ) ) {
         auto *mons = mounted_creature.get();
-        available_charges += units::from_kilojoule( mons->battery_item->ammo_remaining() );
+        available_charges += units::from_kilojoule( static_cast<std::int64_t>
+                             ( mons->battery_item->ammo_remaining() ) );
     }
 
     bool has_bio_powered_ups = false;
@@ -9649,7 +9650,7 @@ units::energy Character::available_ups() const
     }
 
     cache_visit_items_with( flag_IS_UPS, [&available_charges]( const item & it ) {
-        available_charges += units::from_kilojoule( it.ammo_remaining() );
+        available_charges += units::from_kilojoule( static_cast<std::int64_t>( it.ammo_remaining() ) );
     } );
 
     return available_charges;
@@ -9716,7 +9717,7 @@ std::list<item> Character::use_charges( const itype_id &what, int qty, const int
     } else if( what == itype_UPS ) {
         // Fairly sure that nothing comes here. But handle it anyways.
         debugmsg( _( "This UPS use needs updating.  Create issue on github." ) );
-        consume_ups( units::from_kilojoule( qty ), radius );
+        consume_ups( units::from_kilojoule( static_cast<std::int64_t>( qty ) ), radius );
         return res;
     }
 
@@ -9749,7 +9750,7 @@ std::list<item> Character::use_charges( const itype_id &what, int qty, const int
     }
 
     if( has_tool_with_UPS ) {
-        consume_ups( units::from_kilojoule( qty ), radius );
+        consume_ups( units::from_kilojoule( static_cast<std::int64_t>( qty ) ), radius );
     }
 
     return res;

diff --git a/src/consumption.cpp b/src/consumption.cpp
@@ -524,7 +524,8 @@ std::pair<int, int> Character::fun_for( const item &comest, bool ignore_already_
     }
 
     if( fun < 0 && has_active_bionic( bio_taste_blocker ) &&
-        get_power_level() > units::from_kilojoule( std::abs( comest.get_comestible_fun() ) ) ) {
+        get_power_level() > units::from_kilojoule( static_cast<std::int64_t>( std::abs(
+                    comest.get_comestible_fun() ) ) ) ) {
         fun = 0;
     }
 
@@ -1122,8 +1123,10 @@ static bool eat( item &food, Character &you, bool force )
     }
 
     if( you.has_active_bionic( bio_taste_blocker ) && food.get_comestible_fun() < 0 &&
-        you.get_power_level() > units::from_kilojoule( std::abs( food.get_comestible_fun() ) ) ) {
-        you.mod_power_level( units::from_kilojoule( food.get_comestible_fun() ) );
+        you.get_power_level() > units::from_kilojoule( static_cast<std::int64_t>( std::abs(
+                    food.get_comestible_fun() ) ) ) ) {
+        you.mod_power_level( units::from_kilojoule( static_cast<std::int64_t>
+                             ( food.get_comestible_fun() ) ) );
     }
 
     if( food.has_flag( flag_FUNGAL_VECTOR ) && !you.has_trait( trait_M_IMMUNE ) ) {

diff --git a/src/crafting.cpp b/src/crafting.cpp
@@ -2373,7 +2373,7 @@ void Character::consume_tools( map &m, const comp_selection<tool_comp> &tool, in
         m.use_charges( reachable_pts, tool.comp.type, quantity, return_true<item>, bcp );
         // Map::use_charges() does not handle UPS charges.
         if( quantity > 0 ) {
-            m.consume_ups( reachable_pts, units::from_kilojoule( quantity ) );
+            m.consume_ups( reachable_pts, units::from_kilojoule( static_cast<std::int64_t>( quantity ) ) );
         }
     }
 

diff --git a/src/explosion.cpp b/src/explosion.cpp
@@ -751,7 +751,8 @@ void emp_blast( const tripoint &p )
             add_msg( m_bad, _( "The EMP blast drains your power." ) );
             int max_drain = ( player_character.get_power_level() > 1000_kJ ? 1000 : units::to_kilojoule(
                                   player_character.get_power_level() ) );
-            player_character.mod_power_level( units::from_kilojoule( -rng( 1 + max_drain / 3, max_drain ) ) );
+            player_character.mod_power_level( units::from_kilojoule( static_cast<std::int64_t>( -rng(
+                                                  1 + max_drain / 3, max_drain ) ) ) );
         }
         // TODO: More effects?
         //e-handcuffs effects

diff --git a/src/item.cpp b/src/item.cpp
@@ -10640,7 +10640,7 @@ units::energy item::energy_remaining( const Character *carrier ) const
     if( is_magazine() ) {
         for( const item *e : contents.all_items_top( pocket_type::MAGAZINE ) ) {
             if( e->typeId() == itype_battery ) {
-                ret += units::from_kilojoule( e->charges );
+                ret += units::from_kilojoule( static_cast<std::int64_t>( e->charges ) );
             }
         }
     }
@@ -10785,7 +10785,7 @@ int item::ammo_consume( int qty, const tripoint &pos, Character *carrier )
     // Guns handle energy in energy_consume()
     if( carrier != nullptr && type->tool &&
         ( has_flag( flag_USE_UPS ) || has_flag( flag_USES_BIONIC_POWER ) ) ) {
-        units::energy wanted_energy = units::from_kilojoule( qty );
+        units::energy wanted_energy = units::from_kilojoule( static_cast<std::int64_t>( qty ) );
 
         if( has_flag( flag_USE_UPS ) ) {
             wanted_energy -= carrier->consume_ups( wanted_energy );
@@ -10817,7 +10817,7 @@ units::energy item::energy_consume( units::energy qty, const tripoint &pos, Char
     // Consume battery(ammo) and other fuel (if allowed)
     if( is_battery() || fuel_efficiency >= 0 ) {
         int consumed_kj = contents.ammo_consume( units::to_kilojoule( qty ), pos, fuel_efficiency );
-        qty -= units::from_kilojoule( consumed_kj );
+        qty -= units::from_kilojoule( static_cast<std::int64_t>( consumed_kj ) );
         // fix negative quantity
         if( qty < 0_J ) {
             qty = 0_J;
@@ -10845,7 +10845,7 @@ units::energy item::energy_consume( units::energy qty, const tripoint &pos, Char
     // Should happen only if battery powered and energy per shot is not integer kJ.
     if( qty > 0_kJ && is_battery() ) {
         int consumed_kj = contents.ammo_consume( 1, pos );
-        qty -= units::from_kilojoule( consumed_kj );
+        qty -= units::from_kilojoule( static_cast<std::int64_t>( consumed_kj ) );
     }
 
     return wanted_energy - qty;

diff --git a/src/item_contents.cpp b/src/item_contents.cpp
@@ -1230,7 +1230,8 @@ int item_contents::ammo_consume( int qty, const tripoint &pos, float fuel_effici
             if( !pocket.empty() && pocket.front().is_fuel() && fuel_efficiency >= 0 ) {
                 // if using fuel instead of battery, everything is in kJ
                 // charges is going to be the energy needed over the energy in 1 unit of fuel * the efficiency of the generator
-                int charges_used = ceil( static_cast<float>( units::from_kilojoule( qty ).value() ) / (
+                int charges_used = ceil( static_cast<float>( units::from_kilojoule( static_cast<std::int64_t>
+                                         ( qty ) ).value() ) / (
                                              static_cast<float>( pocket.front().fuel_energy().value() ) * fuel_efficiency ) );
 
                 const int res = pocket.ammo_consume( charges_used );

diff --git a/src/magic.cpp b/src/magic.cpp
@@ -2188,7 +2188,7 @@ bool known_magic::has_enough_energy( const Character &guy, const spell &sp ) con
         case magic_energy_type::mana:
             return available_mana() >= cost;
         case magic_energy_type::bionic:
-            return guy.get_power_level() >= units::from_kilojoule( cost );
+            return guy.get_power_level() >= units::from_kilojoule( static_cast<std::int64_t>( cost ) );
         case magic_energy_type::stamina:
             return guy.get_stamina() >= cost;
         case magic_energy_type::hp:

diff --git a/src/magic_spell_effect.cpp b/src/magic_spell_effect.cpp
@@ -1146,7 +1146,7 @@ void spell_effect::recover_energy( const spell &sp, Creature &caster, const trip
         you->mod_fatigue( -healing );
     } else if( energy_source == "BIONIC" ) {
         if( healing > 0 ) {
-            you->mod_power_level( units::from_kilojoule( healing ) );
+            you->mod_power_level( units::from_kilojoule( static_cast<std::int64_t>( healing ) ) );
         } else {
             you->mod_stamina( healing );
         }

diff --git a/src/monster.cpp b/src/monster.cpp
@@ -2901,7 +2901,7 @@ units::energy monster::use_mech_power( units::energy amt )
     const int max_drain = battery_item->ammo_remaining();
     const int consumption = std::min( static_cast<int>( units::to_kilojoule( amt ) ), max_drain );
     battery_item->ammo_consume( consumption, pos(), nullptr );
-    return units::from_kilojoule( consumption );
+    return units::from_kilojoule( static_cast<std::int64_t>( consumption ) );
 }
 
 int monster::mech_str_addition() const

diff --git a/src/move_mode.cpp b/src/move_mode.cpp
@@ -183,7 +183,7 @@ float move_mode::move_speed_mult() const
 
 units::energy move_mode::mech_power_use() const
 {
-    return units::from_kilojoule( _mech_power_use );
+    return units::from_kilojoule( static_cast<std::int64_t>( _mech_power_use ) );
 }
 
 int move_mode::swim_speed_mod() const

diff --git a/src/vehicle.cpp b/src/vehicle.cpp
@@ -4843,7 +4843,8 @@ void vehicle::consume_fuel( int load, bool idling )
         }
         // decreased stamina burn scalable with load
         if( player_character.has_active_bionic( bio_jointservo ) ) {
-            player_character.mod_power_level( units::from_kilojoule( -std::max( eff_load / 20, 1 ) ) );
+            player_character.mod_power_level( units::from_kilojoule( static_cast<std::int64_t>( -std::max(
+                                                  eff_load / 20, 1 ) ) ) );
             mod -= std::max( eff_load / 5, 5 );
         }
 

diff --git a/src/wish.cpp b/src/wish.cpp
@@ -429,7 +429,7 @@ void debug_menu::wishbionics( Character *you )
                 int new_value = 0;
                 if( query_int( new_value, _( "Set the value to (in kJ)?  Currently: %s" ),
                                units::display( power_max ) ) ) {
-                    you->set_max_power_level( units::from_kilojoule( new_value ) );
+                    you->set_max_power_level( units::from_kilojoule( static_cast<std::int64_t>( new_value ) ) );
                     you->set_power_level( you->get_power_level() );
                 }
                 break;
@@ -438,7 +438,7 @@ void debug_menu::wishbionics( Character *you )
                 int new_value = 0;
                 if( query_int( new_value, _( "Set the value to (in J)?  Currently: %s" ),
                                units::display( power_max ) ) ) {
-                    you->set_max_power_level( units::from_joule( new_value ) );
+                    you->set_max_power_level( units::from_joule( static_cast<std::int64_t>( new_value ) ) );
                     you->set_power_level( you->get_power_level() );
                 }
                 break;
@@ -447,15 +447,15 @@ void debug_menu::wishbionics( Character *you )
                 int new_value = 0;
                 if( query_int( new_value, _( "Set the value to (in kJ)?  Currently: %s" ),
                                units::display( power_level ) ) ) {
-                    you->set_power_level( units::from_kilojoule( new_value ) );
+                    you->set_power_level( units::from_kilojoule( static_cast<std::int64_t>( new_value ) ) );
                 }
                 break;
             }
             case 6: {
                 int new_value = 0;
                 if( query_int( new_value, _( "Set the value to (in J)?  Currently: %s" ),
                                units::display( power_level ) ) ) {
-                    you->set_power_level( units::from_joule( new_value ) );
+                    you->set_power_level( units::from_joule( static_cast<std::int64_t>( new_value ) ) );
                 }
                 break;
             }

diff --git a/tests/food_fun_for_test.cpp b/tests/food_fun_for_test.cpp
@@ -427,7 +427,8 @@ TEST_CASE( "fun_for_bionic_bio_taste_blocker", "[fun_for][food][bionic]" )
                 // Needs 1 kJ per negative fun unit to nullify bad taste
                 dummy.set_power_level( 10_kJ );
                 REQUIRE( garlic_fun < -10 );
-                REQUIRE_FALSE( dummy.get_power_level() > units::from_kilojoule( std::abs( garlic_fun ) ) );
+                REQUIRE_FALSE( dummy.get_power_level() > units::from_kilojoule( static_cast<std::int64_t>( std::abs(
+                                   garlic_fun ) ) ) );
 
                 THEN( "the bad taste remains" ) {
                     actual_fun = dummy.fun_for( garlic );
@@ -438,7 +439,8 @@ TEST_CASE( "fun_for_bionic_bio_taste_blocker", "[fun_for][food][bionic]" )
             WHEN( "it has enough power" ) {
                 REQUIRE( garlic_fun >= -20 );
                 dummy.set_power_level( 20_kJ );
-                REQUIRE( dummy.get_power_level() > units::from_kilojoule( std::abs( garlic_fun ) ) );
+                REQUIRE( dummy.get_power_level() > units::from_kilojoule( static_cast<std::int64_t>( std::abs(
+                             garlic_fun ) ) ) );
 
                 THEN( "the bad taste is nullified" ) {
                     actual_fun = dummy.fun_for( garlic );

diff --git a/tools/clang-tidy-plugin/UnitOverflowCheck.cpp b/tools/clang-tidy-plugin/UnitOverflowCheck.cpp
@@ -57,7 +57,9 @@ void UnitOverflowCheck::check( const ast_matchers::MatchFinder::MatchResult &Res
             minVal = llvm::APInt::getSignedMinValue( width ).getSExtValue();
             maxVal = llvm::APInt::getSignedMaxValue( width ).getSExtValue();
         } else {
+            // NOLINTNEXTLINE(clang-analyzer-core.UndefinedBinaryOperatorResult)
             minVal = llvm::APInt::getMinValue( width ).getSExtValue();
+            // NOLINTNEXTLINE(clang-analyzer-core.UndefinedBinaryOperatorResult)
             maxVal = llvm::APInt::getMaxValue( width ).getSExtValue();
         }
         const std::int64_t multiplier = FunctionAndQuantityTypes.at( functionName ).ConversionFactor;