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

[contrib] Support ARM and RISC-V symbol check #13724

Merged
merged 1 commit into from Aug 31, 2018

Conversation

Projects
None yet
6 participants
@ken2812221
Copy link
Member

ken2812221 commented Jul 20, 2018

Solve the TODO in the gitian-descripter

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch from 71548d3 to cf1935b Jul 20, 2018

@fanquake fanquake requested a review from laanwj Jul 21, 2018

@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Jul 22, 2018

Note to reviewers: This pull request conflicts with the following ones:
  • #14091 (Minor style enhacement in documentation by fedsten)
  • #14066 (gitian-linux: Build binaries for 64-bit POWER by luke-jr)
  • #14065 (Symbol checks for ARM and RISC-V by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jul 22, 2018

Thanks for adding this!

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch from cf1935b to 8f62ece Jul 22, 2018

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jul 22, 2018

utACK 8f62ece

(though as it is dependent on #13665, this should only go in after the 0.17 branch-off)

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented Jul 22, 2018

#13665 is not ready to merge since it would export a lot of symbols.

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Jul 22, 2018

Have you run the symbol check after cross compilation? It seems to fail for me:

  CXXLD    test/test_bitcoin_fuzzy
  CXXLD    bitcoind
  CXXLD    bitcoin-cli
  CXXLD    bitcoin-tx
  CXXLD    test/test_bitcoin
  AR       qt/libbitcoinqt.a
  OBJCXXLD qt/bitcoin-qt
make[2]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/src'
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/src'
Making all in doc/man
make[1]: Entering directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/doc/man'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/doc/man'
make[1]: Entering directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf'
make[1]: Nothing to be done for 'all-am'.
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf'
+ make -j9 -C src check-security
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/src'
Checking binary security...
make: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/src'
+ make -j9 -C src check-symbols
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/src'
Checking glibc back compat...
qt/bitcoin-qt: export of symbol _bss_end__ not allowed
qt/bitcoin-qt: export of symbol __bss_end__ not allowed
qt/bitcoin-qt: export of symbol __bss_start__ not allowed
qt/bitcoin-qt: export of symbol __end__ not allowed
make: *** [check-symbols] Error 1
Makefile:10512: recipe for target 'check-symbols' failed
make: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-arm-linux-gnueabihf/src'

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch from 8f62ece to 60976f2 Jul 23, 2018

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented Jul 23, 2018

@MarcoFalke Fixed. Sorry about not testing before I commited. I've tested 85ec33a (master + 60976f2) that can successfully do gitian build.

@bitcoin bitcoin deleted a comment from DrahtBot Jul 23, 2018

@laanwj laanwj added this to the 0.18.0 milestone Aug 2, 2018

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Aug 16, 2018

Needs rebase after #13665

@bitcoin bitcoin deleted a comment from DrahtBot Aug 16, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch from 60976f2 to 6c22009 Aug 16, 2018

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented Aug 16, 2018

Need to skip RISC-V for now, the linker would export so many symbols.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Aug 21, 2018

Gitian builds for commit 4732fa1 (master):

Gitian builds for commit 73a989a (master and this pull):

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Aug 23, 2018

Need to skip RISC-V for now, the linker would export so many symbols.

FWIW checking the imports (against symbol versions) is much more important than checking the exports, as that's what prevents binary incompatibilities.

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch 2 times, most recently from 3a0ea44 to 6d8150d Aug 25, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch from 6d8150d to 26f44c2 Aug 25, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:symbol-check-all branch from 26f44c2 to c516c3a Aug 25, 2018

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Aug 25, 2018

utACK c516c3a

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Aug 25, 2018

Hmm, didn't see this sooner. FWIW, I have another alternative to this we can evaluate.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Aug 31, 2018

I'm going ahead and merging this, @luke-jr please rebase your stuff on top.

@laanwj laanwj merged commit c516c3a into bitcoin:master Aug 31, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Aug 31, 2018

Merge #13724: [contrib] Support ARM and RISC-V symbol check
c516c3a [contrib] Support ARM and RISC-V symbol check (Chun Kuan Lee)

Pull request description:

  Solve the TODO in the gitian-descripter

Tree-SHA512: 8115e2958af3dde43d9d9d05f0b1b1b93b1c2aa513e771a3e4e1342a5d78af2b0e40c0bbb7e9a0d15954897317e6f5a0d80996239af3b376d5ddd527f73428ae

@ken2812221 ken2812221 deleted the ken2812221:symbol-check-all branch Aug 31, 2018

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