[PATCH] linker_lists.h: Add attribute used to ll_entry_start macro

[Tom Rini]

On Thu, Mar 23, 2023 at 03:38:33PM -0700, appujee wrote:

> added comments: https://lists.denx.de/pipermail/u-boot/2023-March/513078.html

OK, and that references a gcc bugzilla entry where Andrew Pinski suggest
a change to correct the code, and says not to do what you're suggesting
here. And since there's still issues (as seen when moving to clang-15),
I want to see a patch that does what he suggests instead please, thanks.

> 

> On Thu, Mar 23, 2023 at 1:31 PM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Thu, Mar 23, 2023 at 01:29:29PM -0700, appujee wrote:
> >
> > > > So, saying "unused" and then "used" doesn't seem to make any sense.
> > > unused and used attributes do not cancel each other. They have different
> > > semantics. I agree this part of the code needs some attention. zero sized
> > > arrays are not C compliant as I understand it, even more so when it is
> > > declared outside of a struct.
> > >
> >
> > Ah, yeah, at least a comment explaining what and why we're doing what
> > we're doing here, and probably some other parts of the linker list code
> > that need addressing then, please.
> >
> >
> > > On Wed, Mar 22, 2023 at 4:58 PM Tom Rini <trini at konsulko.com> wrote:
> > >
> > > > On Tue, Feb 21, 2023 at 03:33:20PM -0800, Aditya Kumar wrote:
> > > >
> > > > > The variable gets dropped by clang compiler in an optimized builds.
> > > > > Adding attribute((used)) allows the symbol to be preserved. Similar
> > > > > changes have been proposed in the past e.g.,
> > > > > 569524741a01e1a96fc2b75dd7e5d12e41ce6c2b for ll_entry_declare macro.
> > > > >
> > > > > Signed-off-by: AdityaK <appujee at google.com>
> > > > > Reviewed-by: Simon Glass <sjg at chromium.org>
> > > > > ---
> > > > >  include/linker_lists.h | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/linker_lists.h b/include/linker_lists.h
> > > > > index d3da9d44e8..4cd13c3bc8 100644
> > > > > --- a/include/linker_lists.h
> > > > > +++ b/include/linker_lists.h
> > > > > @@ -125,7 +125,7 @@
> > > > >  #define ll_entry_start(_type, _list) \
> > > > >  ({ \
> > > > >   static char start[0] __aligned(CONFIG_LINKER_LIST_ALIGN) \
> > > > > - __attribute__((unused)) \
> > > > > + __attribute__((unused)) __attribute__((used)) \
> > > > >   __section("__u_boot_list_2_"#_list"_1"); \
> > > > >   (_type *)&start; \
> > > > >  })
> > > >
> > > > So, saying "unused" and then "used" doesn't seem to make any sense. And
> > > > given some other problems we see with newer clang, which Simon reports
> > > > this patch doesn't fully fix, we probably need to give that area a good
> > > > going over to see what attributes do and don't make sense, really.
> > > >
> > > > --
> > > > Tom
> > > >
> >
> > --
> > Tom

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230323/ed7b6389/attachment.sig>