Received-SPF: pass (google.com: domain of linux-kernel+bounces-108267-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240319-tracing-fully-silence-wstring-compare-v1-0-81adb44403f5@kernel.org>
 <20240319-tracing-fully-silence-wstring-compare-v1-2-81adb44403f5@kernel.org>
In-Reply-To: <20240319-tracing-fully-silence-wstring-compare-v1-2-81adb44403f5@kernel.org>
From: Justin Stitt <justinstitt@google.com>
Date: Tue, 19 Mar 2024 17:30:41 -0700
Message-ID: <CAFhGd8q_4QfjPwGhtfOA40ZGNF7_wf3V43rAvcb_qXQnz1TFOw@mail.gmail.com>
Subject: Re: [PATCH 2/2] tracing: Ignore -Wstring-compare with diagnostic macros
To: Nathan Chancellor <nathan@kernel.org>
Cc: rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, 
	ndesaulniers@google.com, morbo@google.com, linux-kernel@vger.kernel.org, 
	linux-trace-kernel@vger.kernel.org, llvm@lists.linux.dev, 
	patches@lists.linux.dev, Linux Kernel Functional Testing <lkft@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 19, 2024 at 9:08=E2=80=AFAM Nathan Chancellor <nathan@kernel.or=
g> wrote:
>
> Commit b1afefa62ca9 ("tracing: Use strcmp() in __assign_str() WARN_ON()
> check") addressed a clang warning, -Wstring-compare, with the use of
> __builtin_constant_p() to dispatch to strcmp() if the source string is a
> string literal and a direct comparison if not. Unfortunately, even with
> this change, the warning is still present because __builtin_constant_p()
> is not evaluated at this stage of the pipeline, so clang still thinks
> the else branch could occur for this situation:
>
>   include/trace/events/sunrpc.h:705:4: error: result of comparison agains=
t a string literal is unspecified (use an explicit string comparison functi=
on instead) [-Werror,-Wstring-compare]
>   ...
>   include/trace/stages/stage6_event_callback.h:40:15: note: expanded from=
 macro '__assign_str'
>      40 |                              (src) !=3D __data_offsets.dst##_pt=
r_);       \
>         |                                    ^
>   ...
>
> Use the compiler diagnostic macros to disable this warning around the
> WARN_ON_ONCE() expression since a string comparison function, strcmp(),
> will always be used for the comparison of string literals.
>
> Fixes: b1afefa62ca9 ("tracing: Use strcmp() in __assign_str() WARN_ON() c=
heck")
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Closes: https://lore.kernel.org/all/CA+G9fYs=3DOTKAZS6g1P1Ewadfr0qoe6LgOV=
SohqkXmFXotEODdg@mail.gmail.com/
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  include/trace/stages/stage6_event_callback.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace=
/stages/stage6_event_callback.h
> index 83da83a0c14f..56a4eea5a48e 100644
> --- a/include/trace/stages/stage6_event_callback.h
> +++ b/include/trace/stages/stage6_event_callback.h
> @@ -35,9 +35,14 @@
>         do {                                                            \
>                 char *__str__ =3D __get_str(dst);                        =
 \
>                 int __len__ =3D __get_dynamic_array_len(dst) - 1;        =
 \
> +               __diag_push();                                          \
> +               __diag_ignore(clang, 11, "-Wstring-compare",            \
> +                             "__builtin_constant_p() ensures strcmp()" \
> +                             "will be used for string literals");      \
>                 WARN_ON_ONCE(__builtin_constant_p(src) ?                \
>                              strcmp((src), __data_offsets.dst##_ptr_) : \
>                              (src) !=3D __data_offsets.dst##_ptr_);      =
 \

What exactly is the point of the literal string comparison? Why
doesn't strcmp do the trick?

> +               __diag_pop();                                           \
>                 memcpy(__str__, __data_offsets.dst##_ptr_ ? :           \
>                        EVENT_NULL_STR, __len__);                        \
>                 __str__[__len__] =3D '\0';                               =
 \
>
> --
> 2.44.0
>