libc: silence unsigned->signed warnings with ioctl
This also tweaks cdefs to make __overloadable usable outside of
FORTIFY. It had to be FORTIFY-only before we had unmarked overload
support in clang+Bionic.
Bug: https://github.com/android-ndk/ndk/issues/402
Test: Internal master builds + `mma`. `mma` in Bionic fails if the
change to ioctl is undone.
Change-Id: Ib386b1786e1dca625e6d5a18682005adc734d9c1
diff --git a/libc/include/bits/ioctl.h b/libc/include/bits/ioctl.h
index 0cf87d2..3357c1b 100644
--- a/libc/include/bits/ioctl.h
+++ b/libc/include/bits/ioctl.h
@@ -35,6 +35,28 @@
int ioctl(int __fd, int __request, ...);
+/*
+ * Work around unsigned -> signed conversion warnings: many common ioctl
+ * constants are unsigned.
+ *
+ * Since this workaround introduces an overload to ioctl, it's possible that it
+ * will break existing code that takes the address of ioctl. If such a breakage
+ * occurs, you can work around it by either:
+ * - specifying a concrete, correct type for ioctl (whether it be through a cast
+ * in `(int (*)(int, int, ...))ioctl`, creating a temporary variable with the
+ * type of the ioctl you prefer, ...), or
+ * - defining BIONIC_IOCTL_NO_SIGNEDNESS_OVERLOAD, which will make the
+ * overloading go away.
+ *
+ * FIXME: __has_extension is more or less a clang version check. Remove it when
+ * we don't need to support old clang code.
+ */
+#if defined(__clang__) && __has_extension(overloadable_unmarked) && \
+ !defined(BIONIC_IOCTL_NO_SIGNEDNESS_OVERLOAD)
+/* enable_if(1) just exists to break overloading ties. */
+int ioctl(int __fd, unsigned __request, ...) __overloadable __enable_if(1, "") __RENAME(ioctl);
+#endif
+
__END_DECLS
#endif
diff --git a/libc/include/sys/cdefs.h b/libc/include/sys/cdefs.h
index c270bb5..5760689 100644
--- a/libc/include/sys/cdefs.h
+++ b/libc/include/sys/cdefs.h
@@ -343,21 +343,7 @@
# define __BIONIC_INCLUDE_FORTIFY_HEADERS 1
#endif
-/*
- * Used to support clangisms with FORTIFY. Because these change how symbols are
- * emitted, we need to ensure that bionic itself is built fortified. But lots
- * of external code (especially stuff using configure) likes to declare
- * functions directly, and they can't know that the overloadable attribute
- * exists. This leads to errors like:
- *
- * dcigettext.c:151:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
- * char *getcwd ();
- * ^
- *
- * To avoid this and keep such software building, don't use overloadable if
- * we're not using fortify.
- */
-#if defined(__clang__) && defined(__BIONIC_FORTIFY)
+#if defined(__clang__)
# define __overloadable __attribute__((overloadable))
#else
# define __overloadable
diff --git a/tests/Android.bp b/tests/Android.bp
index ec90296..eb3b5f4 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -48,6 +48,18 @@
// All standard tests.
// -----------------------------------------------------------------------------
+// Test diagnostics emitted by clang. The library that results is useless; we
+// just want to run '-Xclang -verify', which will fail if the diagnostics don't
+// match up with what the source file says they should be.
+cc_test_library {
+ name: "clang_diagnostic_tests",
+ cflags: [
+ "-Xclang",
+ "-verify",
+ ],
+ srcs: ["sys_ioctl_diag_test.cpp"],
+}
+
cc_test_library {
name: "libBionicStandardTests",
defaults: ["bionic_tests_defaults"],
diff --git a/tests/sys_ioctl_diag_test.cpp b/tests/sys_ioctl_diag_test.cpp
new file mode 100644
index 0000000..e271e6c
--- /dev/null
+++ b/tests/sys_ioctl_diag_test.cpp
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2018 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.
+ */
+
+// This file makes uses of clang's built-in diagnostic checker.
+// While not officially supported by clang, it's used by clang for all of its
+// own diagnostic tests. Please see
+// https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details
+// for details.
+
+// expected-no-diagnostics
+
+#include <sys/ioctl.h>
+
+#pragma clang diagnostic warning "-Wsign-conversion"
+
+void check_no_signedness_warnings(int i, unsigned x) {
+ ioctl(i, i);
+ ioctl(i, x);
+
+ ioctl(i, i, nullptr);
+ ioctl(i, x, nullptr);
+}