From fb37bbf675e598af3a88b6b74acda5fc82c93513 Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Thu, 10 Oct 2019 11:46:09 +0100
Subject: [PATCH] Disable some signed shift UBSAN warnings

On Clang we can do this globally via a sanitizer blacklist, but that's not supported on GCC (yet):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61978

We disable these warnings because all compilers implement them in the
same way according to the N2218 proposal to standardize the behaviour:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2218.htm
---
 Source/player.cpp | 12 ++++++++++++
 Source/render.cpp |  3 +++
 Source/sha.cpp    | 22 ++++++----------------
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/Source/player.cpp b/Source/player.cpp
index 6c2c35b17..c58346982 100644
--- a/Source/player.cpp
+++ b/Source/player.cpp
@@ -1293,6 +1293,9 @@ void StartWalk(int pnum, int xvel, int yvel, int xadd, int yadd, int EndDir, int
 	}
 }
 
+#if defined(__clang__) || defined(__GNUC__)
+__attribute__((no_sanitize("shift-base")))
+#endif
 void StartWalk2(int pnum, int xvel, int yvel, int xoff, int yoff, int xadd, int yadd, int EndDir, int sdir)
 {
 	int px, py;
@@ -1372,6 +1375,9 @@ void StartWalk2(int pnum, int xvel, int yvel, int xoff, int yoff, int xadd, int
 	}
 }
 
+#if defined(__clang__) || defined(__GNUC__)
+__attribute__((no_sanitize("shift-base")))
+#endif
 void StartWalk3(int pnum, int xvel, int yvel, int xoff, int yoff, int xadd, int yadd, int mapx, int mapy, int EndDir, int sdir)
 {
 	int px, py, x, y;
@@ -1678,6 +1684,9 @@ void RespawnDeadItem(ItemStruct *itm, int x, int y)
 	itm->_itype = ITYPE_NONE;
 }
 
+#if defined(__clang__) || defined(__GNUC__)
+__attribute__((no_sanitize("shift-base")))
+#endif
 void StartPlayerKill(int pnum, int earflag)
 {
 	BOOL diablolevel;
@@ -2019,6 +2028,9 @@ void InitLevelChange(int pnum)
 	}
 }
 
+#if defined(__clang__) || defined(__GNUC__)
+__attribute__((no_sanitize("shift-base")))
+#endif
 void StartNewLvl(int pnum, int fom, int lvl)
 {
 	InitLevelChange(pnum);
diff --git a/Source/render.cpp b/Source/render.cpp
index 9f24dde8f..c11acc051 100644
--- a/Source/render.cpp
+++ b/Source/render.cpp
@@ -163,6 +163,9 @@ inline static void RenderLine(BYTE **dst, BYTE **src, int n, BYTE *tbl, DWORD ma
 	}
 }
 
+#if defined(__clang__) || defined(__GNUC__)
+__attribute__((no_sanitize("shift-base")))
+#endif
 void RenderTile(BYTE *pBuff)
 {
 	int i, j;
diff --git a/Source/sha.cpp b/Source/sha.cpp
index 6cb9bacf4..1538ce3d7 100644
--- a/Source/sha.cpp
+++ b/Source/sha.cpp
@@ -4,29 +4,19 @@
 
 DEVILUTION_BEGIN_NAMESPACE
 
-// Diablo's "SHA1" is different from actual SHA1 in that it uses arithmetic
+// NOTE: Diablo's "SHA1" is different from actual SHA1 in that it uses arithmetic
 // right shifts (sign bit extension).
-//
-// Note that:
-// 1. Tere is a C++20 proposal to make right shift always defined
-//    as an arithmetic shift with sign extension:
-//    http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0907r1.html
-// 2. Modern C++ compilers (GCC 8+, Clang 7+) compile the portable code
-//    to the same assembly as the implementation defined `value >> bits`:
-//    https://stackoverflow.com/a/53766752
 
 namespace {
 
-std::uint32_t asr(std::int32_t value, std::int32_t amount)
-{
-	return value < 0 ? ~(~value >> amount) : value >> amount;
-}
-
 /*
  * Diablo-"SHA1" circular left shift.
  */
-std::uint32_t SHA1CircularShift(std::uint32_t bits, std::uint32_t word) {
-	return (word << bits) | asr(word, 32 - bits);
+#if defined(__clang__) || defined(__GNUC__)
+__attribute__((no_sanitize("shift-base")))
+#endif
+std::uint32_t SHA1CircularShift(std::uint32_t bits, std::int32_t word) {
+	return (word << bits) | (word >> (32 - bits));
 }
 
 } // namespace