* [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused @ 2018-09-26 7:02 Nathan Chancellor 2018-09-26 7:13 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Nathan Chancellor @ 2018-09-26 7:02 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: devel, linux-kernel, Nathan Chancellor Clang emits the following warning: drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable 'acpi_ids' is not needed and will not be emitted [-Wunneeded-internal-declaration] static const struct acpi_device_id acpi_ids[] = { ^ 1 warning generated. Mark the declaration as maybe unused like a few other instances of this construct in the kernel. Link: https://github.com/ClangBuiltLinux/linux/issues/169 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c index 6d02904de63f..3285bf36291b 100644 --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = { SDIO_DEVICE(0x024c, 0xb723), }, { /* end: all zeroes */ }, }; -static const struct acpi_device_id acpi_ids[] = { +static const struct acpi_device_id acpi_ids[] __maybe_unused = { {"OBDA8723", 0x0000}, {} }; -- 2.19.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused 2018-09-26 7:02 [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused Nathan Chancellor @ 2018-09-26 7:13 ` Greg Kroah-Hartman 2018-09-26 7:41 ` Nathan Chancellor 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2018-09-26 7:13 UTC (permalink / raw) To: Nathan Chancellor; +Cc: devel, linux-kernel On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote: > Clang emits the following warning: > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable > 'acpi_ids' is not needed and will not be emitted > [-Wunneeded-internal-declaration] > static const struct acpi_device_id acpi_ids[] = { > ^ > 1 warning generated. > > Mark the declaration as maybe unused like a few other instances of this > construct in the kernel. > > Link: https://github.com/ClangBuiltLinux/linux/issues/169 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > index 6d02904de63f..3285bf36291b 100644 > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = > { SDIO_DEVICE(0x024c, 0xb723), }, > { /* end: all zeroes */ }, > }; > -static const struct acpi_device_id acpi_ids[] = { > +static const struct acpi_device_id acpi_ids[] __maybe_unused = { But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE() macro does a functional thing. Why is gcc not reporting an issue with this and clang is? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused 2018-09-26 7:13 ` Greg Kroah-Hartman @ 2018-09-26 7:41 ` Nathan Chancellor 2018-09-26 17:55 ` Nick Desaulniers 0 siblings, 1 reply; 7+ messages in thread From: Nathan Chancellor @ 2018-09-26 7:41 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: devel, linux-kernel, Nick Desaulniers On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote: > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote: > > Clang emits the following warning: > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable > > 'acpi_ids' is not needed and will not be emitted > > [-Wunneeded-internal-declaration] > > static const struct acpi_device_id acpi_ids[] = { > > ^ > > 1 warning generated. > > > > Mark the declaration as maybe unused like a few other instances of this > > construct in the kernel. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169 > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > --- > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > index 6d02904de63f..3285bf36291b 100644 > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = > > { SDIO_DEVICE(0x024c, 0xb723), }, > > { /* end: all zeroes */ }, > > }; > > -static const struct acpi_device_id acpi_ids[] = { > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = { > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE() > macro does a functional thing. Why is gcc not reporting an issue with > this and clang is? > > thanks, > > greg k-h I am not entirely sure, I've added Nick to this thread to see what he thinks. I'm by no means a Clang expert at the moment. Thanks, Nathan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused 2018-09-26 7:41 ` Nathan Chancellor @ 2018-09-26 17:55 ` Nick Desaulniers 2018-09-26 18:28 ` Nick Desaulniers 0 siblings, 1 reply; 7+ messages in thread From: Nick Desaulniers @ 2018-09-26 17:55 UTC (permalink / raw) To: Nathan Chancellor; +Cc: Greg KH, devel, LKML On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote: > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote: > > > Clang emits the following warning: > > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable > > > 'acpi_ids' is not needed and will not be emitted > > > [-Wunneeded-internal-declaration] > > > static const struct acpi_device_id acpi_ids[] = { > > > ^ > > > 1 warning generated. > > > > > > Mark the declaration as maybe unused like a few other instances of this > > > construct in the kernel. > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169 > > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > > --- > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > index 6d02904de63f..3285bf36291b 100644 > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = > > > { SDIO_DEVICE(0x024c, 0xb723), }, > > > { /* end: all zeroes */ }, > > > }; > > > -static const struct acpi_device_id acpi_ids[] = { > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = { > > > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE() > > macro does a functional thing. Why is gcc not reporting an issue with > > this and clang is? Looks like GCC and Clang handle __attribute__((alias)) differently when optimizations are enabled. Clang has -Wunneeded-internal-declaration (GCC doesn't have that specific flag, which is why GCC doesn't report, but its behavior is also different), as part of -Wall, which warns that it thinks the static var is dead, then removes it from -O1 and up. https://godbolt.org/z/Dpxnbp I consider this a bug in Clang: https://bugs.llvm.org/show_bug.cgi?id=39088 As Nathan notes in this comment in V1: https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having additional references to the static array is enough to keep it around. When there are no other references, then it should not be marked static, in order to still be emitted. We can work around this by removing `static` from struct definitions that no other references than being passed to the MODULE_DEVICE_TABLE macro. GCC and Clang will then both emit references to both identifiers. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused 2018-09-26 17:55 ` Nick Desaulniers @ 2018-09-26 18:28 ` Nick Desaulniers 2018-09-26 18:39 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Nick Desaulniers @ 2018-09-26 18:28 UTC (permalink / raw) To: Nathan Chancellor; +Cc: Greg KH, devel, LKML, Pirama Arumuga Nainar On Wed, Sep 26, 2018 at 10:55 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote: > > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote: > > > > Clang emits the following warning: > > > > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable > > > > 'acpi_ids' is not needed and will not be emitted > > > > [-Wunneeded-internal-declaration] > > > > static const struct acpi_device_id acpi_ids[] = { > > > > ^ > > > > 1 warning generated. > > > > > > > > Mark the declaration as maybe unused like a few other instances of this > > > > construct in the kernel. > > > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169 > > > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > > > --- > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > index 6d02904de63f..3285bf36291b 100644 > > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = > > > > { SDIO_DEVICE(0x024c, 0xb723), }, > > > > { /* end: all zeroes */ }, > > > > }; > > > > -static const struct acpi_device_id acpi_ids[] = { > > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = { > > > > > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE() > > > macro does a functional thing. Why is gcc not reporting an issue with > > > this and clang is? > > Looks like GCC and Clang handle __attribute__((alias)) differently > when optimizations are enabled. Clang has > -Wunneeded-internal-declaration (GCC doesn't have that specific flag, > which is why GCC doesn't report, but its behavior is also different), > as part of -Wall, which warns that it thinks the static var is dead, > then removes it from -O1 and up. > > https://godbolt.org/z/Dpxnbp > > I consider this a bug in Clang: https://bugs.llvm.org/show_bug.cgi?id=39088 > > As Nathan notes in this comment in V1: > https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having > additional references to the static array is enough to keep it around. > When there are no other references, then it should not be marked > static, in order to still be emitted. > > We can work around this by removing `static` from struct definitions > that no other references than being passed to the MODULE_DEVICE_TABLE > macro. GCC and Clang will then both emit references to both > identifiers. Sorry, after thinking more about this and discussing more with a coworker, I think Clang is doing the right thing, and that `static` should be removed from declarations passed to MODULE_DEVICE_TABLE without other references. Clang's Dead Code Elimination (DCE) here seems to be more aggressive, but it still looks correct to me. int foo [] = { 42, 0xDEAD }; // extern extern typeof(foo) bar __attribute__((unused, alias("foo"))); GCC and Clang at -O2 both produce references to foo and bar. Adding `static` to foo, then GCC produces both references, while Clang moves the data to bar and removes foo. This is safe because foo has no other references, and bar is what file2alias.c cares about in this case. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused 2018-09-26 18:28 ` Nick Desaulniers @ 2018-09-26 18:39 ` Greg KH 2018-09-26 22:21 ` Nick Desaulniers 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2018-09-26 18:39 UTC (permalink / raw) To: Nick Desaulniers; +Cc: Nathan Chancellor, devel, LKML, Pirama Arumuga Nainar On Wed, Sep 26, 2018 at 11:28:46AM -0700, Nick Desaulniers wrote: > On Wed, Sep 26, 2018 at 10:55 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor > > <natechancellor@gmail.com> wrote: > > > > > > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote: > > > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote: > > > > > Clang emits the following warning: > > > > > > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable > > > > > 'acpi_ids' is not needed and will not be emitted > > > > > [-Wunneeded-internal-declaration] > > > > > static const struct acpi_device_id acpi_ids[] = { > > > > > ^ > > > > > 1 warning generated. > > > > > > > > > > Mark the declaration as maybe unused like a few other instances of this > > > > > construct in the kernel. > > > > > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169 > > > > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > > > > --- > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > > index 6d02904de63f..3285bf36291b 100644 > > > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = > > > > > { SDIO_DEVICE(0x024c, 0xb723), }, > > > > > { /* end: all zeroes */ }, > > > > > }; > > > > > -static const struct acpi_device_id acpi_ids[] = { > > > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = { > > > > > > > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE() > > > > macro does a functional thing. Why is gcc not reporting an issue with > > > > this and clang is? > > > > Looks like GCC and Clang handle __attribute__((alias)) differently > > when optimizations are enabled. Clang has > > -Wunneeded-internal-declaration (GCC doesn't have that specific flag, > > which is why GCC doesn't report, but its behavior is also different), > > as part of -Wall, which warns that it thinks the static var is dead, > > then removes it from -O1 and up. > > > > https://godbolt.org/z/Dpxnbp > > > > I consider this a bug in Clang: https://bugs.llvm.org/show_bug.cgi?id=39088 > > > > As Nathan notes in this comment in V1: > > https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having > > additional references to the static array is enough to keep it around. > > When there are no other references, then it should not be marked > > static, in order to still be emitted. > > > > We can work around this by removing `static` from struct definitions > > that no other references than being passed to the MODULE_DEVICE_TABLE > > macro. GCC and Clang will then both emit references to both > > identifiers. > > Sorry, after thinking more about this and discussing more with a > coworker, I think Clang is doing the right thing, and that `static` > should be removed from declarations passed to MODULE_DEVICE_TABLE > without other references. Clang's Dead Code Elimination (DCE) here > seems to be more aggressive, but it still looks correct to me. > > int foo [] = { 42, 0xDEAD }; // extern > extern typeof(foo) bar __attribute__((unused, alias("foo"))); > > GCC and Clang at -O2 both produce references to foo and bar. > > Adding `static` to foo, then GCC produces both references, while Clang > moves the data to bar and removes foo. This is safe because foo has > no other references, and bar is what file2alias.c cares about in this > case. But we want the variable to be static, and not part of any namespace outside of this file. If we start changing them all, we will start running into namespace collisions. These go into a separate part of the linker table, shouldn't that be enough to ensure that the compiler keeps it around? Or is that after the compiler processes things? Anything we can do to add to the MODULE_DEVICE_TABLE() macro instead? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused 2018-09-26 18:39 ` Greg KH @ 2018-09-26 22:21 ` Nick Desaulniers 0 siblings, 0 replies; 7+ messages in thread From: Nick Desaulniers @ 2018-09-26 22:21 UTC (permalink / raw) To: Greg KH; +Cc: Nathan Chancellor, devel, LKML, Pirama Arumuga Nainar On Wed, Sep 26, 2018 at 11:39 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Sep 26, 2018 at 11:28:46AM -0700, Nick Desaulniers wrote: > > On Wed, Sep 26, 2018 at 10:55 AM Nick Desaulniers > > <ndesaulniers@google.com> wrote: > > > > > > On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor > > > <natechancellor@gmail.com> wrote: > > > > > > > > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote: > > > > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote: > > > > > > Clang emits the following warning: > > > > > > > > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable > > > > > > 'acpi_ids' is not needed and will not be emitted > > > > > > [-Wunneeded-internal-declaration] > > > > > > static const struct acpi_device_id acpi_ids[] = { > > > > > > ^ > > > > > > 1 warning generated. > > > > > > > > > > > > Mark the declaration as maybe unused like a few other instances of this > > > > > > construct in the kernel. > > > > > > > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169 > > > > > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > > > > > --- > > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > > > index 6d02904de63f..3285bf36291b 100644 > > > > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > > > > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] = > > > > > > { SDIO_DEVICE(0x024c, 0xb723), }, > > > > > > { /* end: all zeroes */ }, > > > > > > }; > > > > > > -static const struct acpi_device_id acpi_ids[] = { > > > > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = { > > > > > > > > > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE() > > > > > macro does a functional thing. Why is gcc not reporting an issue with > > > > > this and clang is? > > > > > > Looks like GCC and Clang handle __attribute__((alias)) differently > > > when optimizations are enabled. Clang has > > > -Wunneeded-internal-declaration (GCC doesn't have that specific flag, > > > which is why GCC doesn't report, but its behavior is also different), > > > as part of -Wall, which warns that it thinks the static var is dead, > > > then removes it from -O1 and up. > > > > > > https://godbolt.org/z/Dpxnbp > > > > > > I consider this a bug in Clang: https://bugs.llvm.org/show_bug.cgi?id=39088 > > > > > > As Nathan notes in this comment in V1: > > > https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having > > > additional references to the static array is enough to keep it around. > > > When there are no other references, then it should not be marked > > > static, in order to still be emitted. > > > > > > We can work around this by removing `static` from struct definitions > > > that no other references than being passed to the MODULE_DEVICE_TABLE > > > macro. GCC and Clang will then both emit references to both > > > identifiers. > > > > Sorry, after thinking more about this and discussing more with a > > coworker, I think Clang is doing the right thing, and that `static` > > should be removed from declarations passed to MODULE_DEVICE_TABLE > > without other references. Clang's Dead Code Elimination (DCE) here > > seems to be more aggressive, but it still looks correct to me. > > > > int foo [] = { 42, 0xDEAD }; // extern > > extern typeof(foo) bar __attribute__((unused, alias("foo"))); > > > > GCC and Clang at -O2 both produce references to foo and bar. > > > > Adding `static` to foo, then GCC produces both references, while Clang > > moves the data to bar and removes foo. This is safe because foo has > > no other references, and bar is what file2alias.c cares about in this > > case. > > But we want the variable to be static, and not part of any namespace > outside of this file. If we start changing them all, we will start > running into namespace collisions. That's a fair point. `__attribute__((used))` (in the kernel, we use the macro `__used`) will silence this warning in Clang for this particular instance while being no functional change for GCC or Clang. Nathan's v2 solution was very close! Either attribute silences the warning, but __used makes it clearer that we don't want the warning, without inhibiting dead code elimination from pruning the unreferenced internal symbol while moving its data to the external/globally reference-able symbol. Exploring more of what GCC is doing (feel free to skip this, as it's minutia): GCC emits BOTH symbols foo and bar (from my above simplified case, when foo is static), but only bar (the alias) is in the global namespace (extern linkage). I missed in the disassembly that only bar had the `.globl` assembler directive and assumed labels were by default globl (their not, I've discovered from testing. Today I learned). More concretely, looking at the symbols of the translation unit in question (rather than my simplified toy): $ nm -S drivers/staging/rtl8723bs/os_dep/sdio_intf.o | grep acpi 00000000000000e0 0000000000000040 r acpi_ids 00000000000000e0 0000000000000040 R __mod_acpi__acpi_ids_device_tabl Shows that both symbols are created, `man nm` says about the capitalization and r vs R: "if uppercase, the symbol is global (external)." "R" "r" The symbol is in a read only data section. The disassembler only shows the reference to __mod_acpi__acpi_ids_device_tabl at file offset 0x15a0: $ objdump -DF drivers/staging/rtl8723bs/os_dep/sdio_intf.o |less 00000000000000e0 <__mod_acpi__acpi_ids_device_table> (File Offset: 0x15a0): And a dump of the binary shows the data from acpi_ids at that offset: $ hexdump -C drivers/staging/rtl8723bs/os_dep/sdio_intf.o| less 000015a0 4f 42 44 41 38 37 32 33 00 00 00 00 00 00 00 00 |OBDA8723........| ^ should look familiar based on the declaration in question. The same can be verified with Clang, which wont produce the internal only symbol (that has zero references) acpi_ids, and will place __mod_acpi__acpi_ids_device_tabl at file offset 0x1610 for that translation unit. > > These go into a separate part of the linker table, > shouldn't that be > enough to ensure that the compiler keeps it around? Or is that after > the compiler processes things? > > Anything we can do to add to the MODULE_DEVICE_TABLE() macro instead? -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-09-26 22:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-26 7:02 [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused Nathan Chancellor 2018-09-26 7:13 ` Greg Kroah-Hartman 2018-09-26 7:41 ` Nathan Chancellor 2018-09-26 17:55 ` Nick Desaulniers 2018-09-26 18:28 ` Nick Desaulniers 2018-09-26 18:39 ` Greg KH 2018-09-26 22:21 ` Nick Desaulniers
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).