linux-kernel - Re: [PATCH RFC] selftests/rseq: fix kselftest Clang build warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKwvOdmgh0XJi_at98KNwfPS4=Hq0+vkjT3SzTARZtEMyOx04w@mail.gmail.com>
Date:   Fri, 8 Sep 2023 16:11:03 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Justin Stitt <justinstitt@...gle.com>
Cc:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Shuah Khan <shuah@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Tom Rix <trix@...hat.com>, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH RFC] selftests/rseq: fix kselftest Clang build warnings

On Fri, Sep 8, 2023 at 4:03 PM Justin Stitt <justinstitt@...gle.com> wrote:
>
> Hi,
>
> I am experiencing many warnings when trying to build tools/testing/selftests.
>
> Here's one such example from rseq tree:
> |  param_test.c:1234:10: error: address argument to atomic operation must be a pointer to _Atomic type ('intptr_t *' (aka 'long *') invalid)
> |   1234 |         while (!atomic_load(&args->percpu_list_ptr)) {}
> |        |                 ^           ~~~~~~~~~~~~~~~~~~~~~~
> |  /usr/local/google/home/justinstitt/repos/tc-build/build/llvm/final/lib/clang/18/include/stdatomic.h:140:29: note: expanded from macro 'atomic_load'
> |    140 | #define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST)
> |        |                             ^                 ~~~~~~

ah, so __c11_atomic_load is a compiler built in (similar to cstdint.h)
(see the path of the note diagnostic).

Your changes below look correct to me (except for the last hunk.
percpu_list_ptr is already an intptr_t, the cast seems redundant);
does GCC error with your below changes?

>
> I added the _Atomic type in various locations to silence _all_ (10) of these
> warnings. I'm wondering, though, perhaps the absence of these _Atomic
> types in the first place is on purpose? Am I on the right track to fix
> these warnings without damaging the legitimacy of the tests at hand?
>
> I'd like some feedback about where to go from here and if others are
> experiencing the same issues. Thanks!
>
> FWIW here's my specific build incantation on Clang-18 (49d41de57896e935cd5726719c5208bce22694ae):
> $ make LLVM=1 -j128 ARCH=x86_64 mrproper headers defconfig kselftest-merge
> $ make LLVM=1 ARCH=x86_64 -C tools/testing/selftests
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
> Signed-off-by: Justin Stitt <justinstitt@...gle.com>
> ---
>  tools/testing/selftests/rseq/param_test.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/rseq/param_test.c b/tools/testing/selftests/rseq/param_test.c
> index bf951a490bb4..94802aeed2c6 100644
> --- a/tools/testing/selftests/rseq/param_test.c
> +++ b/tools/testing/selftests/rseq/param_test.c
> @@ -356,7 +356,7 @@ struct inc_thread_test_data {
>  };
>
>  struct percpu_list_node {
> -       intptr_t data;
> +       _Atomic intptr_t data;
>         struct percpu_list_node *next;
>  };
>
> @@ -1212,8 +1212,8 @@ static int set_signal_handler(void)
>  /* Test MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU membarrier command. */
>  #ifdef TEST_MEMBARRIER
>  struct test_membarrier_thread_args {
> -       int stop;
> -       intptr_t percpu_list_ptr;
> +       _Atomic int stop;
> +       _Atomic intptr_t percpu_list_ptr;
>  };
>
>  /* Worker threads modify data in their "active" percpu lists. */
> @@ -1240,7 +1240,7 @@ void *test_membarrier_worker_thread(void *arg)
>                         int cpu = get_current_cpu_id();
>
>                         ret = rseq_offset_deref_addv(RSEQ_MO_RELAXED, RSEQ_PERCPU,
> -                               &args->percpu_list_ptr,
> +                               (intptr_t*)&args->percpu_list_ptr,
>                                 sizeof(struct percpu_list_entry) * cpu, 1, cpu);
>                 } while (rseq_unlikely(ret));
>         }
>
> ---
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> change-id: 20230908-kselftest-param_test-c-1763b62e762f
>
> Best regards,
> --
> Justin Stitt <justinstitt@...gle.com>
>


-- 
Thanks,
~Nick Desaulniers