* [PATCH v2] tracing: Use address-of operator on section symbols
@ 2020-02-20 5:10 Nathan Chancellor
2020-03-19 2:00 ` Nathan Chancellor
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2020-02-20 5:10 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar
Cc: linux-kernel, clang-built-linux, Nathan Chancellor, Nick Desaulniers
Clang warns:
../kernel/trace/trace.c:9335:33: warning: array comparison always
evaluates to true [-Wtautological-compare]
if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt)
^
1 warning generated.
These are not true arrays, they are linker defined symbols, which are
just addresses. Using the address of operator silences the warning and
does not change the runtime result of the check (tested with some print
statements compiled in with clang + ld.lld and gcc + ld.bfd in QEMU).
Link: https://github.com/ClangBuiltLinux/linux/issues/893
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2: https://lore.kernel.org/lkml/20200219045423.54190-4-natechancellor@gmail.com/
* No longer a series because there is no prerequisite patch.
* Use address-of operator instead of casting to unsigned long.
NOTE: The code generation does seem to change, unlike every other call
site that I did this change to but the result of the check remains the
same as noted in the commit message and I cannot really understand what
has changed in the assembly. Please let me know if there is something
catastrophically wrong.
kernel/trace/trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c797a15a1fc7..78727dd9a6f5 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9332,7 +9332,7 @@ __init static int tracer_alloc_buffers(void)
goto out_free_buffer_mask;
/* Only allocate trace_printk buffers if a trace_printk exists */
- if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt)
+ if (&__stop___trace_bprintk_fmt != &__start___trace_bprintk_fmt)
/* Must be called before global_trace.buffer is allocated */
trace_printk_init_buffers();
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-02-20 5:10 [PATCH v2] tracing: Use address-of operator on section symbols Nathan Chancellor
@ 2020-03-19 2:00 ` Nathan Chancellor
2020-03-19 14:33 ` Steven Rostedt
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2020-03-19 2:00 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar
Cc: linux-kernel, clang-built-linux, Nick Desaulniers
On Wed, Feb 19, 2020 at 10:10:12PM -0700, Nathan Chancellor wrote:
> Clang warns:
>
> ../kernel/trace/trace.c:9335:33: warning: array comparison always
> evaluates to true [-Wtautological-compare]
> if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt)
> ^
> 1 warning generated.
>
> These are not true arrays, they are linker defined symbols, which are
> just addresses. Using the address of operator silences the warning and
> does not change the runtime result of the check (tested with some print
> statements compiled in with clang + ld.lld and gcc + ld.bfd in QEMU).
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/893
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2: https://lore.kernel.org/lkml/20200219045423.54190-4-natechancellor@gmail.com/
>
> * No longer a series because there is no prerequisite patch.
> * Use address-of operator instead of casting to unsigned long.
>
> NOTE: The code generation does seem to change, unlike every other call
> site that I did this change to but the result of the check remains the
> same as noted in the commit message and I cannot really understand what
> has changed in the assembly. Please let me know if there is something
> catastrophically wrong.
>
> kernel/trace/trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c797a15a1fc7..78727dd9a6f5 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -9332,7 +9332,7 @@ __init static int tracer_alloc_buffers(void)
> goto out_free_buffer_mask;
>
> /* Only allocate trace_printk buffers if a trace_printk exists */
> - if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt)
> + if (&__stop___trace_bprintk_fmt != &__start___trace_bprintk_fmt)
> /* Must be called before global_trace.buffer is allocated */
> trace_printk_init_buffers();
>
> --
> 2.25.1
>
Gentle ping for review/acceptance.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-03-19 2:00 ` Nathan Chancellor
@ 2020-03-19 14:33 ` Steven Rostedt
2020-03-19 15:27 ` Nick Desaulniers
2020-03-26 19:46 ` Nathan Chancellor
0 siblings, 2 replies; 8+ messages in thread
From: Steven Rostedt @ 2020-03-19 14:33 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Ingo Molnar, linux-kernel, clang-built-linux, Nick Desaulniers
On Wed, 18 Mar 2020 19:00:04 -0700
Nathan Chancellor <natechancellor@gmail.com> wrote:
> Gentle ping for review/acceptance.
Hmm, my local patchwork had this patch rejected. I'll go and apply it, run
some tests and see if it works. Perhaps I meant to reject v1 and
accidentally rejected v2 :-/
Thanks for the ping!
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-03-19 14:33 ` Steven Rostedt
@ 2020-03-19 15:27 ` Nick Desaulniers
2020-03-19 16:01 ` Steven Rostedt
2020-03-26 19:46 ` Nathan Chancellor
1 sibling, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2020-03-19 15:27 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Nathan Chancellor, Ingo Molnar, LKML, clang-built-linux
On Thu, Mar 19, 2020 at 7:33 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 18 Mar 2020 19:00:04 -0700
> Nathan Chancellor <natechancellor@gmail.com> wrote:
>
> > Gentle ping for review/acceptance.
>
> Hmm, my local patchwork had this patch rejected. I'll go and apply it, run
Local patchwork? As in patchwork.kernel.org? (Is there a client, or
can you host your own instance?)
> some tests and see if it works. Perhaps I meant to reject v1 and
> accidentally rejected v2 :-/
>
> Thanks for the ping!
>
> -- Steve
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-03-19 15:27 ` Nick Desaulniers
@ 2020-03-19 16:01 ` Steven Rostedt
0 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2020-03-19 16:01 UTC (permalink / raw)
To: Nick Desaulniers; +Cc: Nathan Chancellor, Ingo Molnar, LKML, clang-built-linux
On Thu, 19 Mar 2020 08:27:11 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Thu, Mar 19, 2020 at 7:33 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Wed, 18 Mar 2020 19:00:04 -0700
> > Nathan Chancellor <natechancellor@gmail.com> wrote:
> >
> > > Gentle ping for review/acceptance.
> >
> > Hmm, my local patchwork had this patch rejected. I'll go and apply it, run
>
> Local patchwork? As in patchwork.kernel.org? (Is there a client, or
> can you host your own instance?)
>
No, I went into a lot of effort to get it up and running on my own inbox.
I did this because I wasn't able to keep up with all the patches being sent
directly to me. It's a bit fragile, and the workflow mailing list is
working on ways to have this be a more general solution for maintainers.
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-03-19 14:33 ` Steven Rostedt
2020-03-19 15:27 ` Nick Desaulniers
@ 2020-03-26 19:46 ` Nathan Chancellor
2020-03-26 21:31 ` Steven Rostedt
1 sibling, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2020-03-26 19:46 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, linux-kernel, clang-built-linux, Nick Desaulniers
On Thu, Mar 19, 2020 at 10:33:12AM -0400, Steven Rostedt wrote:
> On Wed, 18 Mar 2020 19:00:04 -0700
> Nathan Chancellor <natechancellor@gmail.com> wrote:
>
> > Gentle ping for review/acceptance.
>
> Hmm, my local patchwork had this patch rejected. I'll go and apply it, run
> some tests and see if it works. Perhaps I meant to reject v1 and
> accidentally rejected v2 :-/
>
> Thanks for the ping!
>
> -- Steve
Hi Steve,
Did you ever get around to running any tests? If so, were there any
issues? This warning is one of two remaining across several different
configurations so I sent the patch to turn on the warning and I want
to make sure this gets picked up at some point:
https://lore.kernel.org/lkml/20200326194155.29107-1-natechancellor@gmail.com/
If you have not had time, totally fine, I just want to make sure it does
not fall through the cracks again :)
Thank you,
Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-03-26 19:46 ` Nathan Chancellor
@ 2020-03-26 21:31 ` Steven Rostedt
2020-03-26 21:34 ` Nathan Chancellor
0 siblings, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2020-03-26 21:31 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Ingo Molnar, linux-kernel, clang-built-linux, Nick Desaulniers
On Thu, 26 Mar 2020 12:46:52 -0700
Nathan Chancellor <natechancellor@gmail.com> wrote:
> On Thu, Mar 19, 2020 at 10:33:12AM -0400, Steven Rostedt wrote:
> > On Wed, 18 Mar 2020 19:00:04 -0700
> > Nathan Chancellor <natechancellor@gmail.com> wrote:
> >
> > > Gentle ping for review/acceptance.
> >
> > Hmm, my local patchwork had this patch rejected. I'll go and apply it, run
> > some tests and see if it works. Perhaps I meant to reject v1 and
> > accidentally rejected v2 :-/
> >
> > Thanks for the ping!
> >
> > -- Steve
>
> Hi Steve,
>
> Did you ever get around to running any tests? If so, were there any
> issues? This warning is one of two remaining across several different
> configurations so I sent the patch to turn on the warning and I want
> to make sure this gets picked up at some point:
>
> https://lore.kernel.org/lkml/20200326194155.29107-1-natechancellor@gmail.com/
>
> If you have not had time, totally fine, I just want to make sure it does
> not fall through the cracks again :)
>
I have applied it to my queue. But the code I have in with it failed my
tests, and I'm just about to kick off another round (I believe I fixed
everything). And hopefully if it all passes, I can get it out to my
linux-next branch by tomorrow.
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tracing: Use address-of operator on section symbols
2020-03-26 21:31 ` Steven Rostedt
@ 2020-03-26 21:34 ` Nathan Chancellor
0 siblings, 0 replies; 8+ messages in thread
From: Nathan Chancellor @ 2020-03-26 21:34 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, linux-kernel, clang-built-linux, Nick Desaulniers
On Thu, Mar 26, 2020 at 05:31:52PM -0400, Steven Rostedt wrote:
> On Thu, 26 Mar 2020 12:46:52 -0700
> Nathan Chancellor <natechancellor@gmail.com> wrote:
>
> > On Thu, Mar 19, 2020 at 10:33:12AM -0400, Steven Rostedt wrote:
> > > On Wed, 18 Mar 2020 19:00:04 -0700
> > > Nathan Chancellor <natechancellor@gmail.com> wrote:
> > >
> > > > Gentle ping for review/acceptance.
> > >
> > > Hmm, my local patchwork had this patch rejected. I'll go and apply it, run
> > > some tests and see if it works. Perhaps I meant to reject v1 and
> > > accidentally rejected v2 :-/
> > >
> > > Thanks for the ping!
> > >
> > > -- Steve
> >
> > Hi Steve,
> >
> > Did you ever get around to running any tests? If so, were there any
> > issues? This warning is one of two remaining across several different
> > configurations so I sent the patch to turn on the warning and I want
> > to make sure this gets picked up at some point:
> >
> > https://lore.kernel.org/lkml/20200326194155.29107-1-natechancellor@gmail.com/
> >
> > If you have not had time, totally fine, I just want to make sure it does
> > not fall through the cracks again :)
> >
>
> I have applied it to my queue. But the code I have in with it failed my
> tests, and I'm just about to kick off another round (I believe I fixed
> everything). And hopefully if it all passes, I can get it out to my
> linux-next branch by tomorrow.
>
> -- Steve
Awesome, thank you for the quick reply and keeping me in the loop!
Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-26 21:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 5:10 [PATCH v2] tracing: Use address-of operator on section symbols Nathan Chancellor
2020-03-19 2:00 ` Nathan Chancellor
2020-03-19 14:33 ` Steven Rostedt
2020-03-19 15:27 ` Nick Desaulniers
2020-03-19 16:01 ` Steven Rostedt
2020-03-26 19:46 ` Nathan Chancellor
2020-03-26 21:31 ` Steven Rostedt
2020-03-26 21:34 ` Nathan Chancellor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).