Skip to content
New issue

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

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

Already on GitHub? # to your account

PEP 571: Update GCC_4.5.0 #1180

Merged
merged 3 commits into from
Oct 1, 2019
Merged

PEP 571: Update GCC_4.5.0 #1180

merged 3 commits into from
Oct 1, 2019

Conversation

veblush
Copy link
Contributor

@veblush veblush commented Sep 27, 2019

manylinux2010 is based on CentOS 6 which is bundled with GCC 4.4 and corresponding libgcc_s.so. Current PEP says that its maximum version is GCC_4.3.0 but this should be updated to GCC_4.4.0 because 32-bit CentOS 6 has many symbols with GCC_4.4.0 but 64-bit CentOS 6 doesn't. It seems that this constraints was made only with 64-bit CentOS 6.

From libgcc-glibc.ver, they put new symbols introduced with GCC 4.3 in GCC_4.4.0 only for 32 bits. As a result, those symbols are placed in different version tag depending on architecture.

Comment in the source says:

# 128 bit long double support was introduced with GCC 4.3.0 to 64bit
# and with GCC 4.4.0 to 32bit.  These lines make the symbols to get
# a @@GCC_4.3.0 or @@GCC_4.4.0 attached.

Without this change, application built on 32-bit CentOS 6 and linked with following functions cannot be manylinux2010-compliant even though they are.

GCC_4.4.0 {
  __addtf3
  __copysigntf3
  __divtc3
  __divtf3
  __eqtf2
  __extenddftf2
  __extendsftf2
  __fabstf2
  __fixtfdi
  __fixtfsi
  __fixunstfdi
  __fixunstfsi
  __floatditf
  __floatsitf
  __floatunditf
  __floatunsitf
  __getf2
  __gttf2
  __letf2
  __lttf2
  __multc3
  __multf3
  __negtf2
  __netf2
  __powitf2
  __subtf3
  __trunctfdf2
  __trunctfsf2
  __trunctfxf2
  __unordtf2
}

Context: pypa/auditwheel#141 (comment)

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@veblush

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@ncoghlan
Copy link
Contributor

This seems like a sensible change, but with the PEP having been accepted more than a year ago, we should add a short "Summary of Changes" section for this, akin to the one in PEP 440: https://www.python.org/dev/peps/pep-0440/#summary-of-changes-to-pep-440

I also have a technical question: what are the implications (if any) of allowing GCC_4.4.0 symbols on CentOS 6 x86-64 builds?

It isn't necessarily a blocker if there are implications, but the change note in the PEP should either explain those consequences, or else note that there aren't any.

@ncoghlan ncoghlan mentioned this pull request Sep 30, 2019
@mayeut
Copy link
Contributor

mayeut commented Sep 30, 2019

I also have a technical question: what are the implications (if any) of allowing GCC_4.4.0 symbols on CentOS 6 x86-64 builds?

There are no implications on x86_64. There are no symbols with 4.4.0 version (went from 4.3.0 to 4.7.0, c.f. https://github.com/pypa/auditwheel/blob/61edd310244f70ed5972779775bf2ea25f4e0fd8/auditwheel/policy/policy.json#L66 which reminds me I have to update the list of allowed symbols for manylinux2014 to include i686 symbols)

That being said, it's a fair point and I think we might want to manage symbol version per architecture in auditwheel (and check implications - if any - on the respective PEP)

@veblush veblush changed the title PEP 571: Update GCC_4.4.0 PEP 571: Update GCC_4.5.0 Sep 30, 2019
@veblush
Copy link
Contributor Author

veblush commented Sep 30, 2019

I modified the maximum version to GCC_4.5.0 because 32-bit CentOS 6 has one more symbol under it.

This is a list version symbols from libgcc_s.so on i386/centos:6

GCC_3.0
GCC_3.3
GCC_3.3.1
GCC_3.4
GCC_3.4.2
GCC_4.0.0
GCC_4.2.0
GCC_4.3.0
GCC_4.4.0
GCC_4.5.0
GLIBC_2.0

There is only one function, __extendxftf2 under GCC_4.5.0 which was added by this commit. Meanwhile x86_64 has the same function under GCC_4.3.0.

As @mayeut already clarified, x86_64 has no additional symbol from GCC_4_3.0 to GCC_4.5.0 so there should be no implication on x86_64.

This seems like a sensible change, but with the PEP having been accepted more than a year ago, we should add a short "Summary of Changes" section for this, akin to the one in PEP 440:

I added a summary section to the doc. Please review it.

@mayeut
Copy link
Contributor

mayeut commented Sep 30, 2019

@veblush, given the commit mentioned for gcc, I'm not sure why GCC_4.5.0 is included in CentOS 6 (gcc 4.4.7) and I'm tracking this down in the source rpm.

@brettcannon
Copy link
Member

/cc @takluyver as an author of this PEP.

@mayeut
Copy link
Contributor

mayeut commented Sep 30, 2019

ok, seems that RedHat sources for gcc (gcc-4.4.7-20120601, before applying patches from the srpm) differs from official gcc 4.4.7 sources. The GCC_4.5.0 only appears in RH sources.
IMHO, stick to 4.4.0 for this update. @takluyver, any thoughts on this ?

@takluyver
Copy link
Contributor

Thanks for pinging me, but I don't have any specific input. I trust you all to do the necessary legwork to ensure whatever symbols are specified are broadly compatible with 2014-era Linux distros.

Copy link
Contributor

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that explanatory text is exactly the extra published rationale that I thought the change needed.

@ncoghlan ncoghlan merged commit 26d325d into python:master Oct 1, 2019
@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 1, 2019

OK, I've accepted the update with the GCC_4.5.0 change, on the grounds of making the permitted symbol lists consistent between 64-bit and 32-bit systems where manylinux2010 is concerned.

Ubuntu 12.04 shipped with GCC 4.6, Debian 7 (2013) shipped with GCC 4.7, SLES 12 (2014) shipped with GCC 4.8.

This means you have to go back to actual 2010/2011 releases to start running into potential problems with the __extendxftf2 symbol on i686 systems.

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

Successfully merging this pull request may close these issues.

8 participants