Re: [PATCH] x86/boot: Fix build with LLVM toolchain

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Tue, Nov 5, 2024 at 3:32 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 05.11.2024 15:55, Frediano Ziglio wrote:
> > This toolchain generates different object and map files.
> > Account for these changes.
>
> At least briefly mentioning what exactly the differences are would be
> quite nice, imo.
>

What about.

Object have 3 additional sections which must be handled by the linker script.
Map file has 7 columns: VMA, LMA, size, alignment, output section,
definition, symbols, for our symbols output section and definition are
empty.

> > --- a/xen/tools/combine_two_binaries.py
> > +++ b/xen/tools/combine_two_binaries.py
> > @@ -67,13 +67,22 @@ if args.exports is not None:
> >
> >  # Parse mapfile, look for ther symbols we want to export.
> >  if args.mapfile is not None:
> > -    symbol_re = re.compile(r'\s{15,}0x([0-9a-f]+)\s+(\S+)\n')
> > +    symbol_re_clang = \
>
> Is "clang" really appropriate to use here? Even without the alternative
> being named "gnu", I'd expect "llvm" to be more to the point, as at
> the linking stage the original language (encoded in "clang") shouldn't
> matter much anymore.
>

I'll change to "llvm".

> > +        
> > re.compile(r'\s+([0-9a-f]+)\s+([0-9a-f]+)\s+([0-9a-f]+)\s+([0-9a-f]+)\s{15,}(\S+)\n')
>
> Just wondering:
> - How stable is their map file format?

No idea, unfortunately, map files are not really standard.

> - Do they not have an option to write GNU-compatible map files?

I try to search, but I could not find such an option.

> - Why do you declare 5 groups, when you're only after the 1st and last,
> which would then allow to ...
>
> > +    symbol_re_gnu = re.compile(r'\s{15,}0x([0-9a-f]+)\s+(\S+)\n')
> >      for line in open(args.mapfile):
> > -        m = symbol_re.match(line)
> > -        if not m or m.group(2) not in exports:
> > +        name = None
> > +        m = symbol_re_clang.match(line)
> > +        if m:
> > +            name = m.group(5)
> > +        else:
> > +            m = symbol_re_gnu.match(line)
> > +            if m:
> > +                name = m.group(2)
>
> ... uniformly use m.group(2) here (outside of the conditional)?
>

It could be done. The initial idea was testing that VMA and LMA fields
should be identical, which gives a bit more certainty about the format
used.
About map file format, both formats have some headers. Would it be
sensible to detect such headers? BTW, probably a mis-detection would
cause symbols not to be detected.

> Jan
>
> > +        if name is None or name not in exports:
> >              continue
> >          addr = int(m.group(1), 16)
> > -        exports[m.group(2)] = addr
> > +        exports[name] = addr
> >  for (name, addr) in exports.items():
> >      if addr is None:
> >          raise Exception("Required export symbols %s not found" % name)
>

Frediano