x86: Require full ISA support for x86-64 level marker [BZ #27318]

Message ID 20210202215112.1002416-1-hjl.tools@gmail.com
State Superseded
Headers
Series x86: Require full ISA support for x86-64 level marker [BZ #27318] |

Commit Message

H.J. Lu Feb. 2, 2021, 9:51 p.m. UTC
  Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
marker is set in libc.so.  We couldn't set the needed ISA marker to v2
since this libc won't run on all v2 machines.  Technically, the v3 marker
is correct.  But the resulting libc.so won't run on Sandy Brigde, which
is a v2 machine, even when libc is compiled with -march=sandybridge:

$ ./elf/ld.so ./libc.so
./libc.so: (p) CPU ISA level is lower than required: needed: 7; got: 3

Instead, we should require full ISA support for x86-64 level marker to
detect such case:

In file included from ../sysdeps/x86/abi-note.c:28:
../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
   62 | #   error "Invalid ISAs for x86-64 ISA level v3"
      |     ^~~~~
---
 sysdeps/x86/isa-level.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
  

Comments

Joseph Myers Feb. 2, 2021, 11:11 p.m. UTC | #1
On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:

> Instead, we should require full ISA support for x86-64 level marker to
> detect such case:
> 
> In file included from ../sysdeps/x86/abi-note.c:28:
> ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
>    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
>       |     ^~~~~

When does this error occur (what conditions for compilation / 
configuration of glibc)?

It's definitely valid to build glibc with a compiler defaulting to any 
-march option valid for x86_64, including those between two ISA levels 
such as -march=sandybridge; that must not produce an error with default 
configure options, and must produce a glibc binary that does in fact 
execute correctly on the processor corresponding to the compiler default.

It's OK for compiling glibc with -march=sandybridge to produce a glibc 
binary that does not contain ISA level markers, or contains ISA level 
markers that permit execution on some processors that don't have all the 
instructions used in the binary.  It's not OK for it to fail to compile 
glibc, or to produce a binary that doesn't execute on Sandy Bridge.
  
H.J. Lu Feb. 2, 2021, 11:16 p.m. UTC | #2
On Tue, Feb 2, 2021 at 3:11 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:
>
> > Instead, we should require full ISA support for x86-64 level marker to
> > detect such case:
> >
> > In file included from ../sysdeps/x86/abi-note.c:28:
> > ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
> >    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
> >       |     ^~~~~
>
> When does this error occur (what conditions for compilation /
> configuration of glibc)?

It happens at compile time when glibc is built with "-march=sandybridge".

> It's definitely valid to build glibc with a compiler defaulting to any
> -march option valid for x86_64, including those between two ISA levels
> such as -march=sandybridge; that must not produce an error with default
> configure options, and must produce a glibc binary that does in fact
> execute correctly on the processor corresponding to the compiler default.
>
> It's OK for compiling glibc with -march=sandybridge to produce a glibc
> binary that does not contain ISA level markers, or contains ISA level
> markers that permit execution on some processors that don't have all the
> instructions used in the binary.  It's not OK for it to fail to compile
> glibc, or to produce a binary that doesn't execute on Sandy Bridge.
>

We can add a configure option, --disable-isa-level, to unset
INCLUDE_X86_ISA_LEVEL.  The resulting libc.so doesn't have a marker
and won't run on all machines.
  
Florian Weimer Feb. 3, 2021, 9:29 a.m. UTC | #3
* H. J. Lu via Libc-alpha:

> Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> marker is set in libc.so.  We couldn't set the needed ISA marker to v2
> since this libc won't run on all v2 machines.  Technically, the v3 marker
> is correct.

It's not correct due to the way the check is implemented: failing to
load -march=sandybridge binaries on Sandybridge CPUs is clearly wrong.

> But the resulting libc.so won't run on Sandy Brigde, which
> is a v2 machine, even when libc is compiled with -march=sandybridge:
>
> $ ./elf/ld.so ./libc.so
> ./libc.so: (p) CPU ISA level is lower than required: needed: 7; got: 3

They would run were it not for the check.

> Instead, we should require full ISA support for x86-64 level marker to
> detect such case:
>
> In file included from ../sysdeps/x86/abi-note.c:28:
> ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
>    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
>       |     ^~~~~

If you want checks, doing them against an incorrect ABI level that can
never fully match the host CPU is wrong.

You could do something like this:

#ifdef __SSE3___
    if (!CPU_FEATURE_USABLE_P (cpu_features, SSE3))
      _dl_fatal_printf ("Fatal glibc error: CPU does not support SSE3\n");
#endif

And repeat that for all those GCC target macros.

Thanks,
Florian
  
Joseph Myers Feb. 3, 2021, 2:14 p.m. UTC | #4
On Tue, 2 Feb 2021, H.J. Lu wrote:

> On Tue, Feb 2, 2021 at 3:11 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:
> >
> > > Instead, we should require full ISA support for x86-64 level marker to
> > > detect such case:
> > >
> > > In file included from ../sysdeps/x86/abi-note.c:28:
> > > ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
> > >    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
> > >       |     ^~~~~
> >
> > When does this error occur (what conditions for compilation /
> > configuration of glibc)?
> 
> It happens at compile time when glibc is built with "-march=sandybridge".

That's bad.  Since glibc supports execution on Sandy Bridge processors, 
compilation with -march=sandybridge should (a) work, with no special 
configure options needed and (b) produce a glibc that works on Sandy 
Bridge, with no special configure options needed.  I understand that bug 
27318 is reporting that (b) fails at present.  We need to fix (b) without 
breaking (a).

This is not specific at all to x86_64.  It applies to all architectures 
and processors supported by glibc: compiling with a compiler that defaults 
to any such processor should just work, regardless of how that processor 
relates to particular ISA levels in the glibc-hwcaps machinery.

> We can add a configure option, --disable-isa-level, to unset
> INCLUDE_X86_ISA_LEVEL.  The resulting libc.so doesn't have a marker
> and won't run on all machines.

No special configure option should be needed for (a) and (b) to hold.  
They are general principles for any processor supported by glibc, for any 
architecture.
  

Patch

diff --git a/sysdeps/x86/isa-level.c b/sysdeps/x86/isa-level.c
index aaf524cb56..cbd3526406 100644
--- a/sysdeps/x86/isa-level.c
+++ b/sysdeps/x86/isa-level.c
@@ -31,6 +31,9 @@ 
 #ifdef INCLUDE_X86_ISA_LEVEL
 # if defined __x86_64__ || defined __FXSR__ || !defined _SOFT_FLOAT \
      || defined  __MMX__ || defined __SSE__ || defined __SSE2__
+#  if !defined __SSE__ || !defined __SSE2__
+#   error "Invalid ISAs for x86-64 ISA level baseline"
+#  endif
 #  define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
 # else
 #  define ISA_BASELINE	0
@@ -40,6 +43,12 @@ 
      || (defined __x86_64__ && defined __LAHF_SAHF__) \
      || defined __POPCNT__ || defined __SSE3__ \
      || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
+#  if !defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+     || (defined __x86_64__ && !defined __LAHF_SAHF__) \
+     || !defined __POPCNT__ || !defined __SSE3__ \
+     || !defined __SSSE3__ || !defined __SSE4_1__ || !defined __SSE4_2__
+#   error "Invalid ISAs for x86-64 ISA level v2"
+#  endif
 #  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
 # else
 #  define ISA_V2	0
@@ -48,6 +57,10 @@ 
 # if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
      || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__ \
      || defined __XSAVE__
+# if !defined __AVX__ || !defined __AVX2__ || !defined __F16C__ \
+     || !defined __FMA__ || !defined __LZCNT__ || !defined __MOVBE__
+#   error "Invalid ISAs for x86-64 ISA level v3"
+#  endif
 #  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
 # else
 #  define ISA_V3	0
@@ -55,6 +68,11 @@ 
 
 # if defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
      || defined __AVX512DQ__ || defined __AVX512VL__
+#  if !defined __AVX512F__ || !defined __AVX512BW__ \
+      || !defined __AVX512CD__ || !defined __AVX512DQ__ \
+      || !defined __AVX512VL__
+#   error "Invalid ISAs for x86-64 ISA level v4"
+#  endif
 #  define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
 # else
 #  define ISA_V4	0