From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.6245.1682061712409075097 for ; Fri, 21 Apr 2023 00:21:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Jrh5sGH8; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6F32864E48 for ; Fri, 21 Apr 2023 07:21:51 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D4F70C4339E for ; Fri, 21 Apr 2023 07:21:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1682061710; bh=54x44wpH6GfrNoVtC4yOT63SoCBsedstBwv3iTTn/l4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Jrh5sGH8+ShzAcMRK/dhQqKYc7nAG9ABZJEPQkFdef7yHNXApmI+iAXd10OUDBOPz g72dUTfDAotS7ON/oN4a3ljL//w474/Hnhyf6Df34EG8+Y822QHABRj/Nfqzi5/8aA yQJ0ygy4VvKqneukEN62tOAk/qFa+pLTOacEojy1waXz4sL42blHM43hLtd1Q9Gu4J cvc7lGqD0A7zuhzEYzHEUnQFNCP9pGWGGzNnFAE1NJ7ufp6YaKXKHkBUV8tKNoXQDJ I2Fm/w3vEFgIRRz23spxuohbLpAUq/WqBtOOqsypaWj04nDMS8/lZjuAqLdx2aFpC7 tQ25Sp8iUGcHg== Received: by mail-lj1-f178.google.com with SMTP id 38308e7fff4ca-2a8eb8db083so11711351fa.3 for ; Fri, 21 Apr 2023 00:21:50 -0700 (PDT) X-Gm-Message-State: AAQBX9denQKlfU3LR/KIH1XrNVEdseWBlI/lJ8VKS4Alsopz1qacg6Nh +C56ipcQKCzNTxDBBh9O8D/V/iNwM3C0ux7za3g= X-Google-Smtp-Source: AKy350bJqP04M+4uhKuhZyXa/+i6FJRoN/Y3kFr0SV8FwGechzq4dN2kHYWMpJh7GnLukBVdLiIeXBDOGJve6JlHNkY= X-Received: by 2002:a2e:a30c:0:b0:2a7:6e2e:20e0 with SMTP id l12-20020a2ea30c000000b002a76e2e20e0mr317289lje.7.1682061708876; Fri, 21 Apr 2023 00:21:48 -0700 (PDT) MIME-Version: 1.0 References: <20230421044535.4030762-3-kraxel@redhat.com> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 21 Apr 2023 09:21:37 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v5 02/10] MdePkg: don't set visibility to hidden To: Gerd Hoffmann Cc: =?UTF-8?Q?Marvin_H=C3=A4user?= , devel@edk2.groups.io, Yuwei Chen , Oliver Steffen , Bob Feng , Daniel Schaefer , Chao Li , Dongyan Qian , Michael D Kinney , Pawel Polawski , Baoqi Zhang , Leif Lindholm , Rebecca Cran , Zhiguang Liu , Sunil V L , Ard Biesheuvel , Liming Gao Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 21 Apr 2023 at 08:49, Gerd Hoffmann wrote: > > On Fri, Apr 21, 2023 at 06:01:11AM +0000, Marvin H=C3=A4user wrote: > > > > > On 21. Apr 2023, at 06:45, Gerd Hoffmann wrote: > > > > > > =EF=BB=BFNot needed any more on modern toolchains, they are better > > > in not creating a GOT without this trick. > > > > Hi Gerd, > > > > Thanks! Just out of interest, how did you test this and what were the r= esults? > > Patch #1, adding a linker script assert as suggested by ard, then: > > * compile + test on my local workstation (fedora 37, gcc 12). > * run CI > * compile on some older distros: > - rhel-8 (gcc 8) > - ubuntu-18.04 (gcc 7) > I just realized that on x86, GenFw has some code to deal with GOT entries if they are emitted. I'm not sure how often that gets exercised, given our prior use of hidden visibility, but at least the GOT entries should be covered by relocations if they exist. *However*, one thing we are not taking into account is the fact that relaxations are not usually reflected in the relocations emitted by the compiler when using --emit-relocs. So we might end up with occurrences like the below (taken from the Linux kernel but the idea is the same) ffffffff82fa59d5: 4c 8d 0d 24 66 88 ff lea -0x7799dc(%rip),%r9 ffffffff82fa59d8: R_X86_64_REX_GOTPCRELX level4_kernel_pgt-0x4 ffffffff82fa59dc: 49 8d 69 67 lea 0x67(%r9),%rbp ffffffff82fa59e0: 4c 8d 15 19 76 88 ff lea -0x7789e7(%rip),%r10 ffffffff82fa59e3: R_X86_64_REX_GOTPCRELX level3_kernel_pgt-0x4 So here, the GOT loads have been relaxed into LEA instructions, but GenFw will decode the immediate and assume it points to the GOT entry rather than the variable itself, and happily emit a PE relocation for it. So it would be better to ASSERT() on non-empty GOT, and ignore such GOTPCREL relocations instead of attempting to relocate the GOT entries they (used to) refer to.