From ee3de1e1f3da2386197a2c586304deb263890a35 Mon Sep 17 00:00:00 2001 From: Brian Ho Date: Fri, 16 Aug 2019 18:44:14 +0000 Subject: [PATCH] Remove undefined behavior from NextPowerOfTwo This CL removes undefined behavior from NextPowerOfTwo(0). Currently on Linux, calling NextPowerOfTwo(0) simplifies down to: - 1ull << (64 - __builtin_clzll(0 - 1)); - 1ull << (64 - __builtin_clzll(INT_MAX)); - 1ull << (64 - 0); - 1ull << 64 Since 64 is the same width as the long long in our left operand, this left shift results in undefined behavior (C++11 standard 5.8.1). For a default Chrome compile, this does not cause any issues; 1ull << 64 results in 0. In ChromeOS, however, we compile with ThinLTO which, among other things, inlines various functions in the interest of performance. When NextPowerOfTwo is inlined, the undefined behavior of our invalid left shift borks the stack which causes the Math.NextPowerOfTwo unit test to fail. BUG= chromium:993457 TEST= verified that Math.NextPowerOfTwo now passes with LTO Change-Id: I2702ba0b780203643da1d98ad0380098c7b3eab0 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10180 Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya Commit-Queue: Brian Ho --- src/common/Math.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/Math.cpp b/src/common/Math.cpp index aab0e451ce..3258fc3466 100644 --- a/src/common/Math.cpp +++ b/src/common/Math.cpp @@ -63,7 +63,7 @@ uint64_t NextPowerOfTwo(uint64_t n) { #if defined(DAWN_COMPILER_MSVC) return n <= 1 ? 1 : 1ull << (64 - __lzcnt64(n - 1)); #else - return n == 1 ? 1 : 1ull << (64 - __builtin_clzll(n - 1)); + return n <= 1 ? 1 : 1ull << (64 - __builtin_clzll(n - 1)); #endif }