2020-02-20 05:14:45

by Nathan Chancellor

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

Clang warns:

../lib/dynamic_debug.c:1034:24: warning: array comparison always
evaluates to false [-Wtautological-compare]
if (__start___verbose == __stop___verbose) {
^
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 resulting assembly with either clang/ld.lld or
gcc/ld (tested with diff + objdump -Dr).

Link: https://github.com/ClangBuiltLinux/linux/issues/894
Suggested-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
v1 -> v2: https://lore.kernel.org/lkml/[email protected]/

* No longer a series because there is no prerequisite patch.
* Use address-of operator instead of casting to unsigned long.

lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index aae17d9522e5..8f199f403ab5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1031,7 +1031,7 @@ static int __init dynamic_debug_init(void)
int n = 0, entries = 0, modct = 0;
int verbose_bytes = 0;

- if (__start___verbose == __stop___verbose) {
+ if (&__start___verbose == &__stop___verbose) {
pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
return 1;
}
--
2.25.1

2020-03-19 02:01:14

by Nathan Chancellor

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

On Wed, Feb 19, 2020 at 10:13:20PM -0700, Nathan Chancellor wrote:
> Clang warns:
>
> ../lib/dynamic_debug.c:1034:24: warning: array comparison always
> evaluates to false [-Wtautological-compare]
> if (__start___verbose == __stop___verbose) {
> ^
> 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 resulting assembly with either clang/ld.lld or
> gcc/ld (tested with diff + objdump -Dr).
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/894
> Suggested-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> v1 -> v2: https://lore.kernel.org/lkml/[email protected]/
>
> * No longer a series because there is no prerequisite patch.
> * Use address-of operator instead of casting to unsigned long.
>
> lib/dynamic_debug.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index aae17d9522e5..8f199f403ab5 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -1031,7 +1031,7 @@ static int __init dynamic_debug_init(void)
> int n = 0, entries = 0, modct = 0;
> int verbose_bytes = 0;
>
> - if (__start___verbose == __stop___verbose) {
> + if (&__start___verbose == &__stop___verbose) {
> pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
> return 1;
> }
> --
> 2.25.1
>

Gentle ping for review/acceptance.

Cheers,
Nathan

2020-03-19 20:31:48

by Jason Baron

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

On 3/18/20 9:59 PM, Nathan Chancellor wrote:
> On Wed, Feb 19, 2020 at 10:13:20PM -0700, Nathan Chancellor wrote:
>> Clang warns:
>>
>> ../lib/dynamic_debug.c:1034:24: warning: array comparison always
>> evaluates to false [-Wtautological-compare]
>> if (__start___verbose == __stop___verbose) {
>> ^
>> 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 resulting assembly with either clang/ld.lld or
>> gcc/ld (tested with diff + objdump -Dr).
>>
>> Link: https://github.com/ClangBuiltLinux/linux/issues/894
>> Suggested-by: Nick Desaulniers <[email protected]>
>> Signed-off-by: Nathan Chancellor <[email protected]>
>> ---
>> v1 -> v2: https://lore.kernel.org/lkml/[email protected]/
>>
>> * No longer a series because there is no prerequisite patch.
>> * Use address-of operator instead of casting to unsigned long.
>>
>> lib/dynamic_debug.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>> index aae17d9522e5..8f199f403ab5 100644
>> --- a/lib/dynamic_debug.c
>> +++ b/lib/dynamic_debug.c
>> @@ -1031,7 +1031,7 @@ static int __init dynamic_debug_init(void)
>> int n = 0, entries = 0, modct = 0;
>> int verbose_bytes = 0;
>>
>> - if (__start___verbose == __stop___verbose) {
>> + if (&__start___verbose == &__stop___verbose) {
>> pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
>> return 1;
>> }
>> --
>> 2.25.1
>>
>
> Gentle ping for review/acceptance.
>
> Cheers,
> Nathan

Works for me.

Acked-by: Jason Baron <[email protected]>

2020-03-19 21:44:37

by Nathan Chancellor

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

On Thu, Mar 19, 2020 at 04:03:39PM -0400, Jason Baron wrote:
>
>
> On 3/18/20 9:59 PM, Nathan Chancellor wrote:
> > On Wed, Feb 19, 2020 at 10:13:20PM -0700, Nathan Chancellor wrote:
> >> Clang warns:
> >>
> >> ../lib/dynamic_debug.c:1034:24: warning: array comparison always
> >> evaluates to false [-Wtautological-compare]
> >> if (__start___verbose == __stop___verbose) {
> >> ^
> >> 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 resulting assembly with either clang/ld.lld or
> >> gcc/ld (tested with diff + objdump -Dr).
> >>
> >> Link: https://github.com/ClangBuiltLinux/linux/issues/894
> >> Suggested-by: Nick Desaulniers <[email protected]>
> >> Signed-off-by: Nathan Chancellor <[email protected]>
> >> ---
> >> v1 -> v2: https://lore.kernel.org/lkml/[email protected]/
> >>
> >> * No longer a series because there is no prerequisite patch.
> >> * Use address-of operator instead of casting to unsigned long.
> >>
> >> lib/dynamic_debug.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> >> index aae17d9522e5..8f199f403ab5 100644
> >> --- a/lib/dynamic_debug.c
> >> +++ b/lib/dynamic_debug.c
> >> @@ -1031,7 +1031,7 @@ static int __init dynamic_debug_init(void)
> >> int n = 0, entries = 0, modct = 0;
> >> int verbose_bytes = 0;
> >>
> >> - if (__start___verbose == __stop___verbose) {
> >> + if (&__start___verbose == &__stop___verbose) {
> >> pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
> >> return 1;
> >> }
> >> --
> >> 2.25.1
> >>
> >
> > Gentle ping for review/acceptance.
> >
> > Cheers,
> > Nathan
>
> Works for me.
>
> Acked-by: Jason Baron <[email protected]>

Thank you!

Andrew, I assume you'll pick this up?

Cheers,
Nathan