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

C99 inline semantics #86

Closed
rdb opened this issue Dec 20, 2016 · 3 comments
Closed

C99 inline semantics #86

rdb opened this issue Dec 20, 2016 · 3 comments

Comments

@rdb
Copy link
Contributor

rdb commented Dec 20, 2016

Some projects quite reasonably assume that C99 inline semantics are used when compiling in -std=c99 or -std=c11 modes, which are well supported by GCC 4.8.

However, the default for the compiler seems to be -fgnu89-inline, which forces incompatible pre-C99 inline semantics even in C99/C11 mode, which can cause quite some confusing "multiple definition" errors in libraries that rely on the correct semantics.

Unfortunately, adding -fno-gnu89-inline isn't quite enough to fix this, because the libc headers in /usr/include are apparently not compatible with C99 semantics. More recent versions of the headers explicitly force gnu89 inline semantics for these headers by using the __gnu_inline__ compiler attribute, which was introduced around the time that C99 support was.

So, besides adding -fno-gnu89-inline to my CFLAGS, I have to add this monkey patch to my dockerfile to make libraries like OpenAL compile:
find /usr/include/ -type f -exec sed -i 's/\bextern _*inline_*\b/extern __inline __attribute__ ((__gnu_inline__))/g' {} +

Assuming we no longer need to support GCC 4.2, perhaps we could add this command to the manylinux dockerfile? It would still only be half of the solution, since I believe it is still rather odd for GCC to apply gnu89 inline semantics in C99/C11 modes, but the latter is much easier to work around by adding said flag to CFLAGS.

Alternatively, would it be possible to update the libc headers without breaking compatibility?

rdb added a commit to rdb/panda3d-thirdparty that referenced this issue Dec 20, 2016
@njsmith
Copy link
Member

njsmith commented Dec 20, 2016

Well, that sounds ugly all around. I guess patching the include files like you suggest wouldn't hurt anything, so why not? Are there any downsides? (It sounds like you might be saying that it would break GCC 4.2 compatibility? I can't quite tell.)

Alternatively I guess it wouldn't be the end of the world to push this off to be part of your build scripts, and RHEL5 goes EOL at the end of March 2017, so probably we'll all be switching to CentOS 6 + GCC 5.2 soon-ish and that should take care of this issue. (Making the transition will require someone write up a "manylinux2" spec and such -- not too hard, but someone has to volunteer to do it, so the exact timeline is not clear.)

@rdb
Copy link
Contributor Author

rdb commented Dec 20, 2016

I can't really think of a downside. GCC 4.2 and below would produce a warning akin to "__gnu_inline__" attribute directive ignored or some such thing. Maybe some esoteric compiler would complain about the attribute. We could certainly make the hack more elaborate to avoid this but I got the impression that we want people to use the bundled GCC 4.8.

I'm fine keeping the hack in my local dockerfile but I figured that other people are likely to run into this as well.

@njsmith
Copy link
Member

njsmith commented Dec 21, 2016

I can't really think of a downside.

Okay, why not :-). Want to submit a PR?

rdb added a commit to rdb/manylinux that referenced this issue Dec 21, 2016
grzanka pushed a commit to grzanka/manylinux that referenced this issue Feb 8, 2017
grzanka added a commit to grzanka/manylinux that referenced this issue Apr 18, 2017
* update openssl fetch

Committer: Robin Becker <robin@reportlab.com>

* Use $TRAVIS_COMMIT as image tag

* Use TRAVIS_COMMIT in deploy.sh

* Empty commit to trigger update to auditwheel 1.5.0 (pypa#81)

* Patch libc headers to make them compatible with C99 inline semantics

Closes: pypa#86

* Add 3.6.0 to CPython versions to build

Closes pypagh-82

* Update to latest bugfix releases of older Python versions

* FIX missing symlink to pip

Also make sure that calls to rm never ask for interactive
confirmation.

* Use Python 3.6 to install certifi and auditwheel

* Split travis build into 2 parallel jobs

* Use the trusty image

This is hopefully a workaround to use the docker service with a build
matrix:

travis-ci/travis-ci#5448

* Update to Python 3.4.6 and Python 3.5.3

* Update OPENSSL_ROOT to latest release [k] (current specified one [j] does not
exist anymore in the OPENSSL_DOWNLOAD_URL ftp repo.)

* README: python 3.6

* BF: use vault repositories for EOL CentOS 5

Main repositories for CentOS 5 now offline; point yum at
vault.centos.org (via IP to specialize to a particular server).

* Reference specific 5.11 release instead of version 5

Also switched the IP address for the DNS name to avoid any future
surprises. Fixes pypa#103

Credit for this fix goes out to Ryan Walker (@ryandub) and Lars Butler
(@larsbutler).

* Changed build.sh to compile SQLite3 from source and install in /usr/local. (pypa#93)

* Bundle copy of epel-release-5.4.noarch.rpm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants