On Mon, Mar 30, 2020 at 10:49 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> On Wed, Mar 11, 2020 at 10:03 AM Jessica Yu <jeyu@xxxxxxxxxx> wrote:
> >
> > In order to preserve backwards compatability with kmod tools, we have to
> > move the namespace field in Module.symvers last, as the depmod -e -E
> > option looks at the first three fields in Module.symvers to check symbol
> > versions (and it's expected they stay in the original order of crc,
> > symbol, module).
> >
> > In addition, update an ancient comment above read_dump() in modpost that
> > suggested that the export type field in Module.symvers was optional. I
> > suspect that there were historical reasons behind that comment that are
> > no longer accurate. We have been unconditionally printing the export
> > type since 2.6.18 (commit bd5cbcedf44), which is over a decade ago now.
> >
> > Fix up read_dump() to treat each field as non-optional. I suspect the
> > original read_dump() code treated the export field as optional in order
> > to support pre <= 2.6.18 Module.symvers (which did not have the export
> > type field). Note that although symbol namespaces are optional, the
> > field will not be omitted from Module.symvers if a symbol does not have
> > a namespace. In this case, the field will simply be empty and the next
> > delimiter or end of line will follow.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: cb9b55d21fe0 ("modpost: add support for symbol namespaces")
> > Tested-by: Matthias Maennich <maennich@xxxxxxxxxx>
> > Reviewed-by: Matthias Maennich <maennich@xxxxxxxxxx>
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Signed-off-by: Jessica Yu <jeyu@xxxxxxxxxx>
> > ---
> > v2:
> >
> > - Explain the changes to read_dump() and the comment (and provide
> > historical context) in the commit message. (Lucas De Marchi)
> >
> > Documentation/kbuild/modules.rst | 4 ++--
> > scripts/export_report.pl | 2 +-
> > scripts/mod/modpost.c | 24 ++++++++++++------------
> > 3 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/Documentation/kbuild/modules.rst b/Documentation/kbuild/modules.rst
> > index 69fa48ee93d6..e0b45a257f21 100644
> > --- a/Documentation/kbuild/modules.rst
> > +++ b/Documentation/kbuild/modules.rst
> > @@ -470,9 +470,9 @@ build.
> >
> > The syntax of the Module.symvers file is::
> >
> > - <CRC> <Symbol> <Namespace> <Module> <Export Type>
> > + <CRC> <Symbol> <Module> <Export Type> <Namespace>
> >
> > - 0xe1cc2a05 usb_stor_suspend USB_STORAGE drivers/usb/storage/usb-storage EXPORT_SYMBOL_GPL
> > + 0xe1cc2a05 usb_stor_suspend drivers/usb/storage/usb-storage EXPORT_SYMBOL_GPL USB_STORAGE
> >
> > The fields are separated by tabs and values may be empty (e.g.
> > if no namespace is defined for an exported symbol).
>
> Despite the documentation here claiming the namespace field can be
> empty, I'm seeing some trouble with this patch when building external
> modules:
> FATAL: parse error in symbol dump file
>
> I've confirmed reverting it make things work again, but its not clear
> to me quite yet why.
>
> The only difference I can find is that the Module.symvers in the
> external module project doesn't seem to have a tab at the end of each
> line (where as Module.symvers for the kernel - which doesn't seem to
> have any namespace names - does).
>
> Is there something I need to tweak on the external Kbuild to get
> Module.symvers to be generated properly (with the empty tab at the
> end) for this new change?
> Or does the parser need to be a bit more flexible?
>
One extra clue on this: I noticed the external module Makefile had
KBUILD_EXTRA_SYMBOLS="$(EXTRA_SYMBOLS)" in the $(MAKE) string, where
EXTRA_SYMBOLS pointed to some files that no longer exist. I removed
the KBUILD_EXTRA_SYMBOLS= argument, and magically, the generated
Module.symvers now had tabs at the end of each line.
I wonder if there some path in the KBUILD_EXTRA_SYMBOLS= handling that
isn't generating the output in the same way?
thanks
-john
