x86-64: Don't set GLRO(dl_platform) to NULL [BZ #22299]
Commit Message
And while we on it - I don't think that dl_hwcap should be reset either:
sysdeps/x86/cpu-features.c: /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
GLRO(dl_hwcap) = 0;
If kernel provide in the AT_HWCAP/AT_HWCAP2 some info, why destroy it?
Or I missed something?
Valery
From: H.J. Lu <hongjiu.lu@intel.com>
To: GNU C Library <libc-alpha@sourceware.org>
Cc: Valery Reznic <valery_reznic@yahoo.com>
Sent: Tuesday, October 17, 2017 6:44 PM
Subject: [PATCH] x86-64: Don't set GLRO(dl_platform) to NULL [BZ #22299]
Since ld.so expands $PLATFORM with GLRO(dl_platform), don't set
GLRO(dl_platform) to NULL.
OK for master and 2.26 branch?
H.J.
---
2017-10-17 Valery Reznic <valery_reznic@yahoo.com>
H.J. Lu <hongjiu.lu@intel.com>
[BZ #22299]
* sysdeps/x86/cpu-features.c (init_cpu_features): Don't set
GLRO(dl_platform) to NULL.
* sysdeps/x86_64/Makefile (tests): Add tst-platform-1.
(modules-names): Add tst-platformmod-1 and
x86_64/tst-platformmod-2.
(CFLAGS-tst-platform-1.c): New.
(CFLAGS-tst-platformmod-1.c): Likewise.
(CFLAGS-tst-platformmod-2.c): Likewise.
(LDFLAGS-tst-platformmod-2.so): Likewise.
($(objpfx)tst-platform-1): Likewise.
($(objpfx)tst-platform-1.out): Likewise.
(tst-platform-1-ENV): Likewise.
($(objpfx)x86_64/tst-platformmod-2.os): Likewise.
* sysdeps/x86_64/tst-platform-1.c: New file.
* sysdeps/x86_64/tst-platformmod-1.c: Likewise.
* sysdeps/x86_64/tst-platformmod-2.c: Likewise.
---
sysdeps/x86/cpu-features.c | 12 ++++++++----
sysdeps/x86_64/Makefile | 18 ++++++++++++++++++
sysdeps/x86_64/tst-platform-1.c | 29 +++++++++++++++++++++++++++++
sysdeps/x86_64/tst-platformmod-1.c | 23 +++++++++++++++++++++++
sysdeps/x86_64/tst-platformmod-2.c | 23 +++++++++++++++++++++++
5 files changed, 101 insertions(+), 4 deletions(-)
create mode 100644 sysdeps/x86_64/tst-platform-1.c
create mode 100644 sysdeps/x86_64/tst-platformmod-1.c
create mode 100644 sysdeps/x86_64/tst-platformmod-2.c
Comments
On Tue, Oct 17, 2017 at 9:09 AM, Valery Reznic <valery_reznic@yahoo.com> wrote:
> And while we on it - I don't think that dl_hwcap should be reset either:
>
> sysdeps/x86/cpu-features.c:
> /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
> GLRO(dl_hwcap) = 0;
>
> If kernel provide in the AT_HWCAP/AT_HWCAP2 some info, why destroy it?
>
Kernel doesn't provide anything useful for x86-64 and we can get everything
we need from CPUID.
On Tue, Oct 17, 2017 at 11:28 PM, Valery Reznic <valery_reznic@yahoo.com> wrote:
> I always thought that it was kernel responsibility to detect platform/hwcap
> and pass them to the application in the auxv vector.
>
X86 glibc doesn't use them.
I see.
So what about [PATCH] x86-64: Don't set GLRO(dl_platform) to NULL [BZ #22299] - any chance to have it in glibc 2.27?
From: H.J. Lu <hjl.tools@gmail.com>
To: Valery Reznic <valery_reznic@yahoo.com>
Cc: GNU C Library <libc-alpha@sourceware.org>
Sent: Wednesday, October 18, 2017 3:36 PM
Subject: Re: [PATCH] x86-64: Don't set GLRO(dl_platform) to NULL [BZ #22299]
On Tue, Oct 17, 2017 at 11:28 PM, Valery Reznic <valery_reznic@yahoo.com> wrote:
> I always thought that it was kernel responsibility to detect platform/hwcap
> and pass them to the application in the auxv vector.
>
X86 glibc doesn't use them.
@@ -335,7 +335,6 @@ no_cpuid:
#endif
 /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
-Â GLRO(dl_platform) = NULL;
#if !HAVE_TUNABLES && defined SHARED
 /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do
   this. */
@@ -346,13 +345,15 @@ no_cpuid:
 GLRO(dl_hwcap) = HWCAP_X86_64;
 if (cpu_features->kind == arch_kind_intel)
  {
+Â Â Â const char *platform = NULL;
+
   if (CPU_FEATURES_ARCH_P (cpu_features, AVX512F_Usable)
    && CPU_FEATURES_CPU_P (cpu_features, AVX512CD))
   {
    if (CPU_FEATURES_CPU_P (cpu_features, AVX512ER))
     {
      if (CPU_FEATURES_CPU_P (cpu_features, AVX512PF))
-Â Â Â Â Â Â GLRO(dl_platform) = "xeon_phi";
+Â Â Â Â Â Â platform = "xeon_phi";
     }
    else
     {
@@ -363,7 +364,7 @@ no_cpuid:
     }
   }
-Â Â Â if (GLRO(dl_platform) == NULL
+Â Â Â if (platform == NULL
    && CPU_FEATURES_ARCH_P (cpu_features, AVX2_Usable)
    && CPU_FEATURES_ARCH_P (cpu_features, FMA_Usable)
    && CPU_FEATURES_CPU_P (cpu_features, BMI1)
@@ -371,7 +372,10 @@ no_cpuid:
    && CPU_FEATURES_CPU_P (cpu_features, LZCNT)
    && CPU_FEATURES_CPU_P (cpu_features, MOVBE)
    && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
-Â Â Â GLRO(dl_platform) = "haswell";
+Â Â Â platform = "haswell";
+
+Â Â Â if (platform)
+Â Â Â GLRO(dl_platform) = platform;
  }
#else
 GLRO(dl_hwcap) = 0;
@@ -58,6 +58,19 @@ LDFLAGS-tst-x86_64mod-1.so = -Wl,-soname,tst-x86_64mod-1.so
$(objpfx)tst-x86_64-1: $(objpfx)x86_64/tst-x86_64mod-1.so
+tests += tst-platform-1
+modules-names += tst-platformmod-1 x86_64/tst-platformmod-2
+CFLAGS-tst-platform-1.c = -mno-avx
+CFLAGS-tst-platformmod-1.c = -mno-avx
+CFLAGS-tst-platformmod-2.c = -mno-avx
+LDFLAGS-tst-platformmod-2.so = -Wl,-soname,tst-platformmod-2.so
+$(objpfx)tst-platform-1: $(objpfx)tst-platformmod-1.so
+$(objpfx)tst-platform-1.out: $(objpfx)x86_64/tst-platformmod-2.so
+# Turn off AVX512F_Usable and AVX2_Usable so that GLRO(dl_platform) is
+# always set to x86_64.
+tst-platform-1-ENV = LD_PRELOAD=$(objpfx)\$$PLATFORM/tst-platformmod-2.so \
+Â Â Â GLIBC_TUNABLES=glibc.tune.hwcaps=-AVX512F_Usable,-AVX2_Usable
+
tests += tst-audit3 tst-audit4 tst-audit5 tst-audit6 tst-audit7 \
   tst-audit10 tst-sse tst-avx tst-avx512
test-extras += tst-audit4-aux tst-audit10-aux \
@@ -141,3 +154,8 @@ do-tests-clean common-mostlyclean: tst-x86_64-1-clean
.PHONY: tst-x86_64-1-clean
tst-x86_64-1-clean:
   -rm -rf $(objpfx)x86_64
+
+$(objpfx)x86_64/tst-platformmod-2.os: $(objpfx)tst-platformmod-2.os
+Â Â Â $(make-target-directory)
+Â Â Â rm -f $@
+Â Â Â ln $< $@
new file mode 100644
@@ -0,0 +1,29 @@
+/* Test PRELOAD with $PLATFORM.
+Â Copyright (C) 2017 Free Software Foundation, Inc.
+Â This file is part of the GNU C Library.
+
+Â The GNU C Library is free software; you can redistribute it and/or
+Â modify it under the terms of the GNU Lesser General Public
+Â License as published by the Free Software Foundation; either
+Â version 2.1 of the License, or (at your option) any later version.
+
+Â The GNU C Library is distributed in the hope that it will be useful,
+Â but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Â Lesser General Public License for more details.
+
+Â You should have received a copy of the GNU Lesser General Public
+Â License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+
+extern int preload (void);
+
+static int
+do_test (void)
+{
+Â return preload () == 0x1234 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
new file mode 100644
@@ -0,0 +1,23 @@
+/* Test PRELOAD with $PLATFORM.
+Â Copyright (C) 2017 Free Software Foundation, Inc.
+Â This file is part of the GNU C Library.
+
+Â The GNU C Library is free software; you can redistribute it and/or
+Â modify it under the terms of the GNU Lesser General Public
+Â License as published by the Free Software Foundation; either
+Â version 2.1 of the License, or (at your option) any later version.
+
+Â The GNU C Library is distributed in the hope that it will be useful,
+Â but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Â Lesser General Public License for more details.
+
+Â You should have received a copy of the GNU Lesser General Public
+Â License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+int
+preload (void)
+{
+Â return 0;
+}
new file mode 100644
@@ -0,0 +1,23 @@
+/* Test PRELOAD with $PLATFORM.
+Â Copyright (C) 2017 Free Software Foundation, Inc.
+Â This file is part of the GNU C Library.
+
+Â The GNU C Library is free software; you can redistribute it and/or
+Â modify it under the terms of the GNU Lesser General Public
+Â License as published by the Free Software Foundation; either
+Â version 2.1 of the License, or (at your option) any later version.
+
+Â The GNU C Library is distributed in the hope that it will be useful,
+Â but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Â Lesser General Public License for more details.
+
+Â You should have received a copy of the GNU Lesser General Public
+Â License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+int
+preload (void)
+{
+Â return 0x1234;
+}