[Bug libc/32932] riscv: __riscv_hwprobe function attributes are incorrect

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=32932

--- Comment #2 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Adhemerval Zanella
<azanella@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=8af8beb1c488dcfec754431c1626979276046545

commit 8af8beb1c488dcfec754431c1626979276046545
Author: Mark Harris <mark.hsj@gmail.com>
Date:   Sat May 24 15:02:38 2025 -0700

    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>

-- 
You are receiving this mail because:
You are on the CC list for the bug.