Skip to content
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

scripts: Add MACHO dylib checks to symbol-check.py #17863

Merged
merged 2 commits into from Jan 22, 2020

Conversation

@fanquake
Copy link
Member

fanquake commented Jan 4, 2020

Based on #17857.

This adds dynamic library checks for MACHO executables to symbol-check.py. The script has been modified to function more like security-check.py. The error output is now also slightly different. i.e:

# Linux x86
bitcoin-cli: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
bitcoin-cli: export of symbol vtable for std::basic_ios<char, std::char_traits<char> > not allowed
bitcoin-cli: NEEDED library libstdc++.so.6 is not allowed
bitcoin-cli: failed IMPORTED_SYMBOLS EXPORTED_SYMBOLS LIBRARY_DEPENDENCIES

# RISCV (skips exported symbols checks)
bitcoin-tx: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
bitcoin-tx: NEEDED library libstdc++.so.6 is not allowed
bitcoin-tx: failed IMPORTED_SYMBOLS LIBRARY_DEPENDENCIES

# macOS
Checking macOS dynamic libraries...
libboost_filesystem.dylib is not in ALLOWED_LIBRARIES!
bitcoind: failed DYNAMIC_LIBRARIES

Compared to v0.19.0.1 the macOS allowed dylibs has been slimmed down somewhat:

 src/qt/bitcoin-qt:
 /usr/lib/libSystem.B.dylib 
-/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration 
 /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit 
 /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation 
 /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices 
 /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit 
 /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices 
 /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation 
-/System/Library/Frameworks/Security.framework/Versions/A/Security 
-/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration 
 /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics 
-/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL 
-/System/Library/Frameworks/AGL.framework/Versions/A/AGL 
 /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon 
 /usr/lib/libc++.1.dylib 
-/System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork 
 /System/Library/Frameworks/CoreText.framework/Versions/A/CoreText 
 /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO 
 /usr/lib/libobjc.A.dylib
The first argument in bin_PROGRAMS (bitcoind) was being silently
dropped and never passed into the check-security.py or check-symbols.py scripts.

This has been the case since the scripts were added to the makefile in
f3d3eaf.

Example of the behavior:

```python
# touch a, touch b, touch c
# python3 args.py < a b c

import sys
if __name__ == '__main__':
    print(sys.argv)
    # ['args.py', 'b', 'c']

    # if you add some lines to "a",
    # you'll see them here..
    for line in sys.stdin:
        print(line)
```
@fanquake fanquake force-pushed the fanquake:add_macos_dylib_checks branch from 63f23a6 to c491368 Jan 4, 2020
@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Jan 5, 2020

Gitian builds

File commit 593f5e2
(master)
commit 486ab1b
(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz d459a67160e0d3b2... ea5bfa09352ccddd...
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz d1cee0bb7645f3fd... 8043a7ce79778249...
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz b44a6ead5f25cced... a65d7f98714e89b7...
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz 733c3789b6618e44... b0fc43eb386fc8da...
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz b69435c1039fd1b6... 493071e2bd99d544...
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz 6559d07f24d3365a... b967fa3911768267...
bitcoin-0.19.99-osx-unsigned.dmg 932ecce4107c3972... aaa57c32f1b334ad...
bitcoin-0.19.99-osx64.tar.gz 9643a190f0171411... e0e3344ab56d6f37...
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz 48408407e1569b74... 299c855e4de33d6d...
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz 4f4beca5efeba401... bc22c9276cb1b11f...
bitcoin-0.19.99-win64-debug.zip 3332f1ff742870ba... 80d907e22325b123...
bitcoin-0.19.99-win64-setup-unsigned.exe 3038f679f63aaace... e3ce39c654bb6766...
bitcoin-0.19.99-win64.zip 36164b6cccc3031e... c576b877ca1947cd...
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz 35f4572db7b0c185... cfc2d75ded34fa63...
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz 7e2a547cc37ef980... c80ebe14e2a1af2e...
bitcoin-0.19.99.tar.gz 3c30932da888c25f... a7f2938e431d0246...
bitcoin-core-linux-0.20-res.yml 2d9d45c467e8164e... 1161dc8a1baf3d94...
bitcoin-core-osx-0.20-res.yml 5ed70c6cfd3b1161... c44e04fae41c3221...
bitcoin-core-win-0.20-res.yml db482b904e4e51e8... 9de825de9a924bc6...
linux-build.log b5193d2b6e9cf133... f70cf3e1cc7ea40f...
osx-build.log 190a3f15b2f45195... 9d7377da796f5c73...
win-build.log 471ed21e9305257d... 30a221d7d9f74808...
bitcoin-core-linux-0.20-res.yml.diff cf9754928fa6b345...
bitcoin-core-osx-0.20-res.yml.diff 63b09fe6d5c51df2...
bitcoin-core-win-0.20-res.yml.diff e34c56a8a24b7c50...
linux-build.log.diff 0ccb6d5a67c8b95a...
osx-build.log.diff 9928b012cf38b48e...
win-build.log.diff 973af9236ee0a38d...
cppfilt = CPPFilt()
ok = True
for sym,version,arch in read_symbols(filename, False):
if arch == 'RISC-V' or sym in IGNORE_EXPORTS:

This comment has been minimized.

Copy link
@laanwj

laanwj Jan 13, 2020

Member

might want to check arch=='RISC-V' at the beginning of this function, instead of for every symbol

edit: oh, read_symbols returns a separate arch per symbol? ok, that's strange, but now I understand this

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jan 22, 2020

ACK c491368

I don't think my code-organizational remark #17863 (review) is enough to hold this up, can always refactor this later.

laanwj added a commit that referenced this pull request Jan 22, 2020
c491368 scripts: add MACHO dylib checking to symbol-check.py (fanquake)
76bf972 scripts: fix check-symbols & check-security argument passing (fanquake)

Pull request description:

  Based on #17857.

  This adds dynamic library checks for MACHO executables to symbol-check.py. The script has been modified to function more like `security-check.py`. The error output is now also slightly different. i.e:
  ```bash
  # Linux x86
  bitcoin-cli: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
  bitcoin-cli: export of symbol vtable for std::basic_ios<char, std::char_traits<char> > not allowed
  bitcoin-cli: NEEDED library libstdc++.so.6 is not allowed
  bitcoin-cli: failed IMPORTED_SYMBOLS EXPORTED_SYMBOLS LIBRARY_DEPENDENCIES

  # RISCV (skips exported symbols checks)
  bitcoin-tx: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
  bitcoin-tx: NEEDED library libstdc++.so.6 is not allowed
  bitcoin-tx: failed IMPORTED_SYMBOLS LIBRARY_DEPENDENCIES

  # macOS
  Checking macOS dynamic libraries...
  libboost_filesystem.dylib is not in ALLOWED_LIBRARIES!
  bitcoind: failed DYNAMIC_LIBRARIES
  ```

  Compared to `v0.19.0.1` the macOS allowed dylibs has been slimmed down somewhat:
  ```diff
   src/qt/bitcoin-qt:
   /usr/lib/libSystem.B.dylib
  -/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration
   /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit
   /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
   /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
   /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
   /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices
   /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  -/System/Library/Frameworks/Security.framework/Versions/A/Security
  -/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
   /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
  -/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL
  -/System/Library/Frameworks/AGL.framework/Versions/A/AGL
   /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon
   /usr/lib/libc++.1.dylib
  -/System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork
   /System/Library/Frameworks/CoreText.framework/Versions/A/CoreText
   /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
   /usr/lib/libobjc.A.dylib
  ```

ACKs for top commit:
  laanwj:
    ACK c491368

Tree-SHA512: f8624e4964e80b3e0d34e8d3cc33f3107938f3ef7a01c07828f09b902b5ea31a53c50f9be03576e1896ed832cf2c399e03a7943a4f537a1e1c705f3804aed979
@laanwj laanwj merged commit c491368 into bitcoin:master Jan 22, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@fanquake fanquake deleted the fanquake:add_macos_dylib_checks branch Jan 23, 2020
sidhujag added a commit to syscoin/syscoin that referenced this pull request Jan 24, 2020
c491368 scripts: add MACHO dylib checking to symbol-check.py (fanquake)
76bf972 scripts: fix check-symbols & check-security argument passing (fanquake)

Pull request description:

  Based on bitcoin#17857.

  This adds dynamic library checks for MACHO executables to symbol-check.py. The script has been modified to function more like `security-check.py`. The error output is now also slightly different. i.e:
  ```bash
  # Linux x86
  bitcoin-cli: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
  bitcoin-cli: export of symbol vtable for std::basic_ios<char, std::char_traits<char> > not allowed
  bitcoin-cli: NEEDED library libstdc++.so.6 is not allowed
  bitcoin-cli: failed IMPORTED_SYMBOLS EXPORTED_SYMBOLS LIBRARY_DEPENDENCIES

  # RISCV (skips exported symbols checks)
  bitcoin-tx: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
  bitcoin-tx: NEEDED library libstdc++.so.6 is not allowed
  bitcoin-tx: failed IMPORTED_SYMBOLS LIBRARY_DEPENDENCIES

  # macOS
  Checking macOS dynamic libraries...
  libboost_filesystem.dylib is not in ALLOWED_LIBRARIES!
  bitcoind: failed DYNAMIC_LIBRARIES
  ```

  Compared to `v0.19.0.1` the macOS allowed dylibs has been slimmed down somewhat:
  ```diff
   src/qt/bitcoin-qt:
   /usr/lib/libSystem.B.dylib
  -/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration
   /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit
   /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
   /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
   /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
   /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices
   /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  -/System/Library/Frameworks/Security.framework/Versions/A/Security
  -/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
   /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
  -/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL
  -/System/Library/Frameworks/AGL.framework/Versions/A/AGL
   /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon
   /usr/lib/libc++.1.dylib
  -/System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork
   /System/Library/Frameworks/CoreText.framework/Versions/A/CoreText
   /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
   /usr/lib/libobjc.A.dylib
  ```

ACKs for top commit:
  laanwj:
    ACK c491368

Tree-SHA512: f8624e4964e80b3e0d34e8d3cc33f3107938f3ef7a01c07828f09b902b5ea31a53c50f9be03576e1896ed832cf2c399e03a7943a4f537a1e1c705f3804aed979
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.