Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@bnoordhuis
Copy link
Member

It seems that it is possible with some toolchains for both __GLIBC__
and __UCLIBC__ to be defined, confusing our "do we have execinfo.h?"
logic.

Assume that when __UCLIBC__ is defined, we are dealing with a libc
that does not have execinfo.h.

Fixes: #8233

@bnoordhuis bnoordhuis added the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 28, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Aug 28, 2016

LGTM

@jbergstroem
Copy link
Member

jbergstroem commented Aug 28, 2016

CI: https://ci.nodejs.org/job/node-test-commit/4810/
Additional test against musl: https://ci.nodejs.org/job/node-test-commit-jbergstroem-alpine34/11/ (execinfo doesn't exist on musl either)

@jasnell
Copy link
Member

jasnell commented Aug 30, 2016

LGTM

It seems that it is possible with some toolchains for both `__GLIBC__`
and `__UCLIBC__` to be defined, confusing our "do we have execinfo.h?"
logic.

Assume that when `__UCLIBC__` is defined, we are dealing with a libc
that does not have execinfo.h.

Fixes: nodejs#8233
PR-URL: nodejs#8308
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis bnoordhuis closed this Sep 5, 2016
@bnoordhuis bnoordhuis deleted the fix8233 branch September 5, 2016 08:23
@bnoordhuis bnoordhuis merged commit a290ddf into nodejs:master Sep 5, 2016
@Fishrock123 Fishrock123 mentioned this pull request Sep 6, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
It seems that it is possible with some toolchains for both `__GLIBC__`
and `__UCLIBC__` to be defined, confusing our "do we have execinfo.h?"
logic.

Assume that when `__UCLIBC__` is defined, we are dealing with a libc
that does not have execinfo.h.

Fixes: nodejs#8233
PR-URL: nodejs#8308
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
It seems that it is possible with some toolchains for both `__GLIBC__`
and `__UCLIBC__` to be defined, confusing our "do we have execinfo.h?"
logic.

Assume that when `__UCLIBC__` is defined, we are dealing with a libc
that does not have execinfo.h.

Fixes: #8233
PR-URL: #8308
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

@bnoordhuis backport to v4?

@bnoordhuis
Copy link
Member Author

Yes.

@MylesBorins
Copy link
Contributor

@bnoordhuis src/backtrace_posix.cc does not exist on v4.x. The source also does not appear to be present. Am I missing something?

@bnoordhuis
Copy link
Member Author

Sorry, it depends on #6734 which is tagged but not back-ported yet. I don't know if or when I'll get around to that.

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.