| Message ID | 20240718173423.1574395-1-maskray@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/vdso: Remove --hash-style=sysv | expand |
On Thu, 18 Jul 2024 10:34:23 -0700, Fangrui Song wrote: > glibc added support for .gnu.hash in 2006 and .hash has been obsoleted > for more than one decade in many Linux distributions. Using > --hash-style=sysv might imply unaddressed issues and confuse readers. > > Just drop the option and rely on the linker default, which is likely > "both", or "gnu" when the distribution really wants to eliminate sysv > hash overhead. > > [...] Applied to arm64 (for-next/core), thanks! [1/1] arm64/vdso: Remove --hash-style=sysv https://git.kernel.org/arm64/c/48f6430505c0 Cheers,
On Thu, 2024-07-18 at 10:34 -0700, Fangrui Song wrote: > glibc added support for .gnu.hash in 2006 and .hash has been obsoleted > for more than one decade in many Linux distributions. Using > --hash-style=sysv might imply unaddressed issues and confuse readers. > > Just drop the option and rely on the linker default, which is likely > "both", or "gnu" when the distribution really wants to eliminate sysv > hash overhead. > > Similar to commit 6b7e26547fad ("x86/vdso: Emit a GNU hash"). > > Signed-off-by: Fangrui Song <maskray@google.com> Hi Fangrui, If I read tools/testing/selftests/vDSO/parse_vdso.c correctly, it does know DT_GNU_HASH as at now. Thus after this change the vDSO selftests are skipped with "Couldn't find __vdso_gettimeofday" etc if the distro enables --hash-style=gnu by default. So it seems we need to add DT_GNU_HASH support for parse_vdso.c to keep test coverage. > --- > arch/arm64/kernel/vdso/Makefile | 2 +- > arch/arm64/kernel/vdso32/Makefile | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile > index d63930c82839..d11da6461278 100644 > --- a/arch/arm64/kernel/vdso/Makefile > +++ b/arch/arm64/kernel/vdso/Makefile > @@ -21,7 +21,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti > # potential future proofing if we end up with internal calls to the exported > # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so > # preparation in build-time C")). > -ldflags-y := -shared -soname=linux-vdso.so.1 --hash-style=sysv \ > +ldflags-y := -shared -soname=linux-vdso.so.1 \ > -Bsymbolic --build-id=sha1 -n $(btildflags-y) > > ifdef CONFIG_LD_ORPHAN_WARN > diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile > index cc4508c604b2..25a2cb6317f3 100644 > --- a/arch/arm64/kernel/vdso32/Makefile > +++ b/arch/arm64/kernel/vdso32/Makefile > @@ -98,7 +98,7 @@ VDSO_AFLAGS += -D__ASSEMBLY__ > # From arm vDSO Makefile > VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1 > VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096 > -VDSO_LDFLAGS += -shared --hash-style=sysv --build-id=sha1 > +VDSO_LDFLAGS += -shared --build-id=sha1 > VDSO_LDFLAGS += --orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL) > >
On Wed, Aug 14, 2024 at 12:56 PM Xi Ruoyao <xry111@xry111.site> wrote: > > On Thu, 2024-07-18 at 10:34 -0700, Fangrui Song wrote: > > glibc added support for .gnu.hash in 2006 and .hash has been obsoleted > > for more than one decade in many Linux distributions. Using > > --hash-style=sysv might imply unaddressed issues and confuse readers. > > > > Just drop the option and rely on the linker default, which is likely > > "both", or "gnu" when the distribution really wants to eliminate sysv > > hash overhead. > > > > Similar to commit 6b7e26547fad ("x86/vdso: Emit a GNU hash"). > > > > Signed-off-by: Fangrui Song <maskray@google.com> > > Hi Fangrui, > > If I read tools/testing/selftests/vDSO/parse_vdso.c correctly, it does > know DT_GNU_HASH as at now. Thus after this change the vDSO selftests > are skipped with "Couldn't find __vdso_gettimeofday" etc if the distro > enables --hash-style=gnu by default. > > So it seems we need to add DT_GNU_HASH support for parse_vdso.c to keep > test coverage. Hi Xi, Perhaps the selftests file needs DT_GNU_HASH support like https://github.com/abseil/abseil-cpp/commit/1278ee9bd9bd4916181521fac96d6fa1100e38e6 > > --- > > arch/arm64/kernel/vdso/Makefile | 2 +- > > arch/arm64/kernel/vdso32/Makefile | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile > > index d63930c82839..d11da6461278 100644 > > --- a/arch/arm64/kernel/vdso/Makefile > > +++ b/arch/arm64/kernel/vdso/Makefile > > @@ -21,7 +21,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti > > # potential future proofing if we end up with internal calls to the exported > > # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so > > # preparation in build-time C")). > > -ldflags-y := -shared -soname=linux-vdso.so.1 --hash-style=sysv \ > > +ldflags-y := -shared -soname=linux-vdso.so.1 \ > > -Bsymbolic --build-id=sha1 -n $(btildflags-y) > > > > ifdef CONFIG_LD_ORPHAN_WARN > > diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile > > index cc4508c604b2..25a2cb6317f3 100644 > > --- a/arch/arm64/kernel/vdso32/Makefile > > +++ b/arch/arm64/kernel/vdso32/Makefile > > @@ -98,7 +98,7 @@ VDSO_AFLAGS += -D__ASSEMBLY__ > > # From arm vDSO Makefile > > VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1 > > VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096 > > -VDSO_LDFLAGS += -shared --hash-style=sysv --build-id=sha1 > > +VDSO_LDFLAGS += -shared --build-id=sha1 > > VDSO_LDFLAGS += --orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL) > > > > > > -- > Xi Ruoyao <xry111@xry111.site> > School of Aerospace Science and Technology, Xidian University
On Wed, Aug 14, 2024 at 6:23 PM Fangrui Song <maskray@google.com> wrote: > > On Wed, Aug 14, 2024 at 12:56 PM Xi Ruoyao <xry111@xry111.site> wrote: > > > > On Thu, 2024-07-18 at 10:34 -0700, Fangrui Song wrote: > > > glibc added support for .gnu.hash in 2006 and .hash has been obsoleted > > > for more than one decade in many Linux distributions. Using > > > --hash-style=sysv might imply unaddressed issues and confuse readers. > > > > > > Just drop the option and rely on the linker default, which is likely > > > "both", or "gnu" when the distribution really wants to eliminate sysv > > > hash overhead. > > > > > > Similar to commit 6b7e26547fad ("x86/vdso: Emit a GNU hash"). > > > > > > Signed-off-by: Fangrui Song <maskray@google.com> > > > > Hi Fangrui, > > > > If I read tools/testing/selftests/vDSO/parse_vdso.c correctly, it does > > know DT_GNU_HASH as at now. Thus after this change the vDSO selftests > > are skipped with "Couldn't find __vdso_gettimeofday" etc if the distro > > enables --hash-style=gnu by default. > > > > So it seems we need to add DT_GNU_HASH support for parse_vdso.c to keep > > test coverage. > > Hi Xi, > > Perhaps the selftests file needs DT_GNU_HASH support like > https://github.com/abseil/abseil-cpp/commit/1278ee9bd9bd4916181521fac96d6fa1100e38e6 Created https://lore.kernel.org/linux-kselftest/20240815032614.2747224-1-maskray@google.com/T/#u ([PATCH] selftests/vDSO: support DT_GNU_HASH) > > > > --- > > > arch/arm64/kernel/vdso/Makefile | 2 +- > > > arch/arm64/kernel/vdso32/Makefile | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile > > > index d63930c82839..d11da6461278 100644 > > > --- a/arch/arm64/kernel/vdso/Makefile > > > +++ b/arch/arm64/kernel/vdso/Makefile > > > @@ -21,7 +21,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti > > > # potential future proofing if we end up with internal calls to the exported > > > # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so > > > # preparation in build-time C")). > > > -ldflags-y := -shared -soname=linux-vdso.so.1 --hash-style=sysv \ > > > +ldflags-y := -shared -soname=linux-vdso.so.1 \ > > > -Bsymbolic --build-id=sha1 -n $(btildflags-y) > > > > > > ifdef CONFIG_LD_ORPHAN_WARN > > > diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile > > > index cc4508c604b2..25a2cb6317f3 100644 > > > --- a/arch/arm64/kernel/vdso32/Makefile > > > +++ b/arch/arm64/kernel/vdso32/Makefile > > > @@ -98,7 +98,7 @@ VDSO_AFLAGS += -D__ASSEMBLY__ > > > # From arm vDSO Makefile > > > VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1 > > > VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096 > > > -VDSO_LDFLAGS += -shared --hash-style=sysv --build-id=sha1 > > > +VDSO_LDFLAGS += -shared --build-id=sha1 > > > VDSO_LDFLAGS += --orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL) > > > > > > > > > > -- > > Xi Ruoyao <xry111@xry111.site> > > School of Aerospace Science and Technology, Xidian University > > > > -- > 宋方睿
diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile index d63930c82839..d11da6461278 100644 --- a/arch/arm64/kernel/vdso/Makefile +++ b/arch/arm64/kernel/vdso/Makefile @@ -21,7 +21,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti # potential future proofing if we end up with internal calls to the exported # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so # preparation in build-time C")). -ldflags-y := -shared -soname=linux-vdso.so.1 --hash-style=sysv \ +ldflags-y := -shared -soname=linux-vdso.so.1 \ -Bsymbolic --build-id=sha1 -n $(btildflags-y) ifdef CONFIG_LD_ORPHAN_WARN diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index cc4508c604b2..25a2cb6317f3 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -98,7 +98,7 @@ VDSO_AFLAGS += -D__ASSEMBLY__ # From arm vDSO Makefile VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1 VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096 -VDSO_LDFLAGS += -shared --hash-style=sysv --build-id=sha1 +VDSO_LDFLAGS += -shared --build-id=sha1 VDSO_LDFLAGS += --orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL)
glibc added support for .gnu.hash in 2006 and .hash has been obsoleted for more than one decade in many Linux distributions. Using --hash-style=sysv might imply unaddressed issues and confuse readers. Just drop the option and rely on the linker default, which is likely "both", or "gnu" when the distribution really wants to eliminate sysv hash overhead. Similar to commit 6b7e26547fad ("x86/vdso: Emit a GNU hash"). Signed-off-by: Fangrui Song <maskray@google.com> --- arch/arm64/kernel/vdso/Makefile | 2 +- arch/arm64/kernel/vdso32/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)