* [PATCH] powerpc/boot: Use address-of operator on section symbols
@ 2020-06-24 3:59 Nathan Chancellor
2020-06-25 1:18 ` Geoff Levand
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Nathan Chancellor @ 2020-06-24 3:59 UTC (permalink / raw)
To: Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, Geoff Levand,
linuxppc-dev, linux-kernel, clang-built-linux, Nathan Chancellor,
Joel Stanley
Clang warns:
arch/powerpc/boot/main.c:107:18: warning: array comparison always
evaluates to a constant [-Wtautological-compare]
if (_initrd_end > _initrd_start) {
^
arch/powerpc/boot/main.c:155:20: warning: array comparison always
evaluates to a constant [-Wtautological-compare]
if (_esm_blob_end <= _esm_blob_start)
^
2 warnings 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/212
Reported-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
arch/powerpc/boot/main.c | 4 ++--
arch/powerpc/boot/ps3.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index a9d209135975..cae31a6e8f02 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -104,7 +104,7 @@ static struct addr_range prep_initrd(struct addr_range vmlinux, void *chosen,
{
/* If we have an image attached to us, it overrides anything
* supplied by the loader. */
- if (_initrd_end > _initrd_start) {
+ if (&_initrd_end > &_initrd_start) {
printf("Attached initrd image at 0x%p-0x%p\n\r",
_initrd_start, _initrd_end);
initrd_addr = (unsigned long)_initrd_start;
@@ -152,7 +152,7 @@ static void prep_esm_blob(struct addr_range vmlinux, void *chosen)
unsigned long esm_blob_addr, esm_blob_size;
/* Do we have an ESM (Enter Secure Mode) blob? */
- if (_esm_blob_end <= _esm_blob_start)
+ if (&_esm_blob_end <= &_esm_blob_start)
return;
printf("Attached ESM blob at 0x%p-0x%p\n\r",
diff --git a/arch/powerpc/boot/ps3.c b/arch/powerpc/boot/ps3.c
index c52552a681c5..6e4efbdb6b7c 100644
--- a/arch/powerpc/boot/ps3.c
+++ b/arch/powerpc/boot/ps3.c
@@ -127,7 +127,7 @@ void platform_init(void)
ps3_repository_read_rm_size(&rm_size);
dt_fixup_memory(0, rm_size);
- if (_initrd_end > _initrd_start) {
+ if (&_initrd_end > &_initrd_start) {
setprop_val(chosen, "linux,initrd-start", (u32)(_initrd_start));
setprop_val(chosen, "linux,initrd-end", (u32)(_initrd_end));
}
base-commit: 3e08a95294a4fb3702bb3d35ed08028433c37fe6
--
2.27.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-06-24 3:59 [PATCH] powerpc/boot: Use address-of operator on section symbols Nathan Chancellor
@ 2020-06-25 1:18 ` Geoff Levand
2020-06-25 2:22 ` Nathan Chancellor
2020-06-25 16:32 ` Nick Desaulniers
2020-07-16 12:56 ` Michael Ellerman
2020-07-18 7:50 ` Geert Uytterhoeven
2 siblings, 2 replies; 12+ messages in thread
From: Geoff Levand @ 2020-06-25 1:18 UTC (permalink / raw)
To: Nathan Chancellor, Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
linux-kernel, clang-built-linux, Joel Stanley
Hi Nathan,
On 6/23/20 8:59 PM, Nathan Chancellor wrote:
> 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).
Thanks for your patch. I tested this patch applied to v5.8-rc2 on a
PS3 and it seems OK.
Tested-by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-06-25 1:18 ` Geoff Levand
@ 2020-06-25 2:22 ` Nathan Chancellor
2020-06-25 16:32 ` Nick Desaulniers
1 sibling, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2020-06-25 2:22 UTC (permalink / raw)
To: Geoff Levand
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev, linux-kernel, clang-built-linux, Joel Stanley
Hi Geoff,
On Wed, Jun 24, 2020 at 06:18:48PM -0700, Geoff Levand wrote:
> Hi Nathan,
>
> On 6/23/20 8:59 PM, Nathan Chancellor wrote:
> > 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).
>
> Thanks for your patch. I tested this patch applied to v5.8-rc2 on a
> PS3 and it seems OK.
>
> Tested-by: Geoff Levand <geoff@infradead.org>
Thanks a lot for the quick response and testing, I really appreciate it!
Cheers,
Nathan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-06-25 1:18 ` Geoff Levand
2020-06-25 2:22 ` Nathan Chancellor
@ 2020-06-25 16:32 ` Nick Desaulniers
2020-07-18 9:13 ` Arnd Bergmann
1 sibling, 1 reply; 12+ messages in thread
From: Nick Desaulniers @ 2020-06-25 16:32 UTC (permalink / raw)
To: Geoff Levand
Cc: Nathan Chancellor, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, LKML, clang-built-linux,
Joel Stanley
On Wed, Jun 24, 2020 at 6:19 PM Geoff Levand <geoff@infradead.org> wrote:
>
> Hi Nathan,
>
> On 6/23/20 8:59 PM, Nathan Chancellor wrote:
> > 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).
>
> Thanks for your patch. I tested this patch applied to v5.8-rc2 on a
> PS3 and it seems OK.
PS3? Folks still have ones that can boot Linux? Those ****ers took
my Yellow Dog Linux away from me; I enjoyed depositing that settlement
check! Hopefully by now, folks have figured out how to roll back the
system firmware?
>
> Tested-by: Geoff Levand <geoff@infradead.org>
>
>
> --
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-06-24 3:59 [PATCH] powerpc/boot: Use address-of operator on section symbols Nathan Chancellor
2020-06-25 1:18 ` Geoff Levand
@ 2020-07-16 12:56 ` Michael Ellerman
2020-07-18 7:50 ` Geert Uytterhoeven
2 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2020-07-16 12:56 UTC (permalink / raw)
To: Michael Ellerman, Nathan Chancellor
Cc: linux-kernel, Geoff Levand, Joel Stanley, clang-built-linux,
linuxppc-dev, Paul Mackerras
On Tue, 23 Jun 2020 20:59:20 -0700, Nathan Chancellor wrote:
> Clang warns:
>
> arch/powerpc/boot/main.c:107:18: warning: array comparison always
> evaluates to a constant [-Wtautological-compare]
> if (_initrd_end > _initrd_start) {
> ^
> arch/powerpc/boot/main.c:155:20: warning: array comparison always
> evaluates to a constant [-Wtautological-compare]
> if (_esm_blob_end <= _esm_blob_start)
> ^
> 2 warnings generated.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc/boot: Use address-of operator on section symbols
https://git.kernel.org/powerpc/c/df4232d96e724d09e54a623362f9f610727f059f
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-06-24 3:59 [PATCH] powerpc/boot: Use address-of operator on section symbols Nathan Chancellor
2020-06-25 1:18 ` Geoff Levand
2020-07-16 12:56 ` Michael Ellerman
@ 2020-07-18 7:50 ` Geert Uytterhoeven
2020-07-18 15:31 ` Nathan Chancellor
2020-07-20 21:02 ` Segher Boessenkool
2 siblings, 2 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2020-07-18 7:50 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Geoff Levand, linuxppc-dev, Linux Kernel Mailing List,
clang-built-linux, Joel Stanley, Arnd Bergmann
Hi Nathan,
On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
> arch/powerpc/boot/main.c:107:18: warning: array comparison always
> evaluates to a constant [-Wtautological-compare]
> if (_initrd_end > _initrd_start) {
> ^
> arch/powerpc/boot/main.c:155:20: warning: array comparison always
> evaluates to a constant [-Wtautological-compare]
> if (_esm_blob_end <= _esm_blob_start)
> ^
> 2 warnings 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/212
> Reported-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> arch/powerpc/boot/main.c | 4 ++--
> arch/powerpc/boot/ps3.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
> index a9d209135975..cae31a6e8f02 100644
> --- a/arch/powerpc/boot/main.c
> +++ b/arch/powerpc/boot/main.c
> @@ -104,7 +104,7 @@ static struct addr_range prep_initrd(struct addr_range vmlinux, void *chosen,
> {
> /* If we have an image attached to us, it overrides anything
> * supplied by the loader. */
> - if (_initrd_end > _initrd_start) {
> + if (&_initrd_end > &_initrd_start) {
>
Are you sure that fix is correct?
extern char _initrd_start[];
extern char _initrd_end[];
extern char _esm_blob_start[];
extern char _esm_blob_end[];
Of course the result of their comparison is a constant, as the addresses
are constant. If clangs warns about it, perhaps that warning should be moved
to W=1?
But adding "&" is not correct, according to C.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-06-25 16:32 ` Nick Desaulniers
@ 2020-07-18 9:13 ` Arnd Bergmann
0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2020-07-18 9:13 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Geoff Levand, Nathan Chancellor, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, LKML,
clang-built-linux, Joel Stanley
On Thu, Jun 25, 2020 at 6:32 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Wed, Jun 24, 2020 at 6:19 PM Geoff Levand <geoff@infradead.org> wrote:
> >
> > Hi Nathan,
> >
> > On 6/23/20 8:59 PM, Nathan Chancellor wrote:
> > > 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).
> >
> > Thanks for your patch. I tested this patch applied to v5.8-rc2 on a
> > PS3 and it seems OK.
>
> PS3? Folks still have ones that can boot Linux? Those ****ers took
> my Yellow Dog Linux away from me; I enjoyed depositing that settlement
> check! Hopefully by now, folks have figured out how to roll back the
> system firmware?
I still have the PS3 from Hong Kong with original 1.5 (IIRC) firmware
that I demoed at LCA2006. Haven't booted it in at least 12 years, and
never used it for games or movies other than the free "Casino Royale"
they sent everyone.
Arnd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-07-18 7:50 ` Geert Uytterhoeven
@ 2020-07-18 15:31 ` Nathan Chancellor
2020-07-20 21:02 ` Segher Boessenkool
1 sibling, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2020-07-18 15:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Geoff Levand, linuxppc-dev, Linux Kernel Mailing List,
clang-built-linux, Joel Stanley, Arnd Bergmann
On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
> Hi Nathan,
>
> On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> > arch/powerpc/boot/main.c:107:18: warning: array comparison always
> > evaluates to a constant [-Wtautological-compare]
> > if (_initrd_end > _initrd_start) {
> > ^
> > arch/powerpc/boot/main.c:155:20: warning: array comparison always
> > evaluates to a constant [-Wtautological-compare]
> > if (_esm_blob_end <= _esm_blob_start)
> > ^
> > 2 warnings 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/212
> > Reported-by: Joel Stanley <joel@jms.id.au>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > arch/powerpc/boot/main.c | 4 ++--
> > arch/powerpc/boot/ps3.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
> > index a9d209135975..cae31a6e8f02 100644
> > --- a/arch/powerpc/boot/main.c
> > +++ b/arch/powerpc/boot/main.c
> > @@ -104,7 +104,7 @@ static struct addr_range prep_initrd(struct addr_range vmlinux, void *chosen,
> > {
> > /* If we have an image attached to us, it overrides anything
> > * supplied by the loader. */
> > - if (_initrd_end > _initrd_start) {
> > + if (&_initrd_end > &_initrd_start) {
> >
>
> Are you sure that fix is correct?
>
> extern char _initrd_start[];
> extern char _initrd_end[];
> extern char _esm_blob_start[];
> extern char _esm_blob_end[];
>
> Of course the result of their comparison is a constant, as the addresses
> are constant. If clangs warns about it, perhaps that warning should be moved
> to W=1?
>
> But adding "&" is not correct, according to C.
>
> Gr{oetje,eeting}s,
>
> Geert
>
Hi Geert,
Yes, I have done fairly extensive testing in the past to verify that
this fix is correct.
For example:
$ cat test.c
#include <stdio.h>
extern char _test[];
int main(void)
{
printf("_test: %p\n", _test);
printf("&_test: %p\n", &_test);
return 0;
}
$ cat test.lds
_test = .;
$ clang -Wl,-T test.lds test.c
$ ./a.out
_test: 0x204
&_test: 0x204
$ gcc -fuse-ld=lld -Wl,-T test.lds test.c
$ ./a.out
_test: 0x60a0f76301fb
&_test: 0x60a0f76301fb
I also did runtime verification in QEMU to confirm this is true when I
was testing these commits, which are already present in Linus' tree:
63174f61dfae ("kernel/extable.c: use address-of operator on section symbols")
bf2cbe044da2 ("tracing: Use address-of operator on section symbols")
8306b057a85e ("lib/dynamic_debug.c: use address-of operator on section symbols")
b0d14fc43d39 ("mm/kmemleak.c: use address-of operator on section symbols")
I did a lot of work to get this warning enabled as it can find bugs:
6def1a1d2d58 ("fanotify: Fix the checks in fanotify_fsid_equal")
79ba4f931067 ("IB/hfi1: Fix logical condition in msix_request_irq")
-Wno-tautological-compare disables a bunch of good subwarnings, as I
point out in the commit that enabled it:
afe956c577b2 ("kbuild: Enable -Wtautological-compare")
Cheers,
Nathan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-07-18 7:50 ` Geert Uytterhoeven
2020-07-18 15:31 ` Nathan Chancellor
@ 2020-07-20 21:02 ` Segher Boessenkool
2020-08-03 10:06 ` Geert Uytterhoeven
1 sibling, 1 reply; 12+ messages in thread
From: Segher Boessenkool @ 2020-07-20 21:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Nathan Chancellor, Arnd Bergmann, Geoff Levand,
Linux Kernel Mailing List, clang-built-linux, Paul Mackerras,
Joel Stanley, linuxppc-dev
Hi!
On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
> On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> > /* If we have an image attached to us, it overrides anything
> > * supplied by the loader. */
> > - if (_initrd_end > _initrd_start) {
> > + if (&_initrd_end > &_initrd_start) {
>
> Are you sure that fix is correct?
>
> extern char _initrd_start[];
> extern char _initrd_end[];
> extern char _esm_blob_start[];
> extern char _esm_blob_end[];
>
> Of course the result of their comparison is a constant, as the addresses
> are constant. If clangs warns about it, perhaps that warning should be moved
> to W=1?
>
> But adding "&" is not correct, according to C.
Why not?
6.5.3.2/3
The unary & operator yields the address of its operand. [...]
Otherwise, the result is a pointer to the object or function designated
by its operand.
This is the same as using the name of an array without anything else,
yes. It is a bit clearer if it would not be declared as array, perhaps,
but it is correct just fine like this.
Segher
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-07-20 21:02 ` Segher Boessenkool
@ 2020-08-03 10:06 ` Geert Uytterhoeven
2020-08-03 11:09 ` Michael Ellerman
0 siblings, 1 reply; 12+ messages in thread
From: Geert Uytterhoeven @ 2020-08-03 10:06 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Nathan Chancellor, Arnd Bergmann, Geoff Levand,
Linux Kernel Mailing List, clang-built-linux, Paul Mackerras,
Joel Stanley, linuxppc-dev
Hi Segher,
On Mon, Jul 20, 2020 at 11:03 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
> > On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > > /* If we have an image attached to us, it overrides anything
> > > * supplied by the loader. */
> > > - if (_initrd_end > _initrd_start) {
> > > + if (&_initrd_end > &_initrd_start) {
> >
> > Are you sure that fix is correct?
> >
> > extern char _initrd_start[];
> > extern char _initrd_end[];
> > extern char _esm_blob_start[];
> > extern char _esm_blob_end[];
> >
> > Of course the result of their comparison is a constant, as the addresses
> > are constant. If clangs warns about it, perhaps that warning should be moved
> > to W=1?
> >
> > But adding "&" is not correct, according to C.
>
> Why not?
>
> 6.5.3.2/3
> The unary & operator yields the address of its operand. [...]
> Otherwise, the result is a pointer to the object or function designated
> by its operand.
>
> This is the same as using the name of an array without anything else,
> yes. It is a bit clearer if it would not be declared as array, perhaps,
> but it is correct just fine like this.
Thanks, I stand corrected.
Regardless, the comparison is still a comparison between two constant
addresses, so my fear is that the compiler will start generating
warnings for that in the near or distant future, making this change
futile.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-08-03 10:06 ` Geert Uytterhoeven
@ 2020-08-03 11:09 ` Michael Ellerman
2020-08-03 11:32 ` Geert Uytterhoeven
0 siblings, 1 reply; 12+ messages in thread
From: Michael Ellerman @ 2020-08-03 11:09 UTC (permalink / raw)
To: Geert Uytterhoeven, Segher Boessenkool
Cc: Nathan Chancellor, Arnd Bergmann, Geoff Levand,
Linux Kernel Mailing List, clang-built-linux, Paul Mackerras,
Joel Stanley, linuxppc-dev
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> On Mon, Jul 20, 2020 at 11:03 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>> On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
>> > On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
>> > <natechancellor@gmail.com> wrote:
>> > > /* If we have an image attached to us, it overrides anything
>> > > * supplied by the loader. */
>> > > - if (_initrd_end > _initrd_start) {
>> > > + if (&_initrd_end > &_initrd_start) {
>> >
>> > Are you sure that fix is correct?
>> >
>> > extern char _initrd_start[];
>> > extern char _initrd_end[];
>> > extern char _esm_blob_start[];
>> > extern char _esm_blob_end[];
>> >
>> > Of course the result of their comparison is a constant, as the addresses
>> > are constant. If clangs warns about it, perhaps that warning should be moved
>> > to W=1?
>> >
>> > But adding "&" is not correct, according to C.
>>
>> Why not?
>>
>> 6.5.3.2/3
>> The unary & operator yields the address of its operand. [...]
>> Otherwise, the result is a pointer to the object or function designated
>> by its operand.
>>
>> This is the same as using the name of an array without anything else,
>> yes. It is a bit clearer if it would not be declared as array, perhaps,
>> but it is correct just fine like this.
>
> Thanks, I stand corrected.
>
> Regardless, the comparison is still a comparison between two constant
> addresses, so my fear is that the compiler will start generating
> warnings for that in the near or distant future, making this change
> futile.
They're not constant at compile time though. So I don't think the
compiler could (sensibly) warn about that? (surely!)
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc/boot: Use address-of operator on section symbols
2020-08-03 11:09 ` Michael Ellerman
@ 2020-08-03 11:32 ` Geert Uytterhoeven
0 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2020-08-03 11:32 UTC (permalink / raw)
To: Michael Ellerman
Cc: Segher Boessenkool, Nathan Chancellor, Arnd Bergmann,
Geoff Levand, Linux Kernel Mailing List, clang-built-linux,
Paul Mackerras, Joel Stanley, linuxppc-dev
Hi Michael,
On Mon, Aug 3, 2020 at 1:09 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> > On Mon, Jul 20, 2020 at 11:03 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> >> On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
> >> > On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
> >> > <natechancellor@gmail.com> wrote:
> >> > > /* If we have an image attached to us, it overrides anything
> >> > > * supplied by the loader. */
> >> > > - if (_initrd_end > _initrd_start) {
> >> > > + if (&_initrd_end > &_initrd_start) {
> >> >
> >> > Are you sure that fix is correct?
> >> >
> >> > extern char _initrd_start[];
> >> > extern char _initrd_end[];
> >> > extern char _esm_blob_start[];
> >> > extern char _esm_blob_end[];
> >> >
> >> > Of course the result of their comparison is a constant, as the addresses
> >> > are constant. If clangs warns about it, perhaps that warning should be moved
> >> > to W=1?
> >> >
> >> > But adding "&" is not correct, according to C.
> >>
> >> Why not?
> >>
> >> 6.5.3.2/3
> >> The unary & operator yields the address of its operand. [...]
> >> Otherwise, the result is a pointer to the object or function designated
> >> by its operand.
> >>
> >> This is the same as using the name of an array without anything else,
> >> yes. It is a bit clearer if it would not be declared as array, perhaps,
> >> but it is correct just fine like this.
> >
> > Thanks, I stand corrected.
> >
> > Regardless, the comparison is still a comparison between two constant
> > addresses, so my fear is that the compiler will start generating
> > warnings for that in the near or distant future, making this change
> > futile.
>
> They're not constant at compile time though. So I don't think the
> compiler could (sensibly) warn about that? (surely!)
They're constant, but the compiler doesn't know their value.
That doesn't change by (not) using the address-of operator.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-08-03 11:32 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 3:59 [PATCH] powerpc/boot: Use address-of operator on section symbols Nathan Chancellor
2020-06-25 1:18 ` Geoff Levand
2020-06-25 2:22 ` Nathan Chancellor
2020-06-25 16:32 ` Nick Desaulniers
2020-07-18 9:13 ` Arnd Bergmann
2020-07-16 12:56 ` Michael Ellerman
2020-07-18 7:50 ` Geert Uytterhoeven
2020-07-18 15:31 ` Nathan Chancellor
2020-07-20 21:02 ` Segher Boessenkool
2020-08-03 10:06 ` Geert Uytterhoeven
2020-08-03 11:09 ` Michael Ellerman
2020-08-03 11:32 ` Geert Uytterhoeven
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).