Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gfortran installation on 32bit userland, 64bit kernel: Configure for the same ABI as gcc #29241

Closed
mkoeppe opened this issue Feb 24, 2020 · 37 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Feb 24, 2020

The i386-minimal targets have build errors compiling gfortran (see for example https://github.com/mkoeppe/sage/runs/463683795) because of ABI mismatch. These tests are running 32-bit distributions on a 64-bit kernel. gfortran tries to build native (64-bit) but that fails because the corresponding architecture files are missing.

CC: @dimpase @sagetrac-tmonteil @vbraun @jhpalmieri @orlitzky

Component: porting

Author: Matthias Koeppe

Branch/Commit: c85c26f

Reviewer: Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/29241

@mkoeppe mkoeppe added this to the sage-9.1 milestone Feb 24, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 3, 2020

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 3, 2020

Commit: 3972311

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 3, 2020

comment:2

Tested on docker-debian-buster-i386-minimal, where the gfortran build now succeeds.
Needs review.


New commits:

749f83etox.ini: Add other archs supported by docker
e941bd5fixup for llocal
2441fe9tox.ini: Restructure docker image:tag construction, add multiarch for Linux Docker
2f057cbtox.ini: Add raspbian
7f122bdbuild/bin/write-dockerfile.sh: src/ext has moved and is no longer needed for the configure phase
3972311gfortran on 32bit: Configure for the same ABI as gcc

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 3, 2020

Author: Matthias Koeppe

@mkoeppe mkoeppe changed the title gfortran on 32bit: Configure for the same ABI as gcc gfortran installation on 32bit userland, 64bit kernel: Configure for the same ABI as gcc Mar 3, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 8, 2020

comment:4

Anyone interested in reviewing this?

@dimpase
Copy link
Member

dimpase commented Mar 8, 2020

comment:5

I don't know how to set up an environment where this change matters.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

7ae21c3Merge tag '9.1.beta7' into t/29241/gfortran_on_32bit__configure_for_the_same_abi_as_gcc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2020

Changed commit from 3972311 to 7ae21c3

@jhpalmieri
Copy link
Member

comment:7

Will the gfortran spkg-install file ever be run on OS X?

$ gcc -print-multiarch
clang: error: unknown argument: '-print-multiarch'
clang: error: no input files

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 9, 2020

comment:8

The equivalent option with clang is gcc -print-target-triple

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 9, 2020

comment:10

Well, on OS X building for i386 no longer works. But I'll add clang support just in case for FreeBSD.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2020

Changed commit from 7ae21c3 to 35711ae

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

35711aeUse gcc -print-target-triple for clang

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 9, 2020

comment:13

Replying to @dimpase:

I don't know how to set up an environment where this change matters.

Tests ran at https://github.com/mkoeppe/sage/runs/493828325

Fixed: ubuntu-bionic-i386-minimal, ubuntu-eoan-i386-minimal, debian-buster-i386-minimal.

Remaining error: centos-7-i386-minimal (https://github.com/mkoeppe/sage/runs/493828336)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2020

Changed commit from 35711ae to 7847ee9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

7847ee9build/pkgs/gfortran/spkg-build: Treat i[4-6]86 like i386

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

dcdb8ceUsee gcc -dumpmachine

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2020

Changed commit from 7847ee9 to dcdb8ce

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 10, 2020

comment:16

Tests at https://github.com/mkoeppe/sage/actions/runs/53297072

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 11, 2020

comment:17

This fixed the remaining platform centos-7-i386-minimal.

Ready for review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c830344Use gcc -dumpmachine

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2020

Changed commit from dcdb8ce to c830344

@orlitzky
Copy link
Contributor

comment:20

I'll have to take your word for it that this fixes the problem. Some implementation nitpicks:

ARCH=`$CC -dumpmachine 2>/dev/null || echo unknown`

The form $(command) should be preferred over backticks here (http://mywiki.wooledge.org/BashFAQ/082).

And if you explicitly want to call GCC here, shouldn't you use "gcc" instead of $CC?

    x86_64,i[3-6]86*)

Personally I would just write [3456] there. Ranges in globs are a bash extension, but more importantly they involve the system locale and collation, leading to rare problems that can be avoided by just listing the numbers you want to match.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:21

Replying to @orlitzky:

I'll have to take your word for it that this fixes the problem.

The logs on the various machines are available at the links posted.

Some implementation nitpicks:

ARCH=`$CC -dumpmachine 2>/dev/null || echo unknown`

The form $(command) should be preferred over backticks here (http://mywiki.wooledge.org/BashFAQ/082).

OK will do.

And if you explicitly want to call GCC here, shouldn't you use "gcc" instead of $CC?

No, this needs to run the specific compiler that is configured!

    x86_64,i[3-6]86*)

Personally I would just write [3456] there. Ranges in globs are a bash extension, but more importantly they involve the system locale and collation, leading to rare problems that can be avoided by just listing the numbers you want to match.

OK will do.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:22

Replying to @mkoeppe:

And if you explicitly want to call GCC here, shouldn't you use "gcc" instead of $CC?

No, this needs to run the specific compiler that is configured!

In particular, this is supposed to work also if CC is configured to be "gcc -m32".

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 16, 2020

Changed commit from c830344 to c85c26f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 16, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

c85c26fbuild/pkgs/gfortran/spkg-build: shell style fixes suggested by mjo

@orlitzky
Copy link
Contributor

comment:24

Replying to @mkoeppe:

In particular, this is supposed to work also if CC is configured to be "gcc -m32".

Ah, ok. I was thinking more along the lines of, what if CC is e.g. clang or icc?

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:25

Replying to @orlitzky:

Replying to @mkoeppe:

In particular, this is supposed to work also if CC is configured to be "gcc -m32".

Ah, ok. I was thinking more along the lines of, what if CC is e.g. clang or icc?

Well, clang supports the -dumpmachine option too.

I have not tested with icc; I don't know if Sage is known to build at all with this compiler.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:26

There's some evidence (JuliaLang/julia#9145) that icc supports this option too on Linux (but possibly not on macOS - which is not a problem because 32-bit builds are no longer supported; and the fallback to unknown should take care of it).

@orlitzky
Copy link
Contributor

comment:27

My concern isn't about the -dumpmachine flag per se. We need to build gfortran with the same ABI as gcc, because gfortran tries to use headers from GCC and they won't be where gfortran expects them to be if the ABIs are mismatched. But if CC=clang, why should we insist that the ABI of clang and gfortran are the same? In that case, you still want to ensure that the ABIs of gfortran/gcc agree, not gfortran/clang.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:28

Simply because we cannot link 32bit & 64bit into one executable

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:29

We need to build gfortran with the same ABI as the C compiler that is used, not some gcc that may be on the system.

@orlitzky
Copy link
Contributor

comment:30

Ok, that makes sense. I was focusing on the bits/libc-header-start.h: No such file or directory errors. I guess we could still get build failures if GCC is 32-bit but someone has built a 64-bit clang and sets CC=clang. These are pretty low return-on-investment scenarios, though. Feel free to set positive review if the CI and build bots are happy.

@orlitzky
Copy link
Contributor

Reviewer: Michael Orlitzky

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2020

comment:31

Replying to @orlitzky:

Ok, that makes sense. I was focusing on the bits/libc-header-start.h: No such file or directory errors. I guess we could still get build failures if GCC is 32-bit but someone has built a 64-bit clang and sets CC=clang.

Right. In this situation, probably we would have to build the full gcc, possibly with bootstrapping.

These are pretty low return-on-investment scenarios, though.

I agree. If this ever becomes a problem, this can be fixed separately.

Feel free to set positive review if the CI and build bots are happy.

Thanks!

@vbraun
Copy link
Member

vbraun commented Mar 18, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants