Skip to content

src/randomenv.cpp: fix build on uclibc #20358

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 19, 2020
Merged

src/randomenv.cpp: fix build on uclibc #20358

merged 1 commit into from
Nov 19, 2020
+3 −3

Conversation

ffontaine
Copy link
Contributor

Check for HAVE_STRONG_GETAUXVAL or HAVE_WEAK_GETAUXVAL before using
getauxval to avoid a build failure on uclibc

Signed-off-by: Fabrice Fontaine fontaine.fabrice@gmail.com

@ffontaine
src/randomenv.cpp: fix build on uclibc
330cb33 
Check for HAVE_STRONG_GETAUXVAL or HAVE_WEAK_GETAUXVAL before using
getauxval to avoid a build failure on uclibc

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
@sipa
Copy link
Member

sipa commented Nov 9, 2020

Concept ACK

@fanquake
Copy link
Member

Concept ACK - I'll test with uClibc.

@practicalswift
Copy link
Contributor

Concept ACK

What a nice first-time contribution @ffontaine! Warm welcome as a contributor and hope to see more contributions in the future!

luke-jr
luke-jr reviewed Nov 13, 2020
View reviewed changes
Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

Won't this fail without HAVE_STRONG_GETAUXVAL, since randomenv doesn't define the weak variant?

@laanwj
Copy link
Member

laanwj commented Nov 18, 2020 •
edited
Loading

The code change looks correct to me in any case.

But I'm surprised, uClibc doesn't have getauxval at all? It's a slight bit worrying to me because, besides random seeding, it's used for important kernel→userspace communication such as detecting what instruction sets are supported on ARM (and RISC-V and … probably many other architectures).

@ffontaine
Copy link
Contributor Author

There is no getauxval on uclibc-ng: https://github.com/wbx-github/uclibc-ng/search?q=getauxval.
If this is a security issue for you, I can disable bitcoin with uclibc-ng toolchains in buildroot.

@laanwj
Copy link
Member

laanwj commented Nov 19, 2020

I don't think it's a security issue. There are many random sources used on Linux, one less will not significantly make things worse I would expect.

@laanwj
Copy link
Member

laanwj commented Nov 19, 2020

Code review ACK 330cb33

@laanwj laanwj merged commit 1cc5e69 into bitcoin:master Nov 19, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 19, 2020
@laanwj
Merge bitcoin#20358: src/randomenv.cpp: fix build on uclibc
df565c9 
330cb33 src/randomenv.cpp: fix build on uclibc (Fabrice Fontaine)

Pull request description:

  Check for HAVE_STRONG_GETAUXVAL or HAVE_WEAK_GETAUXVAL before using
  getauxval to avoid a build failure on uclibc

  Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

ACKs for top commit:
  laanwj:
    Code review ACK 330cb33

Tree-SHA512: 94fbbdb0e859f0220d64b2d04565f575b410327f080125fec7fb74205d0bea0e8133561c83a696033d6dc377871133871b72c1aad19aca61e972ce67e0fdf707
@jonasschnelli
Copy link
Contributor

I think this change has no effect.
The HAVE_STRONG_GETAUXVAL as well as HAVE_WEAK_GETAUXVAL symbols are not defined. They have been introduced via the crc32c subtree and are only used as CRC32 CPPFAGS (see Makefile.crc32c.include).

@jonasschnelli jonasschnelli mentioned this pull request Dec 7, 2020
Merged
@maflcko
Copy link
Member

maflcko commented Dec 8, 2020

removing from "needs backport" for now due to previous comment

laanwj added a commit that referenced this pull request Dec 14, 2020
@laanwj
Merge #20594: Fix getauxval calls in randomenv.cpp
94a9cd2 
836a3dc Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli)
41a413b Define correct symbols for getauxval (Jonas Schnelli)

Pull request description:

  PR #20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`.

  These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.

  PR #20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass).

  This PR defines the two symbols correctly and reduces calls to `getauxval` to linux.

ACKs for top commit:
  laanwj:
    Code review ACK 836a3dc
  jonatack:
    utACK 836a3dc

Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 15, 2020
@laanwj
Merge bitcoin#20594: Fix getauxval calls in randomenv.cpp
64c284d 
836a3dc Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli)
41a413b Define correct symbols for getauxval (Jonas Schnelli)

Pull request description:

  PR bitcoin#20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`.

  These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.

  PR bitcoin#20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass).

  This PR defines the two symbols correctly and reduces calls to `getauxval` to linux.

ACKs for top commit:
  laanwj:
    Code review ACK 836a3dc
  jonatack:
    utACK 836a3dc

Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 17, 2021
@laanwj @PastaPastaPasta
Merge bitcoin#20594: Fix getauxval calls in randomenv.cpp
81f6dd8 
836a3dc Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli)
41a413b Define correct symbols for getauxval (Jonas Schnelli)

Pull request description:

  PR bitcoin#20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`.

  These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.

  PR bitcoin#20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass).

  This PR defines the two symbols correctly and reduces calls to `getauxval` to linux.

ACKs for top commit:
  laanwj:
    Code review ACK 836a3dc
  jonatack:
    utACK 836a3dc

Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
thelazier pushed a commit to thelazier/dash that referenced this pull request Sep 25, 2021
@laanwj @thelazier
Merge bitcoin#20594: Fix getauxval calls in randomenv.cpp
5af09a3 
836a3dc Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli)
41a413b Define correct symbols for getauxval (Jonas Schnelli)

Pull request description:

  PR bitcoin#20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`.

  These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.

  PR bitcoin#20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass).

  This PR defines the two symbols correctly and reduces calls to `getauxval` to linux.

ACKs for top commit:
  laanwj:
    Code review ACK 836a3dc
  jonatack:
    utACK 836a3dc

Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
@hebasto hebasto mentioned this pull request Feb 16, 2023
Closed
@hebasto
Copy link
Member

hebasto commented Feb 16, 2023 •
edited
Loading

Hmm, this patch looks buggy...

My mistake. Sorry for the noise.

@bitcoin bitcoin unlocked this conversation Feb 16, 2023
OlegGirko pushed a commit to OlegGirko/dash that referenced this pull request Apr 5, 2023
@laanwj @PastaPastaPasta
Merge bitcoin#20358: src/randomenv.cpp: fix build on uclibc
a193964 
330cb33 src/randomenv.cpp: fix build on uclibc (Fabrice Fontaine)

Pull request description:

  Check for HAVE_STRONG_GETAUXVAL or HAVE_WEAK_GETAUXVAL before using
  getauxval to avoid a build failure on uclibc

  Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

ACKs for top commit:
  laanwj:
    Code review ACK 330cb33

Tree-SHA512: 94fbbdb0e859f0220d64b2d04565f575b410327f080125fec7fb74205d0bea0e8133561c83a696033d6dc377871133871b72c1aad19aca61e972ce67e0fdf707
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Nov 9, 2023
@laanwj @gades
Merge bitcoin#20594: Fix getauxval calls in randomenv.cpp
db8a4c7 
836a3dc Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli)
41a413b Define correct symbols for getauxval (Jonas Schnelli)

Pull request description:

  PR bitcoin#20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`.

  These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.

  PR bitcoin#20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass).

  This PR defines the two symbols correctly and reduces calls to `getauxval` to linux.

ACKs for top commit:
  laanwj:
    Code review ACK 836a3dc
  jonatack:
    utACK 836a3dc

Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
gades pushed a commit to piratecash/pirate that referenced this pull request Dec 9, 2023
@laanwj @gades
Merge bitcoin#20594: Fix getauxval calls in randomenv.cpp
4325ea4 
836a3dc Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli)
41a413b Define correct symbols for getauxval (Jonas Schnelli)

Pull request description:

  PR bitcoin#20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`.

  These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.

  PR bitcoin#20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass).

  This PR defines the two symbols correctly and reduces calls to `getauxval` to linux.

ACKs for top commit:
  laanwj:
    Code review ACK 836a3dc
  jonatack:
    utACK 836a3dc

Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
@bitcoin bitcoin locked and limited conversation to collaborators Feb 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@luke-jr luke-jr luke-jr left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Utils/log/libs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ffontaine @sipa @fanquake @practicalswift @laanwj @jonasschnelli @maflcko @hebasto @luke-jr