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

Replace ancient m4/acx_pthread.m4 with m4/ax_pthread.m4 #1333

Merged
merged 1 commit into from Mar 12, 2018

Conversation

Projects
None yet
4 participants
@cgull
Contributor

cgull commented Mar 19, 2016

This is a notional PR illustrating an issue we're having with Mosh on debian-sid; the root cause seems to lie with protobuf (though there are at least two bugs/changes elsewhere in the build chain that also contribute).

Protobuf uses the very old, obsoleted-in-2009 ACX_PTHREAD macro to detect pthread libraries and set variables appropriately. On Debian Sid (and probably nearly any modern Linux distro), pkg-config --libs protobuf returns -lprotobuf -pthread -lpthread. Usage of -lpthread has been deprecated for a long, long time, but protobuf has been getting away with it until now. Something has recently changed in sid's build toolchain or binutils or glibc so that the oddly-placed -lpthread combined with Mosh's aggressive full-RELRO hardening causes a failure in symbol resolution elsewhere in Mosh, causing mosh-server to segfault on an unresolved PLT entry for fork() at runtime. By adopting the newer AX_PTHREAD macro from autoconf, we get a more-correct -lprotobuf -pthread, and mosh is happier.

See mobile-shell/mosh#727 and https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817929 for much more discussion on the issue.

I have no idea what the best path for protobuf's maintainers to fix this is, but this PR should illustrate the problem nicely.

@googlebot

This comment has been minimized.

googlebot commented Mar 19, 2016

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no label Mar 19, 2016

@cgull

This comment has been minimized.

Contributor

cgull commented Mar 19, 2016

I signed the CLA.

@googlebot

This comment has been minimized.

googlebot commented Mar 19, 2016

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Mar 19, 2016

@cgull

This comment has been minimized.

Contributor

cgull commented Nov 14, 2017

Any interest in this issue? It's been dormant for 1.5 years.

@grpc-kokoro

This comment has been minimized.

grpc-kokoro commented Nov 14, 2017

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

1 similar comment
@grpc-kokoro

This comment has been minimized.

grpc-kokoro commented Nov 14, 2017

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@liujisi

This comment has been minimized.

Contributor

liujisi commented Nov 14, 2017

OK to test

@liujisi liujisi added the kokoro:run label Nov 14, 2017

@liujisi liujisi merged commit 4787519 into protocolbuffers:master Mar 12, 2018

5 checks passed

Jenkins Build finished.
Details
Jenkins 32bit Build finished.
Details
cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment