windows: Fix remaining compiler warnings (MSVC) by practicalswift · Pull Request #14151 · bitcoin/bitcoin

practicalswift · 2018-09-05T13:09:56Z

Fix remaining compiler warnings (MSVC).

Before:

$ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715 /nologo
…\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch
…\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size
…\boost\test\tools\old\impl.hpp(107): warning C4805: '==': unsafe mix of type 'const Left' and type 'const Right' in operation
…\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)
…\test\script_tests.cpp(188): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation
…\test\script_tests.cpp(190): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation
…\test\script_tests.cpp(191): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation
$

After:

$ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715;C4805 /nologo
$

src/script/script.cpp

src/test/crypto_tests.cpp

sipsorcery · 2018-09-08T20:26:23Z

tACK e99684c

Prior to this PR:

msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715 /nologo
c:\projects\bitcoin-72c17\src\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch [C:\projects\bitcoin-72c17\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj]
c:\projects\bitcoin-72c17\src\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size [C:\projects\bitcoin-72c17\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\tools\vcpkg\installed\x64-windows-static\include\boost\test\tools\old\impl.hpp(107): warning C4805: '==': unsafe mix of type 'const Left' and type 'const Right' in operation [C:\projects\bitcoin-72c17\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin-72c17\src\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [C:\projects\bitcoin-72c17\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin-72c17\src\test\script_tests.cpp(188): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation [C:\projects\bitcoin-72c17\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin-72c17\src\test\script_tests.cpp(190): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation [C:\projects\bitcoin-72c17\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin-72c17\src\test\script_tests.cpp(191): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation [C:\projects\bitcoin-72c17\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
move build_msvc\%PLATFORM%\%CONFIGURATION%\*.iobj build_msvc\cache\

Subsequently:

msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715;C4805 /nologo
move build_msvc\%PLATFORM%\%CONFIGURATION%\*.iobj build_msvc\cache\

src/test/allocator_tests.cpp

practicalswift · 2018-09-09T19:51:08Z

@sipsorcery @MarcoFalke Please re-review :-)

practicalswift · 2018-09-10T05:27:05Z

@ken2812221 Squashed. Please re-review :-)

appveyor.yml

NicolasDorier · 2018-09-10T13:38:31Z

Adding /warnaserror is a bit too much?

NicolasDorier · 2018-09-10T13:48:27Z

For test, I tried running a dry compile with /warnaserror, it worked fine but two warning show up. (If I recompile a second time, without cleaning, it does not show)

"C:\Sources\bitcoin\build_msvc\bitcoin.sln" (default target) (1) ->
"C:\Sources\bitcoin\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj" (default target) (2) ->
(ClCompile target) ->
  c:\program files (x86)\microsoft visual studio\2017\buildtools\vc\tools\msvc\14.14.26428\include\xutility(3182): error C4996: 'std::equal::_Unchecked_iterators::_Deprecate': Call to 'st
d::equal' with parameters that may be unsafe - this call relies on the caller to check that the passed values are correct. To disable this warning, use -D_SCL_SECURE_NO_WARNINGS. See docu
mentation on how to use Visual C++ 'Checked Iterators' [C:\Sources\bitcoin\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj]


"C:\Sources\bitcoin\build_msvc\bitcoin.sln" (default target) (1) ->
"C:\Sources\bitcoin\build_msvc\testconsensus\testconsensus.vcxproj" (default target) (3) ->
"C:\Sources\bitcoin\build_msvc\libbitcoin_util\libbitcoin_util.vcxproj" (default target) (4) ->
(Lib target) ->
  sync.obj : error LNK4221: This object file does not define any previously undefined public symbols, so it will not be used by any link operation that consumes this library [C:\Sources\b
itcoin\build_msvc\libbitcoin_util\libbitcoin_util.vcxproj]

That said, I don't think this is very important,

tACK 224cc41bb9fd2906d9a61abd0cad9bd77365569b

practicalswift · 2018-09-10T13:55:11Z

@NicolasDorier Personally I'm all for /warnaserror assuming AppVeyor build failures don't block merge (and thus get in the way of the maintainers job). They don't right?

NicolasDorier · 2018-09-10T14:38:43Z

The whole purpose of /warnaserror is to break the build. I tried /warnaserror, and there is the above error for the first build. I don't think it is worth fixing/fixable.

practicalswift · 2018-09-10T15:02:37Z

@NicolasDorier Yes, I know but I thought the project was configured to block merge in case of Travis build failure but not in case of AppVeyor build failure. But I'm probably mistaken :-)

NicolasDorier · 2018-09-10T15:22:03Z

ah yes I think you are right. Well, I think we can ignore /warnaserror for now, except if you find an obvious fix, even if it does not break, I feel uneasy with red failure. :p

sipsorcery · 2018-09-11T20:59:43Z

For a C++ build to exclude specific warnings and set the "treat warnings as error" flag the following node can be added to the common.vcxproj file:

<Project>
  <ItemDefinitionGroup>
	<ClCompile>
	  <DisableSpecificWarnings>4244;4267;4715;4805;</DisableSpecificWarnings>
	  <TreatWarningAsError>true</TreatWarningAsError>
	</ClCompile>
  </ItemDefinitionGroup>
</Project>

When warnings as error is set the script\interpreter.cpp code fails to compile on line 264 and specifically this check:

std::equal(b.begin(), b.end(), pc)

A quick Google found a blog post about Visual C++ range checking safety. To ignore this warning add 4996 to the list of specific warnings and the solution will then compile.

sipsorcery · 2018-09-12T19:01:51Z

tACK f14f4a6.

NicolasDorier · 2019-01-07T14:32:58Z

re-utACK for me

sipsorcery · 2019-01-08T07:20:23Z

tACK bc7aff7 & 1423334.

ken2812221 · 2019-01-13T17:20:45Z

ACK bc7aff79fc0cfb10d25a7dd5928a1cb8c36a063b

Empact · 2019-01-13T18:11:10Z

re-utACK bc7aff7

src/test/crypto_tests.cpp

practicalswift · 2019-01-15T15:57:21Z

@laanwj Added a commit. Please re-review :-)

laanwj · 2019-01-15T16:27:39Z

LGTM, please squash before merge though!
(and please, don't tag me in commit messages, this gets me lots of mails)

practicalswift · 2019-01-15T19:14:11Z

@laanwj Fixed! Sorry for the tag. Please re-review :-)

@NicolasDorier @sipsorcery @ken2812221 @Empact Would you mind re-reviewing?

…rnings from /nowarn to project file.

sipsorcery · 2019-01-15T19:39:05Z

re-utACK b9dafe7.

Personally I would use an integer-suffix instead of the static cast, e.g. 0x0x08000000ul but it's purely a style thing and of no consequence here.

practicalswift · 2019-01-15T19:46:07Z

@sipsorcery I don't think there exists an integer-suffix in C++11 guaranteeing a width of exactly 64 bits on all supported platforms? :-)

sipsorcery · 2019-01-15T21:18:22Z

I don't think there exists an integer-suffix in C++11 guaranteeing a width of exactly 64 bits on all supported platforms? :-)

The C++17 specification states that ul can be represented by unsigned long or unsigned long long so no there is no guarantee of exactly 64 bits. It also indicates that uint64_t, uint32_t etc. are optional.

I've no argument either way.

laanwj · 2019-01-16T12:36:34Z

Personally I would use an integer-suffix instead of the static cast, e.g. 0x0x08000000ul but it's purely a style thing and of no consequence here.

@sipsorcery I don't think there exists an integer-suffix in C++11 guaranteeing a width of exactly 64 bits on all supported platforms? :-)

Yeah, indeed, if only C++ had Rust's u64 suffix it would be easy!

laanwj · 2019-01-16T12:37:09Z

utACK b9dafe7

b9dafe7 Fix remaining compiler warnings (MSVC). Move disabling of specific warnings from /nowarn to project file. (practicalswift) Pull request description: Fix remaining compiler warnings (MSVC). Before: ``` $ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715 /nologo …\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch …\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size …\boost\test\tools\old\impl.hpp(107): warning C4805: '==': unsafe mix of type 'const Left' and type 'const Right' in operation …\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) …\test\script_tests.cpp(188): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation …\test\script_tests.cpp(190): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation …\test\script_tests.cpp(191): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation $ ``` After: ``` $ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715;C4805 /nologo $ ``` Tree-SHA512: 5b30334d3804e869779e77dad75a799e8e5e7eb2e08634cd40035cce140edd623cbb6c8b5806d2158c3df97888d3ea9ff4b8b6a5a83de3fe2cb361e29588c115

…se of uninitialized memory 870f0cd build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (practicalswift) Pull request description: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory. First UBSan, then ASan followed by TSan... and now: yes, the wait is over -- **MSan is finally here!** :) Some historical context: * 2017: Continuous compilation with Clang Thread Safety analysis enabled (#10866, #10923) * 2018: Continuous testing with trapping on signed integer overflows (`-ftrapv`) (#12686) * 2018: Continuous testing of use of locale dependent functions (#13041) * 2018: Continuous testing of format strings (#13705) * 2018: Continuous compilation with MSVC `TreatWarningAsError` (#14151) * 2018: Continuous testing under UndefinedBehaviorSanitizer – UBSan (#14252, #14673, #17006) * 2018: Continuous testing under AddressSanitizer – ASan (#14794, #17205, #17674) * 2018: Continuous testing under ThreadSanitizer – TSan (#14829) * 2019: Continuous testing in an unsigned char environment (`-funsigned-char`) (#15134) * 2019: Continuous compile-time testing of assumptions we're making (#15391) * 2019: Continuous testing of fuzz test cases under Valgrind (#17633, #18159, #18166) * 2020: Finally... MemorySanitizer – MSAN! :) What is the next step? What tools should we add to CI to keep bugs from entering `master`? :) ACKs for top commit: MarcoFalke: ACK 870f0cd Tree-SHA512: 38327c8b75679d97d469fe42e704cacd1217447a5a603701dd8a58ee50b3be2c10248f8d68a479ed081c0c4b254589d3081c9183f991640b06ef689061f75578

b9dafe7 Fix remaining compiler warnings (MSVC). Move disabling of specific warnings from /nowarn to project file. (practicalswift) Pull request description: Fix remaining compiler warnings (MSVC). Before: ``` $ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715 /nologo …\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch …\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size …\boost\test\tools\old\impl.hpp(107): warning C4805: '==': unsafe mix of type 'const Left' and type 'const Right' in operation …\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) …\test\script_tests.cpp(188): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation …\test\script_tests.cpp(190): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation …\test\script_tests.cpp(191): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation $ ``` After: ``` $ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715;C4805 /nologo $ ``` Tree-SHA512: 5b30334d3804e869779e77dad75a799e8e5e7eb2e08634cd40035cce140edd623cbb6c8b5806d2158c3df97888d3ea9ff4b8b6a5a83de3fe2cb361e29588c115

b9dafe7 Fix remaining compiler warnings (MSVC). Move disabling of specific warnings from /nowarn to project file. (practicalswift) Pull request description: Fix remaining compiler warnings (MSVC). Before: ``` $ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715 /nologo …\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch …\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size …\boost\test\tools\old\impl.hpp(107): warning C4805: '==': unsafe mix of type 'const Left' and type 'const Right' in operation …\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) …\test\script_tests.cpp(188): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation …\test\script_tests.cpp(190): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation …\test\script_tests.cpp(191): warning C4805: '==': unsafe mix of type 'int' and type 'bool' in operation $ ``` After: ``` $ msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715;C4805 /nologo $ ``` Tree-SHA512: 5b30334d3804e869779e77dad75a799e8e5e7eb2e08634cd40035cce140edd623cbb6c8b5806d2158c3df97888d3ea9ff4b8b6a5a83de3fe2cb361e29588c115 # Conflicts: # .appveyor.yml # build_msvc/common.vcxproj # src/test/script_tests.cpp

fanquake added the Windows label Sep 5, 2018

practicalswift changed the title ~~Fix remaining compiler warnings (MSVC)~~ windows: Fix remaining compiler warnings (MSVC) Sep 5, 2018

maflcko reviewed Sep 5, 2018

View reviewed changes

src/script/script.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch 3 times, most recently from 383b749 to 699d458 Compare September 5, 2018 13:53

scravy reviewed Sep 6, 2018

View reviewed changes

src/script/script.cpp Outdated Show resolved Hide resolved

scravy reviewed Sep 6, 2018

View reviewed changes

src/test/crypto_tests.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch from 699d458 to e99684c Compare September 6, 2018 12:50

maflcko reviewed Sep 9, 2018

View reviewed changes

src/test/allocator_tests.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch from abf1b0e to 94ce7c9 Compare September 9, 2018 19:34

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch from 94ce7c9 to 224cc41 Compare September 10, 2018 05:27

NicolasDorier reviewed Sep 10, 2018

View reviewed changes

appveyor.yml Outdated Show resolved Hide resolved

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch 3 times, most recently from 4094687 to 224cc41 Compare September 10, 2018 12:51

maflcko added Validation Windows and removed Windows labels Sep 14, 2018

laanwj reviewed Jan 15, 2019

View reviewed changes

src/test/crypto_tests.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch from 514da96 to ed7e26b Compare January 15, 2019 19:13

Fix remaining compiler warnings (MSVC). Move disabling of specific wa…

b9dafe7

…rnings from /nowarn to project file.

practicalswift force-pushed the fix-remaining-compiler-warnings-msvc branch from ed7e26b to b9dafe7 Compare January 15, 2019 19:15

laanwj merged commit b9dafe7 into bitcoin:master Jan 16, 2019

ken2812221 mentioned this pull request Mar 12, 2019

Add Qt programs to msvc build (updated, no code changes) #15529

Merged

hebasto mentioned this pull request Aug 28, 2019

build: AppVeyor MSVC sync.obj linker warning #16736

Closed

practicalswift mentioned this pull request Mar 7, 2020

build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory #18288

Merged

practicalswift deleted the fix-remaining-compiler-warnings-msvc branch April 10, 2021 19:37

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Conversation

practicalswift commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sipsorcery commented Sep 8, 2018

Uh oh!

Uh oh!

practicalswift commented Sep 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Sep 10, 2018

Uh oh!

Uh oh!

NicolasDorier commented Sep 10, 2018

Uh oh!

NicolasDorier commented Sep 10, 2018

Uh oh!

practicalswift commented Sep 10, 2018

Uh oh!

NicolasDorier commented Sep 10, 2018

Uh oh!

practicalswift commented Sep 10, 2018

Uh oh!

NicolasDorier commented Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipsorcery commented Sep 11, 2018

Uh oh!

sipsorcery commented Sep 12, 2018

Uh oh!

NicolasDorier commented Jan 7, 2019

Uh oh!

sipsorcery commented Jan 8, 2019

Uh oh!

ken2812221 commented Jan 13, 2019

Uh oh!

Empact commented Jan 13, 2019

Uh oh!

Uh oh!

practicalswift commented Jan 15, 2019

Uh oh!

laanwj commented Jan 15, 2019

Uh oh!

practicalswift commented Jan 15, 2019

Uh oh!

sipsorcery commented Jan 15, 2019

Uh oh!

practicalswift commented Jan 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipsorcery commented Jan 15, 2019

Uh oh!

laanwj commented Jan 16, 2019

Uh oh!

laanwj commented Jan 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

practicalswift commented Sep 5, 2018 •

edited

Loading

practicalswift commented Sep 9, 2018 •

edited

Loading

NicolasDorier commented Sep 10, 2018 •

edited

Loading

practicalswift commented Jan 15, 2019 •

edited

Loading