From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) by mx.groups.io with SMTP id smtpd.web12.13831.1653522858491472778 for ; Wed, 25 May 2022 16:54:18 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=BVae2n+5; spf=pass (domain: gmail.com, ip: 209.85.217.46, mailfrom: pedro.falcato@gmail.com) Received: by mail-vs1-f46.google.com with SMTP id h5so15023377vsq.5 for ; Wed, 25 May 2022 16:54:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=BbLHKh3VrU+yPNQfDUHYS6TepzAI4ffYc+fQTERvhI4=; b=BVae2n+5ouFGMkD5mSUGv8qKrWha4uv029LqysfhL09OgUg1pZUxSOMFDZEF3gWatR JRejLIMTevmc3at+exCKEC2+IlUqx3Klnatsimciq9//A6N4Fr3lJgBHy2QDte85kdAf TVFlyyIeW3QJ8xD5Bmg2ldFpnaJ9V1w8nOl+E6cv2DjfmCdpPiKtedcU94uFpc4OTcBH FNKrjfMbcAvHb+XQHGYobEx3h669cvEJNwbNS7k6rnkvoWm2KpbtH6AwkMShPwf0GRdd lUQfsr9JyooVWFn7oBk+M7oVx5YZgJcIx0LYOhzedLWn7qYL9vxj6pjUVawZtOaddMD6 0TwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=BbLHKh3VrU+yPNQfDUHYS6TepzAI4ffYc+fQTERvhI4=; b=B2yOFfNZPe5ilAGHU6sJdh/mJYZJXB+Rq22ryuGg6v0ZagYdicSq1m9hpbwn0P4/ft wY/I3xVe/BL4b2slc9sD09q+mfhAp9j2UnIRUyX08wec4ZC0Bm9H+0ZXO1AaH3JpDJWa K/AysikRUacSowrVewV3+1cPbYnA/XZs6YTVbGiBo/czLYKm+H7xk8sSDLN+3Bp6BnIA hLiG/oNeTlY+AhBZW4Pg9cgLPsUach3uGu2FOPDHNRDUyi8ZGyHDW8BqHAGpj0L//X7g 9zGc/rPUsdD8yF0bmP0yknhL1zY/A38wCTegnA9cfaMc2RzXrp0qH0EkegPAUfb2kbBU K2MA== X-Gm-Message-State: AOAM533jtAe3C88fWuxXUvdlOSilXR3bnyAfur9aLOwp1jOp0isz6Vre 8mwFWjR1kcLy0ZnOXcPZxqQbJkuxgifhMOKpGkGJZ8LBNKGzcA== X-Google-Smtp-Source: ABdhPJwMEoB5PcxdgkCbZ7nvSeUisuD0y7/5A3ttOCIlH6V1SPAw/xLNw0yMgWsqKYQT7r+KaWwzj94XDdxpt9s+cMs= X-Received: by 2002:a67:fe57:0:b0:335:ef50:1b94 with SMTP id m23-20020a67fe57000000b00335ef501b94mr12729736vsr.6.1653522857455; Wed, 25 May 2022 16:54:17 -0700 (PDT) MIME-Version: 1.0 References: <19fda580-50da-302b-4c4e-0457fb28174b@quicinc.com> In-Reply-To: From: "Pedro Falcato" Date: Thu, 26 May 2022 00:54:06 +0100 Message-ID: Subject: Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 (clang 14.0.3) NOOPT - undefined reference to `memcpy' To: edk2-devel-groups-io , "Yao, Jiewen" Cc: Ard Biesheuvel , Rebecca Cran , Ard Biesheuvel , "Justen, Jordan L" , Gerd Hoffmann Content-Type: multipart/alternative; boundary="0000000000008bf6d405dfdeca0e" --0000000000008bf6d405dfdeca0e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Jiewen, I'm definitely talking about the long term. Fixing this by just replacing it with CopyMem is definitely the right call for the stable tag. Best regards, Pedro On Thu, May 26, 2022 at 12:35 AM Yao, Jiewen wrote: > OK. Just want to clarify: > > > > 1. Are you discussing the solution now to resolve this issue, maybe in > this stable tag? > 2. Or are you discussing the long term solution? > > > > For me, I am focusing on #1 how to resolve this issue in this stable tag. > > > > I am not talking long term solution #2 here. I think that need wider > discussion in EDKII. > > > > > > > > *From:* Pedro Falcato > *Sent:* Thursday, May 26, 2022 7:31 AM > *To:* edk2-devel-groups-io ; Yao, Jiewen < > jiewen.yao@intel.com> > *Cc:* Ard Biesheuvel ; Rebecca Cran < > quic_rcran@quicinc.com>; Ard Biesheuvel ; > Justen, Jordan L ; Gerd Hoffmann < > kraxel@redhat.com> > *Subject:* Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 (clang > 14.0.3) NOOPT - undefined reference to `memcpy' > > > > Right. But as I said, GCC and Clang depend on 4 standard C library > functions. This behavior is documented, and has been stable for probably > the past decade or more. These are not intrinsics, but bog standard > functions you can totally redirect to the existing UEFI ones. I talked wi= th > the LLVM people on IRC and they confirmed that that was essentially the > reason why it used a memcpy call. > > It's, in my opinion, OK to fix this particular issue if for example Visua= l > Studio does something fancier or requires actual intrinsics, but having > support for this would probably better solidify future compilation using > GNU-like toolchains. > > > > Slightly offtopic, but I would dare to say that moving towards memcpy() > and others(versus CopyMem(), etc) would benefit EDK2 generated ASM qualit= y > as the compiler has more of an idea of what you want to accomplish, if yo= u > pass -fbuiltin and let it assume your standard C library has the correct > behavior; and indeed, GCC and Clang can, for instance, transform memcpy() > calls into rep movsb's or whatnot based on the CPU microarch it's compili= ng > for, or completely alias them if it sees you're doing it because of point= er > aliasing. > > > > Thanks, > > Pedro > > > > On Wed, May 25, 2022 at 11:58 PM Yao, Jiewen wrote= : > > =E2=80=9CDon=E2=80=99t use structure assignment=E2=80=9D is our best know= n method to avoid > compiler intrinsic in EDKII coding style since the project is created. > > A third part library may include structure assignment, then we have to > link intrinsic. > > > > If you only talk about memcpy, that is easy. But there might be others. > The problem is: We don=E2=80=99t know how many intrinsic should be there.= It is > fully based upon compiler. > > > > Gerd proposed to move to intrinsic from cryptopkg to mdepkg. But it is no= t > done yet. > > > > To me, using MemCopy is much easier to resolve this problem. > > > > Thank you > > Yao Jiewen > > > > *From:* devel@edk2.groups.io *On Behalf Of *Pedro > Falcato > *Sent:* Thursday, May 26, 2022 6:48 AM > *To:* Yao, Jiewen > *Cc:* Ard Biesheuvel ; edk2-devel-groups-io < > devel@edk2.groups.io>; Rebecca Cran ; Ard > Biesheuvel ; Justen, Jordan L < > jordan.l.justen@intel.com>; Gerd Hoffmann > *Subject:* Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 (clang > 14.0.3) NOOPT - undefined reference to `memcpy' > > > > Is there a legitimate reason not to define memcpy? It'd be easier to do s= o > and comply to the compiler's requirements. > > > > On Wed, 25 May 2022, 23:38 Yao, Jiewen, wrote: > > Hi > > Would you please use CopyMem() for the structure assignment? > > VgpuGop->GopModeInfo =3D *GopModeInfo; > > > > That is best known method to resolve memcpy symbol issue. > > > > Thank you > > Yao Jiewen > > > > > > *From:* Pedro Falcato > *Sent:* Thursday, May 26, 2022 1:01 AM > *To:* Ard Biesheuvel > *Cc:* edk2-devel-groups-io ; Rebecca Cran < > quic_rcran@quicinc.com>; Ard Biesheuvel ; Yao, > Jiewen ; Justen, Jordan L ; > Gerd Hoffmann > *Subject:* Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 (clang > 14.0.3) NOOPT - undefined reference to `memcpy' > > > > On Wed, May 25, 2022 at 5:45 PM Ard Biesheuvel wrote: > > On Wed, 25 May 2022 at 18:44, Pedro Falcato > wrote: > > > > > > > > On Wed, May 25, 2022 at 4:50 PM Ard Biesheuvel wrote: > >> > >> On Wed, 25 May 2022 at 17:08, Rebecca Cran > wrote: > >> > > >> > I noticed OvmfPkg/OvmfPkgX64.dsc doesn't build with `-t CLANG38 -b > >> > NOOPT` (with clang version 14.0.2) with the latest edk2 master > >> > (07c0c2eb0a5970db614ebce1060fc79d6904bdfd): > >> > > >> > make: Nothing to be done for 'tbuild'. > >> > /usr/bin/ld: /usr/bin/ld: DWARF error: invalid or unhandled FORM > value: > >> > 0x23 > >> > > /home/bcran/src/upstream/uefi/edk2/Build/OvmfX64/NOOPT_CLANG38/X64/OvmfPk= g/VirtioGpuDxe/VirtioGpu/OUTPUT/VirtioGpuDxe.lib(Gop.obj): > >> > in function `GopSetMode': > >> > Gop.c:(.text.GopSetMode+0x418): undefined reference to `memcpy' > >> > >> Can you dump the object file to see where the memcpy() call is emitted= ? > > > > I think I found the smoking gun: > https://github.com/tianocore/edk2/blob/916f90baa547b3ebef8fa87c530e2f0c8e= 35e1e3/OvmfPkg/VirtioGpuDxe/Gop.c#L512 > > Indeed. We don't support struct assignment in Tianocore code, exactly > for this reason. > > Note: We should think about providing some basic libc functions in base > EDK2 as some are required by the clang/GCC compilers (see > https://gcc.gnu.org/onlinedocs/gcc/Standards.html, grep for memcpy or > freestanding). > > Passing -ffreestanding would also be a good idea as to let the compiler > know it's dealing with a freestanding environment vs a hosted user-space > one. > > > > All the best, > > Pedro > > > > -- > > Pedro Falcato >=20 > > --=20 Pedro Falcato --0000000000008bf6d405dfdeca0e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Jiewen,

I'm definitely t= alking about the long term. Fixing this by just replacing it with CopyMem i= s definitely the right call for the stable tag.

Be= st regards,
Pedro

<= div dir=3D"ltr" class=3D"gmail_attr">On Thu, May 26, 2022 at 12:35 AM Yao, = Jiewen <jiewen.yao@intel.com= > wrote:

OK. Just want to clarify:

=C2=A0

  1. Are you discussing the solution now to resolve this issue, maybe i= n this stable tag?
  2. Or are you discussing the lon= g term solution?

=C2=A0

For me, I am focusing on #1 how to resolve this issu= e in this stable tag.

=C2=A0

I am not talking long term solution #2 here. I think= that need wider discussion in EDKII.

=C2=A0

=C2=A0

=C2=A0

From: Pedro Falcato <pedro.falcato@gmail.com> Sent: Thursday, May 26, 2022 7:31 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Yao, Jiewen <jiewen.yao@intel.com>= ;
Cc: Ard Biesheuvel <ardb@kernel.org>; Rebecca Cran <quic_rcran@quicinc.com>; Ard Bies= heuvel <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com&= gt;; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 = (clang 14.0.3) NOOPT - undefined reference to `memcpy'

=C2=A0

Right. But as I said, GCC and Clang depend on 4 stan= dard C library functions. This behavior is documented, and has been stable = for probably the past decade or more. These are not intrinsics, but bog sta= ndard functions you can totally redirect to the existing UEFI ones. I talked with the LLVM people on IRC and they c= onfirmed that that was essentially the reason why it used a memcpy call.=

It's, in my opinion, OK to fix this particular i= ssue if for example Visual Studio does something fancier or requires actual= intrinsics, but having support for this would probably better solidify fut= ure compilation using GNU-like toolchains.

=C2=A0

Slightly offtopic, but I would dare to say that movi= ng towards memcpy() and others(versus CopyMem(), etc) would benefit EDK2 ge= nerated ASM quality as the compiler has more of an idea of what you want to= accomplish, if you pass -fbuiltin and let it assume your standard C library has the correct behavior; and in= deed, GCC and Clang can, for instance, transform memcpy() calls into rep mo= vsb's or whatnot based on the CPU microarch it's compiling for, or = completely alias them if it sees you're doing it because of pointer aliasing.

=C2=A0

Thanks,

Pedro

=C2=A0

On Wed, May 25, 2022 at 11:58 PM Yao, Jiewen <jiewen.yao@intel.com= > wrote:

=E2=80=9CDon=E2=80=99t use structure assignment=E2= =80=9D is our best known method to avoid compiler intrinsic in EDKII coding= style since the project is created.

A third part library may include structure assignmen= t, then we have to link intrinsic.

=C2=A0

If you only talk about memcpy, that is easy. But the= re might be others. The problem is: We don=E2=80=99t know how many intrinsi= c should be there. It is fully based upon compiler.

=C2=A0

Gerd proposed to move to intrinsic from cryptopkg to= mdepkg. But it is not done yet.

=C2=A0

To me, using MemCopy is much easier to resolve this = problem.

=C2=A0

Thank you

Yao Jiewen

=C2=A0

From: devel@edk2.groups= .io <devel= @edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Thursday, May 26, 2022 6:48 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io <devel@edk2.groups.io>; Rebe= cca Cran <qu= ic_rcran@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@int= el.com>; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 = (clang 14.0.3) NOOPT - undefined reference to `memcpy'

=C2=A0

Is there a legitimate reason not to define memcpy? I= t'd be easier to do so and comply to the compiler's requirements.

=C2=A0

On Wed, 25 May 2022, 23:38 Yao, Jiewen, <jiewen.yao@intel.com= > wrote:

Hi

Would you please use CopyMem() for the structure ass= ignment?

Vgp= uGop->GopModeInfo=C2=A0 =3D *GopModeInfo;

=C2=A0

That is best known method to resolve memcpy symbol i= ssue.

=C2=A0

Tha= nk you

Yao= Jiewen

=C2=A0

=C2=A0

From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Thursday, May 26, 2022 1:01 AM
To: Ard Biesheuvel <ardb@kernel.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Rebecca Cran <quic_rcran@quicinc.com= >; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <= jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; G= erd Hoffmann <kra= xel@redhat.com>
Subject: Re: [edk2-devel] OvmfPkgX64 doesn't build with CLANG38 = (clang 14.0.3) NOOPT - undefined reference to `memcpy'

=C2=A0

On Wed, May 25, 2022 at 5:45 PM Ard Biesheuvel <<= a href=3D"mailto:ardb@kernel.org" target=3D"_blank">ardb@kernel.org>= wrote:

On Wed, 25 May 2022 at 18:44, Pedro Falcato <pedro.falcato@gmai= l.com> wrote:
>
>
>
> On Wed, May 25, 2022 at 4:50 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Wed, 25 May 2022 at 17:08, Rebecca Cran <quic_rcran@quicinc.com> wro= te:
>> >
>> > I noticed OvmfPkg/OvmfPkgX64.dsc doesn't build with `-t C= LANG38 -b
>> > NOOPT` (with clang version 14.0.2) with the latest edk2 maste= r
>> > (07c0c2eb0a5970db614ebce1060fc79d6904bdfd):
>> >
>> > make: Nothing to be done for 'tbuild'.
>> > /usr/bin/ld: /usr/bin/ld: DWARF error: invalid or unhandled F= ORM value:
>> > 0x23
>> > /home/bcran/src/upstream/uefi/edk2/Build/OvmfX64/NOOPT_CLANG3= 8/X64/OvmfPkg/VirtioGpuDxe/VirtioGpu/OUTPUT/VirtioGpuDxe.lib(Gop.obj):
>> > in function `GopSetMode':
>> > Gop.c:(.text.GopSetMode+0x418): undefined reference to `memcp= y'
>>
>> Can you dump the object file to see where the memcpy() call is emi= tted?
>
> I think I found the smoking gun: https://github.com/tianocore/edk2/blob/916f90baa547b3ebef8fa87c530e2f0c8e35= e1e3/OvmfPkg/VirtioGpuDxe/Gop.c#L512

Indeed. We don't support struct assignment in Tianocore code, exactly for this reason.

Note: We should think about providing some basic lib= c functions in base EDK2 as some are required by the clang/GCC compilers (s= ee https://gcc.gnu.org/onlinedocs/gcc/Standards.html, grep for memcpy= or freestanding).

Passing -ffreestanding would also be a good idea as = to let the compiler know it's dealing with a freestanding environment v= s a hosted user-space one.

=C2=A0

All the best,

Pedro



--

Pedro Falcato

=20



--
Pedro Falcato
--0000000000008bf6d405dfdeca0e--