riscv: Correct __riscv_hwprobe function prototype [BZ #32932]

The third argument to __riscv_hwprobe is the size in bytes of the
cpu bitmask pointed to by the fourth argument, however in the access
attribute (read_only, 4, 3) it is used as an element count (i.e., the
number of unsigned longs that make up the bitmask), resulting in a
false compiler warning:

$ gcc -c hwprobe1.c
hwprobe1.c: In function 'main':
hwprobe1.c:15:11: warning: '__riscv_hwprobe' reading 1024 bytes from a region of size 128 [-Wstringop-overread]
   15 |     ret = __riscv_hwprobe (pairs, 1, sizeof(cpus), cpus, 0);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hwprobe1.c:9:23: note: source object 'cpus' of size 128
    9 |     unsigned long int cpus[16];
      |                       ^~~~
In file included from hwprobe1.c:1:
/usr/include/riscv64-linux-gnu/sys/hwprobe.h:66:12: note: in a call to function '__riscv_hwprobe' declared with attribute 'access (read_only, 4, 3)'
   66 | extern int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count,
      |            ^~~~~~~~~~~~~~~
$

The documentation (https://docs.kernel.org/arch/riscv/hwprobe.html)
claims that the cpu bitmask has the type cpu_set_t *, which would be
consistent with other functions that take a cpu bitmask such as
sched_setaffinity and sched_getaffinity.  It also uses the name
cpusetsize for the third argument, which is much more accurate than
cpu_count since it is a size in bytes and not a cpu count.  The
(read_only, 4, 3) access attribute in the glibc prototype claims
that the cpu bitmask is only read, however when flags is
RISCV_HWPROBE_WHICH_CPUS it is both read and written.

Therefore, in the glibc prototype the type of the fourth argument is
changed to cpu_set_t * to match the documentation, the name of the
third argument is changed to cpusetsize as in the documentation, and the
incorrect access attribute that applies to these arguments is removed.
Almost all existing callers pass a null pointer for the fourth
argument, however a transparent union is introduced for compatibility
with callers that cast a pointer to the old argument type, and a
macro is introduced allowing callers the ability to distinguish
between the old and new prototype when needed.

The access attributes are being specified with __fortified_attr_access,
however this macro is for fortified functions; the regular
__attr_access macro is for non-fortified functions such as this one.
Using the incorrect macro results in no access checks at fortify level
3, because it is assumed that the fortified function will be doing the
checking.  It is changed to use the correct macro so that the access
checks will work regardless of fortify level.

Also because __riscv_hwprobe is not a cancellation point, __THROW
is added, consistent with similar functions.  (However, it is omitted
from the typedef because GCC does not accept it there.)

The __wur (warn_unused_result) attribute is helpful for functions that
cannot be used safely without checking the result, however code such
as the following does not require the result to be checked and should
not produce a warning:
    struct riscv_hwprobe pair = { RISCV_HWPROBE_KEY_IMA_EXT_0, 0 };
    __riscv_hwprobe (&pair, 1, 0, NULL, 0);
    if (pair.value & RISCV_HWPROBE_EXT_ZBB) ...
Therefore this attribute is omitted.

The comment claiming that the second argument to the ifunc selector
is a pointer to the vDSO function is corrected.  It is a pointer to
the regular glibc function (which returns errors as positive values),
not the vDSO function (which returns errors as negative values).

Fixes commit 426d0e1aa8f17426d13707594111df712d2b8911 ("riscv: Add
Linux hwprobe syscall support").

Fixes: BZ #32932
Signed-off-by: Mark Harris <mark.hsj@gmail.com>
Signed-off-by: Mark Harris <mark.hsj@gmail.com>
Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
Acked-by: Palmer Dabbelt <palmer@dabbelt.com>

diff --git a/sysdeps/unix/sysv/linux/riscv/hwprobe.c b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
index e0cbd22cbe3785af8a937931f63f028b73984226..bc7f6f38e14d11699c3fef31e04e11bfd04c296c 100644 (file)
--- a/sysdeps/unix/sysv/linux/riscv/hwprobe.c
+++ b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
@@ -23,13 +23,13 @@
 #include <sysdep-vdso.h>
 
 int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
-                    size_t cpu_count, unsigned long int *cpus,
+                    size_t cpusetsize, __RISCV_HWPROBE_CPUS_TYPE cpus,
                     unsigned int flags)
 {
   int r;
 
   r = INTERNAL_VSYSCALL (riscv_hwprobe, 5, pairs, pair_count,
-                         cpu_count, cpus, flags);
+                         cpusetsize, cpus.__ul, flags);
 
   /* Negate negative errno values to match pthreads API. */
   return -r;

diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
index bebad6cf70dfe9b91738cad3c514468af69a659e..40415aae801bdd5a2b72ba4211a489a63d941fd2 100644 (file)
--- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
+++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
@@ -21,6 +21,7 @@
 #define _SYS_HWPROBE_H 1
 
 #include <features.h>
+#include <sched.h>
 #include <stddef.h>
 #include <errno.h>
 #ifdef __has_include
@@ -63,22 +64,39 @@ struct riscv_hwprobe {
 
 __BEGIN_DECLS
 
-extern int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count,
-                           size_t __cpu_count, unsigned long int *__cpus,
+#if defined __cplusplus || !__GNUC_PREREQ (2, 7)
+# define __RISCV_HWPROBE_CPUS_TYPE cpu_set_t *
+#else
+/* The fourth argument to __riscv_hwprobe should be a null pointer or a
+   pointer to a cpu_set_t (either the fixed-size type or allocated with
+   CPU_ALLOC).  However, early versions of this header file used the
+   argument type unsigned long int *.  The transparent union allows
+   the argument to be either cpu_set_t * or unsigned long int * for
+   compatibility.  The older header file requiring unsigned long int *
+   can be identified by the lack of the __RISCV_HWPROBE_CPUS_TYPE macro.
+   In C++ and with compilers that do not support transparent unions, the
+   argument type must be cpu_set_t *.  */
+typedef union {
+       cpu_set_t *__cs;
+       unsigned long int *__ul;
+} __RISCV_HWPROBE_CPUS_TYPE __attribute__ ((__transparent_union__));
+# define __RISCV_HWPROBE_CPUS_TYPE __RISCV_HWPROBE_CPUS_TYPE
+#endif
+
+extern int __riscv_hwprobe (struct riscv_hwprobe *__pairs,
+                           size_t __pair_count, size_t __cpusetsize,
+                           __RISCV_HWPROBE_CPUS_TYPE __cpus,
                            unsigned int __flags)
-     __nonnull ((1)) __wur
-     __fortified_attr_access (__read_write__, 1, 2)
-     __fortified_attr_access (__read_only__, 4, 3);
+     __THROW __nonnull ((1)) __attr_access ((__read_write__, 1, 2));
 
-/* A pointer to the __riscv_hwprobe vDSO function is passed as the second
+/* A pointer to the __riscv_hwprobe function is passed as the second
    argument to ifunc selector routines. Include a function pointer type for
    convenience in calling the function in those settings. */
-typedef int (*__riscv_hwprobe_t) (struct riscv_hwprobe *__pairs, size_t __pair_count,
-                                 size_t __cpu_count, unsigned long int *__cpus,
+typedef int (*__riscv_hwprobe_t) (struct riscv_hwprobe *__pairs,
+                                 size_t __pair_count, size_t __cpusetsize,
+                                 __RISCV_HWPROBE_CPUS_TYPE __cpus,
                                  unsigned int __flags)
-     __nonnull ((1)) __wur
-     __fortified_attr_access (__read_write__, 1, 2)
-     __fortified_attr_access (__read_only__, 4, 3);
+     __nonnull ((1)) __attr_access ((__read_write__, 1, 2));
 
 /* Helper function usable from ifunc selectors that probes a single key. */
 static __inline int