Skip to content

Conversation

fanquake
Copy link
Member

@fanquake 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
bitcoin@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 add_macos_dylib_checks branch from 63f23a6 to c491368 Compare January 4, 2020 03:25
@DrahtBot
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:
Copy link
Member

@laanwj laanwj Jan 13, 2020

Choose a reason for hiding this comment

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

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
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
@fanquake fanquake deleted the add_macos_dylib_checks branch January 23, 2020 00:03
sidhujag pushed 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
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants