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

Do not use ThreadLocal $errno because this symbol is GLIBC_PRIVATE. #7496

Merged

Conversation

felixvf
Copy link
Contributor

@felixvf felixvf commented Mar 2, 2019

No description provided.

…d therefore shall not be used by any other external programs.
@bcardiff bcardiff requested a review from RX14 March 2, 2019 19:31
@ysbaddaden
Copy link
Contributor

Please provide background for this claim, what historical glibc versions we're breaking, ...

@felixvf
Copy link
Contributor Author

felixvf commented Mar 3, 2019

@ysbaddaden, when running this script

#!/bin/sh

echo >demo.cr 'puts "errno=#{Errno.value}."'
crystal build demo.cr
./demo
LD_DEBUG=bindings ./demo 2>&1 | grep -i errno

on a system with glibc-2.26, you may get this output:

errno=0.
      6798:     binding file /lib64/libpthread.so.0 [0] to /lib64/libc.so.6 [0]: normal symbol `errno' [GLIBC_PRIVATE]
      6798:     binding file /lib64/libpthread.so.0 [0] to /lib64/libc.so.6 [0]: normal symbol `__h_errno' [GLIBC_PRIVATE]
      6798:     binding file /lib64/librt.so.1 [0] to /lib64/libc.so.6 [0]: normal symbol `errno' [GLIBC_PRIVATE]
      6798:     binding file ./demo [0] to /lib64/libc.so.6 [0]: normal symbol `errno' [GLIBC_PRIVATE]
errno=25.

The important aspect is binding file ./demo [0] to /lib64/libc.so.6 [0]: normal symbol `errno' [GLIBC_PRIVATE]. It shows that the compild output references the errno symbol. However, the symbol errno is considered GLIBC_PRIVATE. As of https://groups.google.com/forum/#!topic/uk.comp.os.linux/PX3rstGtZRQ, this means that the the symbol errno is only for the purpose of communicating with other parts of the GNU C library. This means that referencing the errno symbol from programs or libraries not being part of the GNU C library is an error.

There are standard packaging tools which enforce that binaries to be packaged do not reference symbols they shall not reference. This means that current Crystal-generated binaries which access Errno (including crystal itself) do not pass this check and therefore cannot be packaged.

This pull request fixes that situation.

Copy link
Contributor

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

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

Thanks for the details. _errno_location seems to have been introduced in glibc 2.2.5 (released in 2002), so we can hopefully use it without breaking anything.

One less ThreadLocal.

Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

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

Thank you @felixvf 👍

@ysbaddaden ysbaddaden merged commit 8198f53 into crystal-lang:master Mar 4, 2019
@ysbaddaden ysbaddaden added this to the 0.28.0 milestone Mar 4, 2019
@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib labels Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants