From f24286a1ae4997d7a3ca72a33b9ea31c04c0de79 Mon Sep 17 00:00:00 2001 From: PeterN Date: Fri, 4 Nov 2022 07:15:59 +0000 Subject: [PATCH] Fix: Ensure 31-bit shifts are unsigned. (#10128) Shifting a signed 32-bit integer by 31 bits is undefined behaviour. A few more than necessary are switched to unsigned for consistentency. --- src/map_type.h | 8 +-- src/newgrf.cpp | 166 +++++++++++++++++++++++------------------------ src/newgrf_spritegroup.h | 2 +- src/slope_type.h | 2 +- src/table/sprites.h | 12 ++-- src/widget.cpp | 2 +- 6 files changed, 96 insertions(+), 96 deletions(-) diff --git a/src/map_type.h b/src/map_type.h index ba5af450a4..89012d6753 100644 --- a/src/map_type.h +++ b/src/map_type.h @@ -60,10 +60,10 @@ struct TileIndexDiffC { }; /** Minimal and maximal map width and height */ -static const uint MIN_MAP_SIZE_BITS = 6; ///< Minimal size of map is equal to 2 ^ MIN_MAP_SIZE_BITS -static const uint MAX_MAP_SIZE_BITS = 12; ///< Maximal size of map is equal to 2 ^ MAX_MAP_SIZE_BITS -static const uint MIN_MAP_SIZE = 1 << MIN_MAP_SIZE_BITS; ///< Minimal map size = 64 -static const uint MAX_MAP_SIZE = 1 << MAX_MAP_SIZE_BITS; ///< Maximal map size = 4096 +static const uint MIN_MAP_SIZE_BITS = 6; ///< Minimal size of map is equal to 2 ^ MIN_MAP_SIZE_BITS +static const uint MAX_MAP_SIZE_BITS = 12; ///< Maximal size of map is equal to 2 ^ MAX_MAP_SIZE_BITS +static const uint MIN_MAP_SIZE = 1U << MIN_MAP_SIZE_BITS; ///< Minimal map size = 64 +static const uint MAX_MAP_SIZE = 1U << MAX_MAP_SIZE_BITS; ///< Maximal map size = 4096 /** * Approximation of the length of a straight track, relative to a diagonal diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 0a3dff80bb..09e8c6fd4e 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -8365,89 +8365,89 @@ static void GRFUnsafe(ByteReader *buf) /** Initialize the TTDPatch flags */ static void InitializeGRFSpecial() { - _ttdpatch_flags[0] = ((_settings_game.station.never_expire_airports ? 1 : 0) << 0x0C) // keepsmallairport - | (1 << 0x0D) // newairports - | (1 << 0x0E) // largestations - | ((_settings_game.construction.max_bridge_length > 16 ? 1 : 0) << 0x0F) // longbridges - | (0 << 0x10) // loadtime - | (1 << 0x12) // presignals - | (1 << 0x13) // extpresignals - | ((_settings_game.vehicle.never_expire_vehicles ? 1 : 0) << 0x16) // enginespersist - | (1 << 0x1B) // multihead - | (1 << 0x1D) // lowmemory - | (1 << 0x1E); // generalfixes - - _ttdpatch_flags[1] = ((_settings_game.economy.station_noise_level ? 1 : 0) << 0x07) // moreairports - based on units of noise - | (1 << 0x08) // mammothtrains - | (1 << 0x09) // trainrefit - | (0 << 0x0B) // subsidiaries - | ((_settings_game.order.gradual_loading ? 1 : 0) << 0x0C) // gradualloading - | (1 << 0x12) // unifiedmaglevmode - set bit 0 mode. Not revelant to OTTD - | (1 << 0x13) // unifiedmaglevmode - set bit 1 mode - | (1 << 0x14) // bridgespeedlimits - | (1 << 0x16) // eternalgame - | (1 << 0x17) // newtrains - | (1 << 0x18) // newrvs - | (1 << 0x19) // newships - | (1 << 0x1A) // newplanes - | ((_settings_game.construction.train_signal_side == 1 ? 1 : 0) << 0x1B) // signalsontrafficside - | ((_settings_game.vehicle.disable_elrails ? 0 : 1) << 0x1C); // electrifiedrailway - - _ttdpatch_flags[2] = (1 << 0x01) // loadallgraphics - obsolote - | (1 << 0x03) // semaphores - | (1 << 0x0A) // newobjects - | (0 << 0x0B) // enhancedgui - | (0 << 0x0C) // newagerating - | ((_settings_game.construction.build_on_slopes ? 1 : 0) << 0x0D) // buildonslopes - | (1 << 0x0E) // fullloadany - | (1 << 0x0F) // planespeed - | (0 << 0x10) // moreindustriesperclimate - obsolete - | (0 << 0x11) // moretoylandfeatures - | (1 << 0x12) // newstations - | (1 << 0x13) // tracktypecostdiff - | (1 << 0x14) // manualconvert - | ((_settings_game.construction.build_on_slopes ? 1 : 0) << 0x15) // buildoncoasts - | (1 << 0x16) // canals - | (1 << 0x17) // newstartyear - | ((_settings_game.vehicle.freight_trains > 1 ? 1 : 0) << 0x18) // freighttrains - | (1 << 0x19) // newhouses - | (1 << 0x1A) // newbridges - | (1 << 0x1B) // newtownnames - | (1 << 0x1C) // moreanimation - | ((_settings_game.vehicle.wagon_speed_limits ? 1 : 0) << 0x1D) // wagonspeedlimits - | (1 << 0x1E) // newshistory - | (0 << 0x1F); // custombridgeheads - - _ttdpatch_flags[3] = (0 << 0x00) // newcargodistribution - | (1 << 0x01) // windowsnap - | ((_settings_game.economy.allow_town_roads || _generating_world ? 0 : 1) << 0x02) // townbuildnoroad - | (1 << 0x03) // pathbasedsignalling - | (0 << 0x04) // aichoosechance - | (1 << 0x05) // resolutionwidth - | (1 << 0x06) // resolutionheight - | (1 << 0x07) // newindustries - | ((_settings_game.order.improved_load ? 1 : 0) << 0x08) // fifoloading - | (0 << 0x09) // townroadbranchprob - | (0 << 0x0A) // tempsnowline - | (1 << 0x0B) // newcargo - | (1 << 0x0C) // enhancemultiplayer - | (1 << 0x0D) // onewayroads - | (1 << 0x0E) // irregularstations - | (1 << 0x0F) // statistics - | (1 << 0x10) // newsounds - | (1 << 0x11) // autoreplace - | (1 << 0x12) // autoslope - | (0 << 0x13) // followvehicle - | (1 << 0x14) // trams - | (0 << 0x15) // enhancetunnels - | (1 << 0x16) // shortrvs - | (1 << 0x17) // articulatedrvs - | ((_settings_game.vehicle.dynamic_engines ? 1 : 0) << 0x18) // dynamic engines - | (1 << 0x1E) // variablerunningcosts - | (1 << 0x1F); // any switch is on - - _ttdpatch_flags[4] = (1 << 0x00) // larger persistent storage - | ((_settings_game.economy.inflation ? 1 : 0) << 0x01); // inflation is on + _ttdpatch_flags[0] = ((_settings_game.station.never_expire_airports ? 1U : 0U) << 0x0C) // keepsmallairport + | (1U << 0x0D) // newairports + | (1U << 0x0E) // largestations + | ((_settings_game.construction.max_bridge_length > 16 ? 1U : 0U) << 0x0F) // longbridges + | (0U << 0x10) // loadtime + | (1U << 0x12) // presignals + | (1U << 0x13) // extpresignals + | ((_settings_game.vehicle.never_expire_vehicles ? 1U : 0U) << 0x16) // enginespersist + | (1U << 0x1B) // multihead + | (1U << 0x1D) // lowmemory + | (1U << 0x1E); // generalfixes + + _ttdpatch_flags[1] = ((_settings_game.economy.station_noise_level ? 1U : 0U) << 0x07) // moreairports - based on units of noise + | (1U << 0x08) // mammothtrains + | (1U << 0x09) // trainrefit + | (0U << 0x0B) // subsidiaries + | ((_settings_game.order.gradual_loading ? 1U : 0U) << 0x0C) // gradualloading + | (1U << 0x12) // unifiedmaglevmode - set bit 0 mode. Not revelant to OTTD + | (1U << 0x13) // unifiedmaglevmode - set bit 1 mode + | (1U << 0x14) // bridgespeedlimits + | (1U << 0x16) // eternalgame + | (1U << 0x17) // newtrains + | (1U << 0x18) // newrvs + | (1U << 0x19) // newships + | (1U << 0x1A) // newplanes + | ((_settings_game.construction.train_signal_side == 1 ? 1U : 0U) << 0x1B) // signalsontrafficside + | ((_settings_game.vehicle.disable_elrails ? 0U : 1U) << 0x1C); // electrifiedrailway + + _ttdpatch_flags[2] = (1U << 0x01) // loadallgraphics - obsolote + | (1U << 0x03) // semaphores + | (1U << 0x0A) // newobjects + | (0U << 0x0B) // enhancedgui + | (0U << 0x0C) // newagerating + | ((_settings_game.construction.build_on_slopes ? 1U : 0U) << 0x0D) // buildonslopes + | (1U << 0x0E) // fullloadany + | (1U << 0x0F) // planespeed + | (0U << 0x10) // moreindustriesperclimate - obsolete + | (0U << 0x11) // moretoylandfeatures + | (1U << 0x12) // newstations + | (1U << 0x13) // tracktypecostdiff + | (1U << 0x14) // manualconvert + | ((_settings_game.construction.build_on_slopes ? 1U : 0U) << 0x15) // buildoncoasts + | (1U << 0x16) // canals + | (1U << 0x17) // newstartyear + | ((_settings_game.vehicle.freight_trains > 1 ? 1U : 0U) << 0x18) // freighttrains + | (1U << 0x19) // newhouses + | (1U << 0x1A) // newbridges + | (1U << 0x1B) // newtownnames + | (1U << 0x1C) // moreanimation + | ((_settings_game.vehicle.wagon_speed_limits ? 1U : 0U) << 0x1D) // wagonspeedlimits + | (1U << 0x1E) // newshistory + | (0U << 0x1F); // custombridgeheads + + _ttdpatch_flags[3] = (0U << 0x00) // newcargodistribution + | (1U << 0x01) // windowsnap + | ((_settings_game.economy.allow_town_roads || _generating_world ? 0U : 1U) << 0x02) // townbuildnoroad + | (1U << 0x03) // pathbasedsignalling + | (0U << 0x04) // aichoosechance + | (1U << 0x05) // resolutionwidth + | (1U << 0x06) // resolutionheight + | (1U << 0x07) // newindustries + | ((_settings_game.order.improved_load ? 1U : 0U) << 0x08) // fifoloading + | (0U << 0x09) // townroadbranchprob + | (0U << 0x0A) // tempsnowline + | (1U << 0x0B) // newcargo + | (1U << 0x0C) // enhancemultiplayer + | (1U << 0x0D) // onewayroads + | (1U << 0x0E) // irregularstations + | (1U << 0x0F) // statistics + | (1U << 0x10) // newsounds + | (1U << 0x11) // autoreplace + | (1U << 0x12) // autoslope + | (0U << 0x13) // followvehicle + | (1U << 0x14) // trams + | (0U << 0x15) // enhancetunnels + | (1U << 0x16) // shortrvs + | (1U << 0x17) // articulatedrvs + | ((_settings_game.vehicle.dynamic_engines ? 1U : 0U) << 0x18) // dynamic engines + | (1U << 0x1E) // variablerunningcosts + | (1U << 0x1F); // any switch is on + + _ttdpatch_flags[4] = (1U << 0x00) // larger persistent storage + | ((_settings_game.economy.inflation ? 1U : 0U) << 0x01); // inflation is on } /** Reset and clear all NewGRF stations */ diff --git a/src/newgrf_spritegroup.h b/src/newgrf_spritegroup.h index b172667612..af28950a88 100644 --- a/src/newgrf_spritegroup.h +++ b/src/newgrf_spritegroup.h @@ -50,7 +50,7 @@ struct ResolverObject; /* SPRITE_WIDTH is 24. ECS has roughly 30 sprite groups per real sprite. * Adding an 'extra' margin would be assuming 64 sprite groups per real * sprite. 64 = 2^6, so 2^30 should be enough (for now) */ -typedef Pool SpriteGroupPool; +typedef Pool SpriteGroupPool; extern SpriteGroupPool _spritegroup_pool; /* Common wrapper for all the different sprite group types */ diff --git a/src/slope_type.h b/src/slope_type.h index ea2ed1aca5..86faa7bb68 100644 --- a/src/slope_type.h +++ b/src/slope_type.h @@ -81,7 +81,7 @@ DECLARE_ENUM_AS_BIT_SET(Slope) * Helper for creating a bitset of slopes. * @param x The slope to convert into a bitset. */ -#define M(x) (1 << (x)) +#define M(x) (1U << (x)) /** Constant bitset with safe slopes for building a level crossing. */ static const uint32 VALID_LEVEL_CROSSING_SLOPES = M(SLOPE_SEN) | M(SLOPE_ENW) | M(SLOPE_NWS) | M(SLOPE_NS) | M(SLOPE_WSE) | M(SLOPE_EW) | M(SLOPE_FLAT); #undef M diff --git a/src/table/sprites.h b/src/table/sprites.h index 16240d4bbc..b0e61b5d02 100644 --- a/src/table/sprites.h +++ b/src/table/sprites.h @@ -1542,18 +1542,18 @@ enum Modifiers { * @see SpriteSetup */ enum SpriteMasks { - MAX_SPRITES = 1 << SPRITE_WIDTH, ///< Maximum number of sprites that can be loaded at a given time + MAX_SPRITES = 1U << SPRITE_WIDTH, ///< Maximum number of sprites that can be loaded at a given time SPRITE_MASK = MAX_SPRITES - 1, ///< The mask to for the main sprite - MAX_PALETTES = 1 << PALETTE_WIDTH, + MAX_PALETTES = 1U << PALETTE_WIDTH, PALETTE_MASK = MAX_PALETTES - 1, ///< The mask for the auxiliary sprite (the one that takes care of recolouring) }; -static_assert( (1 << TRANSPARENT_BIT & SPRITE_MASK) == 0 ); -static_assert( (1 << RECOLOUR_BIT & SPRITE_MASK) == 0 ); +static_assert( (1U << TRANSPARENT_BIT & SPRITE_MASK) == 0 ); +static_assert( (1U << RECOLOUR_BIT & SPRITE_MASK) == 0 ); static_assert( TRANSPARENT_BIT != RECOLOUR_BIT ); -static_assert( (1 << TRANSPARENT_BIT & PALETTE_MASK) == 0); -static_assert( (1 << RECOLOUR_BIT & PALETTE_MASK) == 0 ); +static_assert( (1U << TRANSPARENT_BIT & PALETTE_MASK) == 0 ); +static_assert( (1U << RECOLOUR_BIT & PALETTE_MASK) == 0 ); static const PaletteID PAL_NONE = 0; diff --git a/src/widget.cpp b/src/widget.cpp index 36c58748bf..0a47659611 100644 --- a/src/widget.cpp +++ b/src/widget.cpp @@ -568,7 +568,7 @@ static inline void DrawCloseBox(const Rect &r, Colours colour) if (colour != COLOUR_WHITE) DrawFrameRect(r.left, r.top, r.right, r.bottom, colour, FR_NONE); Dimension d = GetSpriteSize(SPR_CLOSEBOX); int s = UnScaleGUI(1); /* Offset to account for shadow of SPR_CLOSEBOX */ - DrawSprite(SPR_CLOSEBOX, (colour != COLOUR_WHITE ? TC_BLACK : TC_SILVER) | (1 << PALETTE_TEXT_RECOLOUR), CenterBounds(r.left, r.right, d.width - s), CenterBounds(r.top, r.bottom, d.height - s)); + DrawSprite(SPR_CLOSEBOX, (colour != COLOUR_WHITE ? TC_BLACK : TC_SILVER) | (1U << PALETTE_TEXT_RECOLOUR), CenterBounds(r.left, r.right, d.width - s), CenterBounds(r.top, r.bottom, d.height - s)); } /** -- 2.11.4.GIT