section mismatch test doesn't work reliably on arm64

[Masahiro Yamada]

On Sun, Oct 1, 2023 at 1:04 AM Uwe Kleine-König
<u.kleine-koenig at pengutronix.de> wrote:
>
> Hello,
>
> On Sat, Sep 30, 2023 at 08:08:05PM +0900, Masahiro Yamada wrote:
> > On Sat, Sep 30, 2023 at 2:41 AM Uwe Kleine-König
> > <u.kleine-koenig at pengutronix.de> wrote:
> > >
> > > Hello,
> > >
> > > On Fri, Sep 29, 2023 at 10:15:40AM +0200, Uwe Kleine-König wrote:
> > > > by manual inspection I found a section mismatch in
> > > > drivers/hwtracing/coresight/coresight-etm4x-core.c where
> > > > etm4_platform_driver (which lives in ".data") contains a reference to
> > > > etm4_remove_platform_dev() (which lives in ".exit.text").
> > > >
> > > > However building with CONFIG_DEBUG_SECTION_MISMATCH=y +
> > > > CONFIG_CORESIGHT_SOURCE_ETM4X=y doesn't warn about that one.
> > >
> > > Arnd had the right hint in irc: If I do
> > >
> > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > index 34a5386d444a..070e53be1ea2 100644
> > > --- a/scripts/mod/modpost.c
> > > +++ b/scripts/mod/modpost.c
> > > @@ -1017,7 +1017,7 @@ static int secref_whitelist(const char *fromsec, const char *fromsym,
> > >
> > >         /* symbols in data sections that may refer to meminit/exit sections */
> > >         if (match(fromsec, PATTERNS(DATA_SECTIONS)) &&
> > > -           match(tosec, PATTERNS(ALL_XXXINIT_SECTIONS, ALL_EXIT_SECTIONS)) &&
> > > +           match(tosec, PATTERNS(ALL_XXXINIT_SECTIONS, ALL_XXXEXIT_SECTIONS)) &&
> > >             match(fromsym, PATTERNS("*driver")))
> > >                 return 0;
> > >
> > > I get a mismatch warning:
> > >
> > > WARNING: modpost: vmlinux: section mismatch in reference: etm4x_amba_driver+0x98 (section: .data) -> etm4_remove_amba (section: .exit.text)
> > > WARNING: modpost: vmlinux: section mismatch in reference: etm4_platform_driver+0x8 (section: .data) -> etm4_remove_platform_dev (section: .exit.text)
> > > ERROR: modpost: Section mismatches detected.
> > >
> > > I remember that back in the times of CONFIG_HOTPLUG references to
> > > ".devinit.text" and ".devexit.text" were ok. Is a reference to
> > > .exit.text ever ok?
> > >
> > > I started an allyesconfig build on a few archs with the above patch
> > > applied. This will take some time, when it's done I will report what it
> > > found.
> >
> >
> >
> > allmodconfig on x86 detects several issues.
> >
> > [...]
> > WARNING: modpost: sound/soc/codecs/snd-soc-tlv320adc3xxx: section
> > mismatch in reference: adc3xxx_i2c_driver+0x10 (section: .data) ->
> > adc3xxx_i2c_remove (section: .exit.text)
>
> I checked just one of those, this is also a valid warning:
>
>         static void __exit adc3xxx_i2c_remove(struct i2c_client *client)
>         {
>                 ...
>         }
>
>         static struct i2c_driver adc3xxx_i2c_driver = {
>                 ...
>                 .remove = __exit_p(adc3xxx_i2c_remove),
>                 ...
>         };
>
> so if adc3xxx_i2c_remove is discarded (happens with
> CONFIG_SND_SOC_TLV320ADC3XXX=y) no warning happens, but .remove is just
> NULL and so if the driver is unbound no code runs and so it leaks
> resources. My thought was that allyesconfig would catch all problems,
> didn't consider driver authors to be that creative :-)


Yeah, allyesconfig does not detect this issue much due to __exit_p().
allmodconfig does.


The __exit_p() is questionable.
If we fix up all drivers, __exit_p() may go away.




-- 
Best Regards
Masahiro Yamada