Re: [RFC PATCH 1/3] modpost: fix TO_NATIVE() with expressions and consts

From: Alexander Lobakin
Date: Thu Aug 18 2022 - 10:03:46 EST


From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
Date: Thu, 18 Aug 2022 14:26:14 +0200

> On Thu, Aug 18, 2022 at 01:53:04PM +0200, Alexander Lobakin wrote:
> > Macro TO_NATIVE() directly takes a reference to its argument @x
> > without making an intermediate variable. This makes compilers
> > emit build warnings and errors if @x is an expression or a deref
> > of a const pointer (when target Endianness != host Endianness):
> >
> > >> scripts/mod/modpost.h:87:18: error: lvalue required as unary '&' operand
> > 87 | __endian(&(x), &(__x), sizeof(__x)); \
> > | ^
> > scripts/mod/sympath.c:19:25: note: in expansion of macro 'TO_NATIVE'
> > 19 | #define t(x) TO_NATIVE(x)
> > | ^~~~~~~~~
> > scripts/mod/sympath.c:100:31: note: in expansion of macro 't'
> > 100 | eh->e_shoff = t(h(eh->e_shoff) + off);
> >
> > >> scripts/mod/modpost.h:87:24: warning: passing argument 2 of '__endian'
> > discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> > 87 | __endian(&(x), &(__x), sizeof(__x)); \
> > | ^~~~~~
> > scripts/mod/sympath.c:18:25: note: in expansion of macro 'TO_NATIVE'
> > 18 | #define h(x) TO_NATIVE(x)
> > | ^~~~~~~~~
> > scripts/mod/sympath.c:178:48: note: in expansion of macro 'h'
> > 178 | iter < end; iter = (void *)iter + h(eh->e_shentsize)) {
>
> How come this hasn't shown up in cross-builds today?

It doesn't happen with the current code.

>
>
> >
> > Create a temporary variable, assign @x to it and don't use @x after
> > that. This makes it possible to pass expressions as an argument.
> > Also, do a cast-away for the second argument when calling __endian()
> > to avoid 'discarded qualifiers' warning, as typeof() preserves
> > qualifiers and makes compilers think that we're passing pointer
> > to a const.
> >
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@xxxxxxxxxxxxxxx # 4.9+
>
> Where are these build warnings showing up at that we don't see them
> today, yet this is needed to go back to all stable trees?

I thought all fixes should go to the applicable stable trees, am I
wrong? If so, I'll drop the tag in the next spin.

I remember we had such discussion already regarding fixing stuff in
modpost, which can happen only with never mainlained GCC LTO or with
the in-dev code. At the end that fix made it into the stables IIRC.

>
> still confused,
>
> greg k-h

Thanks,
Olek