fuzz: add missing overrides to signature_checker #19548

These functions in fuzz/signature_checker.cpp override virtual member functions and should be marked override instead of virtual, which is for introducing a new virtual function. The overridden virtual functions are in script/interpreter.h:151/156/161.

Also, per MarcoFalke suggestion, add missing parentheses in fuzz/scriptnum_ops.cpp and remove useless unsigned int >= 0 conditional in fuzz/script.cpp.

These changes fix 5 compile warnings in gcc 10 and 3 in clang 11/12.

(clang compiler warnings)

test/fuzz/signature_checker.cpp:31:18: warning: 'CheckSig' overrides a member function but is not marked 'override' [-Wsuggest-override]
    virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
                 ^
./script/interpreter.h:151:18: note: overridden virtual function is here

test/fuzz/signature_checker.cpp:36:18: warning: 'CheckLockTime' overrides a member function but is not marked 'override' [-Wsuggest-override]
    virtual bool CheckLockTime(const CScriptNum& nLockTime) const
                 ^
./script/interpreter.h:156:18: note: overridden virtual function is here

test/fuzz/signature_checker.cpp:41:18: warning: 'CheckSequence' overrides a member function but is not marked 'override' [-Wsuggest-override]
    virtual bool CheckSequence(const CScriptNum& nSequence) const
                 ^
./script/interpreter.h:161:18: note: overridden virtual function is here

3 warnings generated.

(clang compiler warnings)

Which version of clang with which flags does generate those warnings? (no warnings with clang 10.0 for me)

@hebasto ./autogen.sh ; ./configure --enable-c++17 --enable-fuzz --with-sanitizers=address,fuzzer,undefined CC=clang-11 CXX=clang++-11 && make clean

ACK 4064799c9916359e7566b0a6e93a7060df321a10

Thanks! :)

ACK 4064799c9

I think -Wsuggest-override is only introduced in Clang 12: https://clang.llvm.org/docs/DiagnosticsReference.html#wsuggest-override

(and is available in GCC since some.old.version)

Tested with gcc version 10 (./configure --enable-fuzz). When fixing the warnings, it would be good to fix all gcc 10 warnings:

In file included from /usr/include/c++/10/cassert:44,
                 from test/fuzz/scriptnum_ops.cpp:10:
test/fuzz/scriptnum_ops.cpp: In function ‘void test_one_input(const std::vector<unsigned char>&)’:
test/fuzz/scriptnum_ops.cpp:36:52: warning: suggest parentheses around comparison in operand of ‘!=’ [-Wparentheses]
   36 |             assert((script_num <= i) != script_num > i);
      |                                         ~~~~~~~~~~~^~~

In file included from /usr/include/c++/10/cassert:44,
                 from test/fuzz/script.cpp:26:
test/fuzz/script.cpp: In function ‘void test_one_input(const std::vector<unsigned char>&)’:
test/fuzz/script.cpp:51:21: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
   51 |         assert(size >= 0 && size <= 5);
      |                ~~~~~^~~~

test/fuzz/signature_checker.cpp:31:18: warning: ‘virtual bool {anonymous}::FuzzedSignatureChecker::CheckSig(const std::vector<unsigned char>&, const std::vector<unsigned char>&, const CScript&, SigVersion) const’ can be marked override [-Wsuggest-override]
   31 |     virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
      |                  ^~~~~~~~
test/fuzz/signature_checker.cpp:36:18: warning: ‘virtual bool {anonymous}::FuzzedSignatureChecker::CheckLockTime(const CScriptNum&) const’ can be marked override [-Wsuggest-override]
   36 |     virtual bool CheckLockTime(const CScriptNum& nLockTime) const
      |                  ^~~~~~~~~~~~~
test/fuzz/signature_checker.cpp:41:18: warning: ‘virtual bool {anonymous}::FuzzedSignatureChecker::CheckSequence(const CScriptNum&) const’ can be marked override [-Wsuggest-override]
   41 |     virtual bool CheckSequence(const CScriptNum& nSequence) const
      |                  ^~~~~~~~~~~~~

@MarcoFalke reproduced locally with gcc 10.1, done

review ACK c0f09c2c9deaec4cfb35ea587363e6301dd17b88

ACK c0f09c2

Would it be possible to have CI build-only jobs that use

• gcc10 + ./configure --enable-werror
• gcc10 + ./configure --enable-werror --enable-fuzz

gcc10 + ./configure --enable-werror --enable-fuzz @vasild Seems wasteful to have a separate build for just stylistic warnings. And #19388 will also cause the fuzz tests to be built by gcc, so I'd rather wait for that.

gcc10 + ./configure --enable-werror

ACK on adding --enable-werror to one gcc (or just all) of the ci builds