Bug 31883 - ISA level support configure check relies on bashism / is otherwise broken for arithmetic
Summary: ISA level support configure check relies on bashism / is otherwise broken for...
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: build (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.40
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-11 23:28 UTC by Sam James
Modified: 2024-06-15 10:15 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James 2024-06-11 23:28:06 UTC
The fix for PR31867 revealed something kind of nasty.

With /bin/sh = dash (it's not the default in Gentoo though), we get:
```
checking for LAHF/SAHF instruction support... yes
checking for MOVBE instruction support... yes
checking for ISA level support... /var/tmp/portage/sys-libs/glibc-2.39-r8/work/glibc-2.39/configure: 1: eval: Syntax error: "(" unexpected
 * ERROR: sys-libs/glibc-2.39-r8::gentoo failed (configure phase):
 *   failed to configure glibc
 * 
```

Probing, we see with some debugging added:
```
$ ../glibc-2.39/configure --disable-sanity-checks CC="gcc -m32 -march=i486"
+ + gcc -m32 -g -O2 -I../glibc-2.39 -E conftest.c
grep libc_cv_have_x86_isa_level
+ eval libc_cv_have_x86_isa_level=(1 + 0 + 0 + 0)
./glibc-2.39/configure: 1: eval: Syntax error: "(" unexpected
```

With bash, it's evaluated as:
```
+++ grep libc_cv_have_x86_isa_level
++ eval 'libc_cv_have_x86_isa_level=(0' + 0 + 0 + '0)'
+++ libc_cv_have_x86_isa_level=(0 + 0 + 0 + 0)
```

The check was always relying on a Bashism that kind of worked by accident. Shell arrays aren't in POSIX.

Sorry for not spotting it before. The check (both before and after the recent change) seems to work fine with bash, albeit fragile.
Comment 1 Sam James 2024-06-11 23:30:25 UTC
I would just fix it but I'm not really sure what we want to do here.

We can pass it into expr (although expr kind of sucks) or do (( ... )) but then we have to remove the var= prefix before and inject it again after, and then maybe at that point someone has a better idea for doing the check to begin with. Dunno?
Comment 2 Sam James 2024-06-11 23:35:33 UTC
(In reply to Sam James from comment #0)
> 
> Sorry for not spotting it before. The check (both before and after the
> recent change) seems to work fine with bash, albeit fragile.

ionen pointed out on IRC that it doesn't seem to work properly for bash either, as it can't add them up, it just returns the first element which is probably 0 or 1
Comment 3 Sourceware Commits 2024-06-12 21:28:32 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=29807a271edca3e47195bda0c69ae45e245551a9

commit 29807a271edca3e47195bda0c69ae45e245551a9
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Jun 11 20:14:56 2024 -0700

    x86: Properly set x86 minimum ISA level [BZ #31883]
    
    Properly set libc_cv_have_x86_isa_level in shell for MINIMUM_X86_ISA_LEVEL
    defined as
    
    (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4)
    
    Also set __X86_ISA_V2 to 1 for i386 if __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
    is defined.  There are no changes in config.h nor in config.make on x86-64.
    On i386, -march=x86-64-v2 with GCC generates
    
     #define MINIMUM_X86_ISA_LEVEL 2
    
    in config.h and
    
    have-x86-isa-level = 2
    
    in config.make.  This fixes BZ #31883.
    
    Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
    Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
Comment 4 Sam James 2024-06-14 04:08:32 UTC
Fixed, thanks!
Comment 5 Sourceware Commits 2024-06-15 10:15:00 UTC
The release/2.39/master branch has been updated by Sam James <sjames@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f05638731eeaae70c53b4fb30bfc58bc3c52a6d5

commit f05638731eeaae70c53b4fb30bfc58bc3c52a6d5
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Jun 11 20:14:56 2024 -0700

    x86: Properly set x86 minimum ISA level [BZ #31883]
    
    Properly set libc_cv_have_x86_isa_level in shell for MINIMUM_X86_ISA_LEVEL
    defined as
    
    (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4)
    
    Also set __X86_ISA_V2 to 1 for i386 if __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
    is defined.  There are no changes in config.h nor in config.make on x86-64.
    On i386, -march=x86-64-v2 with GCC generates
    
     #define MINIMUM_X86_ISA_LEVEL 2
    
    in config.h and
    
    have-x86-isa-level = 2
    
    in config.make.  This fixes BZ #31883.
    
    Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
    Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
    (cherry picked from commit 29807a271edca3e47195bda0c69ae45e245551a9)