contrib: Add test for ELF symbol-check #20476
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fanquake
reviewed
Nov 30, 2020
Member
fanquake
left a comment
There was a problem hiding this comment.
Concept ACK - thanks for putting this together. Will actually test shortly.
Member
|
I added a test for macOS if you wanted to pull that in as well: fanquake@9e6e59b. |
Contributor
Check both failure cases: - Use a glibc symbol from a version that is too new - Use a symbol from a library that is not in the allowlist And also check a conforming binary. Adding a similar check for Windows PE can be done in a separate PR.
afc20c2 to
ed1bbce
Compare
Member
Author
Thanks ! I didn't mention macOS in the OP because I (mistakenly) thought we only had a symbol check for windows and linux. Good to have a test for that too. I've fixed the comment and cherry-picked that commit. |
Member
Author
|
(close/ropen was to test the IRC bot) |
fanquake
approved these changes
Dec 7, 2020
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 7, 2020
ed1bbce contrib: add MACHO tests to symbol-check tests (fanquake) 5bab08d contrib: Add test for ELF symbol-check (Wladimir J. van der Laan) Pull request description: Check both failure cases: - Use a glibc symbol from a version that is too new - Use a symbol from a library that is not in the allowlist And also check a conforming binary. Adding a similar check for Windows PE can be done in a separate PR. ACKs for top commit: fanquake: ACK ed1bbce Tree-SHA512: fd437612e003922465fe1396efa1fa3a64bd1c7b0a514d2a0a7a0caaaa9fb5cb43e0ed7caec15eb0a3508692c9eb3212d7ba3c7e8180b942dd3e50616ad6e557
fanquake
added a commit
that referenced
this pull request
Dec 10, 2020
ae9b489 contrib: add symbol check test for PE (fanquake) Pull request description: Follow up to #20476. Adds a test for the PE symbol check. One failing case where we link against `-lpdh` and a pass case. ACKs for top commit: laanwj: Code review ACK ae9b489 dongcarl: Code Review ACK ae9b489 Tree-SHA512: 14109d2c7cb98fb445fe1a7f3078e1e88c49fd29583529c53c75bb625f3060d43df0c64542df72272cff81e1b073f74ce6e437ad0e6617ba2bcccacfd1dc8e53
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 10, 2020
ae9b489 contrib: add symbol check test for PE (fanquake) Pull request description: Follow up to bitcoin#20476. Adds a test for the PE symbol check. One failing case where we link against `-lpdh` and a pass case. ACKs for top commit: laanwj: Code review ACK ae9b489 dongcarl: Code Review ACK ae9b489 Tree-SHA512: 14109d2c7cb98fb445fe1a7f3078e1e88c49fd29583529c53c75bb625f3060d43df0c64542df72272cff81e1b073f74ce6e437ad0e6617ba2bcccacfd1dc8e53
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check both failure cases:
And also check a conforming binary.
Adding a similar check for Windows PE can be done in a separate PR.