Make powerof2 macro ubsan safe
Subtracting one from the smallest value expressable by the provided
variable could cause an underflow operation. In particular, this is
problematic when code similar to:
uint64_t foo = 0;
if (powerof2(foo)) {
...;
}
is run with integer sanitization enabled. The macro would subtract one
from zero, underflowing and triggering the sanitizer.
Make the powerof2() macro ubsan safe, by explicitly handling underflows.
Note: This change DOES NOT make powerof2() accurate. We continue to
falsely return "true" for 0 and negative numbers (see attached tests).
Found while investigating Bug: 122975762
Test: see added testcase
Test: atest ziparchive-tests
Change-Id: I5408ce5c18868d797bcae8f115ddb7c4c1ced81e
diff --git a/libc/include/sys/param.h b/libc/include/sys/param.h
index 5cde4b7..16fed86 100644
--- a/libc/include/sys/param.h
+++ b/libc/include/sys/param.h
@@ -51,8 +51,17 @@
#endif
#define roundup(x, y) ((((x)+((y)-1))/(y))*(y))
-/** Returns true if the argument is a power of two. */
-#define powerof2(x) ((((x)-1)&(x))==0)
+/*
+ * Returns true if the binary representation of the argument is all zeros
+ * or has exactly one bit set. Contrary to the macro name, this macro
+ * DOES NOT determine if the provided value is a power of 2. In particular,
+ * this function falsely returns true for powerof2(0) and some negative
+ * numbers.
+ */
+#define powerof2(x) \
+ ({ __typeof__(x) _x = (x); \
+ __typeof__(x) _x2; \
+ __builtin_add_overflow(_x, -1, &_x2) ? 1 : ((_x2&_x) == 0 ); })
/** Returns the lesser of its two arguments. */
#define MIN(a,b) (((a)<(b))?(a):(b))
diff --git a/tests/Android.bp b/tests/Android.bp
index ab11d47..408312a 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -153,6 +153,7 @@
"sys_epoll_test.cpp",
"sys_mman_test.cpp",
"sys_msg_test.cpp",
+ "sys_param_test.cpp",
"sys_personality_test.cpp",
"sys_prctl_test.cpp",
"sys_procfs_test.cpp",
diff --git a/tests/sys_param_test.cpp b/tests/sys_param_test.cpp
new file mode 100644
index 0000000..e4bbf42
--- /dev/null
+++ b/tests/sys_param_test.cpp
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+#include <sys/param.h>
+
+TEST(sys_param_test, powerof2_positives) {
+ ASSERT_TRUE(powerof2(1));
+ ASSERT_TRUE(powerof2(2));
+ ASSERT_TRUE(powerof2(4));
+ ASSERT_TRUE(powerof2(8));
+ ASSERT_FALSE(powerof2(3));
+ ASSERT_FALSE(powerof2(5));
+ ASSERT_FALSE(powerof2(7));
+ ASSERT_FALSE(powerof2(9));
+ ASSERT_FALSE(powerof2(10));
+}
+
+TEST(sys_param_test, powerof2_zero) {
+ // 0 isn't a power of 2, but for compatibility, we assume it is.
+ ASSERT_TRUE(powerof2(0));
+ uint32_t zero = 0;
+ ASSERT_TRUE(powerof2(zero));
+}
+
+TEST(sys_param_test, powerof2_negatives) {
+ // negative numbers can never be a power of 2, but for compatibility,
+ // we assume they can be.
+ int32_t min32 = INT32_MIN;
+ int64_t min64 = INT64_MIN;
+ ASSERT_TRUE(powerof2(min32));
+ ASSERT_FALSE(powerof2(min32 + 1));
+ ASSERT_TRUE(powerof2(min64));
+ ASSERT_FALSE(powerof2(min64 + 1));
+}