[PATCH v2] tracing: Use address-of operator on section symbols

[PATCH v2] tracing: Use address-of operator on section symbols

[Nathan Chancellor]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Nathan Chancellor]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Steven Rostedt]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Nick Desaulniers]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Steven Rostedt]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Nathan Chancellor]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Steven Rostedt]

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

Re: [PATCH v2] tracing: Use address-of operator on section symbols

[Nathan Chancellor]

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