Re: [Minios-devel] [UNIKRAFT PATCH v2 01/23] include/uk: introduce __weak_alias macro

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Yuri,

Please find the comment inline.

Thanks & Regards
Sharan


On 2/6/19 5:27 PM, Yuri Volchkov wrote:
> Hi,
>
> well, this is not a correct behavior of gcc. I don't like the idea of
> polluting build log with warnings about not your problem and which you
> can not fix. And I don't like the idea of ditching something as
> essential as weak aliasing.
I agree.

> Another thing, LTO resolves a weak symbol only if it is referenced
> inside a library. This is exactly the idea behind the workaround for
> this "multiple definition of `main'" patch you reviewed earlier
> today. Yes you could perhaps check if this is the case, and produce the
> warning only if 1) LTO is enabled 2) the weak symbol is referenced
> inside the library it was defined. I think this is way too complex for
> the outcome.
I was referring to the use case where the function is used within the 
library. I was more in favor of adding a code-comment with a "NOTE:"
mentioning about the bug. Just to remind us of this bug in LTO.

> - BR, Yuri.
>
> Sharan Santhanam <sharan.santhanam@xxxxxxxxx> writes:
>
> > Hello Yuri,
> >
> > This patch functionally seems fine.
> >
> > I have a question with regard to its usage especially with LTO enabled.
> > The library which uses this macro should not use the function internally
> > as LTO would internally resolve it with the weak symbol. I think it
> > would be wise to add a comment to warn the users of the macro about it.
> >
> > Thanks & Regards
> > Sharan
> >
> > On 2/4/19 3:36 PM, Yuri Volchkov wrote:
> > > Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
> > > ---
> > >    include/uk/essentials.h | 4 ++++
> > >    1 file changed, 4 insertions(+)
> > >
> > > diff --git a/include/uk/essentials.h b/include/uk/essentials.h
> > > index 3c16e308..dc1d69b0 100644
> > > --- a/include/uk/essentials.h
> > > +++ b/include/uk/essentials.h
> > > @@ -76,6 +76,10 @@ extern "C" {
> > >    #ifndef __align
> > >    #define __align(bytes)         __attribute__((aligned(bytes)))
> > >    #endif
> > > +#ifndef __weak_alias
> > > +#define __weak_alias(old, new) \
> > > +       extern __typeof(old) new __attribute__((weak, alias(#old)))
> > > +#endif
> > >    
> > >    /**
> > >      * Mark a function as constructor
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel