I think the correct fix is to not use a zero sized array at all. AIUI, what Andrew Pinksi intended was: the `asm` fix and attribute(unused) fix aren't the same thing.
Using either way to get around this is probably not going to work in future; but I'm not familiar with the codebase to do large changes. On Thu, Mar 23, 2023 at 3:44 PM Tom Rini <tr...@konsulko.com> wrote: > > 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 <tr...@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 <tr...@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 <appu...@google.com> > > > > > > Reviewed-by: Simon Glass <s...@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