Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

["Laszlo Ersek" <lersek@redhat.com>]

Hi Liming,

On 10/09/19 16:44, Gao, Liming wrote:

> The difference between XCODE/CLANG and GCCXX is the linker. Current
> patches are introduced for the different linker. Clang supports most
> usage of GCC compiler. So, CLANG and XCODE uses GCC family. When I
> enable XCODE or CLANG tool chain in the platform, I don't find other
> incompatible behavior with GCC compiler. So, I think it is safe to let
> CLANG inherit GCC option except for these two cases. When you make new
> changes, and verify them with GCC compiler, that's enough. You don't
> need to specially verify them with XCODE or CLANG. In most case, GCC
> compiler pass, XCODE or CLANG can also pass.

I'd like to provide a counter-example for this.

Consider the issue that Tom reported, at

  http://mid.mail-archive.com/8eb55d97-0ba3-c217-a160-c24730b9f036@amd.com
  https://edk2.groups.io/g/devel/message/48762

in point (2).

(Both links point to the same message, just in different archives.)

Let me summarize that problem.

- In BZ#849, an XCODE toolchain bug was reported, and a workaround was
  requested.

- The workaround was commit 2db0ccc2d7fe ("UefiCpuPkg: Update
  CpuExceptionHandlerLib pass XCODE5 tool chain", 2018-01-16).

- The workaround is binary patching (self-modification) in the exception
  handler's assembly code.

- Unfortunately, this code is also linked into SEC modules, which run
  from flash. Therefore, self-modification is not permitted, and the
  workaround is not good enough for SEC. (Nor for PEI modules that run
  from flash.)

Now, let's consider how we could mitigate this issue for *temporarily*,
for all toolchains *except* XCODE5, until the issue is fixed. Clearly,
toolchains other than XCODE5 have no problems with the original assembly
code (i.e., with the 64-bit absolute address relocations). Therefore, we
could consider reverting the commit, and capturing the results of the
revert in a *separate* NASM source file. This source file (that is, the
original, pre-2db0ccc2d7fe assembly code) would apply to all toolchain
families, except the XCODE family.

Let's say the new NASM source files would be called
- X64/ExceptionHandlerAsmGeneric.nasm -- all except XCODE,
- X64/ExceptionHandlerAsmXcode.nasm -- XCODE,

replacing the current file

- X64/ExceptionHandlerAsm.nasm

So, how can we use the new files, in the INF file

  UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf

?

We could attempt,

> [Sources.X64]
>   X64/ExceptionHandlerAsmGeneric.nasm | GCC
>   X64/ExceptionHandlerAsmXcode.nasm   | XCODE
>   X64/ExceptionHandlerAsmGeneric.nasm | INTEL
>   X64/ExceptionHandlerAsmGeneric.nasm | MSFT

Unfortunately, this does not work.

While it solves the issue on GCC, INTEL and MSFT, it breaks on XCODE:
XCODE picks up *both* the GCC line and the XCODE line. And that's
because XCODE inherits GCC, and there is presently no way to express
"real GCC only".

But, at least, this suggests a solution too. In
"BaseTools/Conf/tools_def.template", for *every* toolchain that
specifies FAMILY=GCC, we should spell out an explicit BUILDRULEFAMILY.
Like this:

> @@ -1995,6 +1995,7 @@
>  #
>  ####################################################################################
>  *_GCC48_*_*_FAMILY               = GCC
> +*_GCC48_*_*_BUILDRULEFAMILY      = GENUINE_GCC
>
>  *_GCC48_*_MAKE_PATH                    = DEF(GCC_HOST_PREFIX)make
>  *_GCC48_*_*_DLL                        = ENV(GCC48_DLL)
> @@ -2134,6 +2135,7 @@
>  #
>  ####################################################################################
>  *_GCC49_*_*_FAMILY               = GCC
> +*_GCC49_*_*_BUILDRULEFAMILY      = GENUINE_GCC
>
>  *_GCC49_*_MAKE_PATH                    = DEF(GCC_HOST_PREFIX)make
>  *_GCC49_*_*_DLL                        = ENV(GCC49_DLL)
> @@ -2280,6 +2282,7 @@
>  #
>  ####################################################################################
>  *_GCC5_*_*_FAMILY                = GCC
> +*_GCC5_*_*_BUILDRULEFAMILY       = GENUINE_GCC
>
>  *_GCC5_*_MAKE_PATH               = DEF(GCC_HOST_PREFIX)make
>  *_GCC5_*_*_DLL                   = ENV(GCC5_DLL)
> @@ -2437,6 +2440,7 @@
>  #
>  ####################################################################################
>  *_CLANG35_*_*_FAMILY             = GCC
> +*_CLANG35_*_*_BUILDRULEFAMILY    = CLANG
>
>  *_CLANG35_*_MAKE_PATH            = make
>  *_CLANG35_*_*_DLL                = ENV(CLANG35_DLL)
> @@ -2517,6 +2521,8 @@
>  #
>  ####################################################################################
>  *_CLANG38_*_*_FAMILY                = GCC
> +*_CLANG38_*_*_BUILDRULEFAMILY       = CLANG
> +
>  *_CLANG38_*_MAKE_PATH               = make
>  *_CLANG38_*_*_DLL                   = ENV(CLANG38_DLL)
>  *_CLANG38_*_ASL_PATH                = DEF(UNIX_IASL_BIN)

And then we could write:

> [Sources.X64]
>   X64/ExceptionHandlerAsmGeneric.nasm | GENUINE_GCC
>   X64/ExceptionHandlerAsmGeneric.nasm | CLANG
>   X64/ExceptionHandlerAsmXcode.nasm   | XCODE
>   X64/ExceptionHandlerAsmGeneric.nasm | INTEL
>   X64/ExceptionHandlerAsmGeneric.nasm | MSFT

replacing plain "GCC", which XCODE inherits, with "GENUINE_GCC" and
"CLANG", neither of which XCODE inherits.

Would you accept the above patch, for
"BaseTools/Conf/tools_def.template"?

Thanks,
Laszlo