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

[Jan Beulich <jbeulich@suse.com>]

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.

> --- 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.

> +        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?
- Do they not have an option to write GNU-compatible map files?
- 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)?

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)