Skip to content

Resolve native compiler warnings and bitset overflow - #758

Merged
huangminghuang merged 1 commit into
masterfrom
chore/sysio-compiler-warnings
Nov 10, 2026
Merged

huangminghuang merged 1 commit into
masterfrom
chore/sysio-compiler-warnings

Conversation

@huangminghuang

@huangminghuang huangminghuang commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Allow GCC CI to accept unused callback parameters and partial aggregate initialization by adding -Wno-unused-parameter -Wno-missing-field-initializers to both C and C++ flags in .cicd/platforms/gcc.Dockerfile. The remaining -Wall -Wextra diagnostics stay enabled.

Fix bitset storage-size rounding near the maximum size_t bit count and guard empty padding storage. On 64-bit hosts, this overflow is unreachable through fc::raw::unpack, whose packed bit count is 32 bits. Tests cover direct size calculations, empty/partial storage, and shrink/regrow behavior.

Use FC_LOG_AND_DROP_NOEXCEPT in the write-stream destructor to make its existing termination on interprocess allocation failure explicit. Ordinary errors still log/drop, and the general macro still propagates interprocess allocation failures. The exception tests document the existing policy and also pass with the original macro.

Other fixes annotate intentional fallthrough, remove ignored qualifiers and redundant moves, correct the clio format argument and signed comparisons, remove unused constants, replace deprecated timestamp calls, and default equivalent memberwise assignments. Account-query results are constructed in place to avoid an extra optional-key variant move. FILE deleter types, range-loop copies, and comment splicing diagnostics are corrected. The snapshot benchmark command stays on one line in the surrounding line comments. Appbase advances to merged appbase#7.

The PR contains one squashed commit.

Validation (local Linux x86-64; full builds and test suites precede the final comment-only edit):

  • Complete default all builds: Clang 18 Release and GCC 14.2 RelWithDebInfo, with ccache disabled. GCC uses the flags extracted from the updated CI Dockerfile.
  • Clang: full chain (1,726 cases), contract (1,090), FC (755), and plugin (321) suites, plus 11 component CTests.
  • GCC: full FC suite (755 cases) and 156 focused chain cases covering ABI, headers/finality, unapplied transactions, snapshots, WASM serialization, KV storage, and votes.
  • Both exception-policy compatibility cases also passed with the original macro definition (Clang; 8 assertions). clangd checks of the bitset and exception tests completed without errors.
  • The clio invalid-JSON check confirms the supplied input appears in the parse error. The complete PR diff passes the whitespace check against master.
  • Complete current native warning inventory: Clang 0; GCC 2 previously audited diagnostics (Boost.Asio allocation and an existing signal-handler pointer cast). Only the two requested GCC warning classes are newly disabled.
  • Final comment-only edit: GCC 14.2 and Clang 18 preprocessing with RUN_PERF_BENCHMARKS and -Werror=comment succeeds. Preprocessed executable tokens match the previously reviewed source after accounting for source-line metadata. clangd checks the edited snapshot test translation unit with zero errors.

@heifner heifner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a lot of churn for two -Wextra warning classes that only the gcc lane enables, and neither found a bug here. Please shrink it:

  • Add -Wno-unused-parameter -Wno-missing-field-initializers to .cicd/platforms/gcc.Dockerfile. Fine to do in this PR.
  • Drop the [[maybe_unused]] parameter changes.
  • Drop every added {} / explicit empty member initializer, production and tests. They make each new field touch every construction site, and a .x = {} overrides a default later added to x.
  • Drop the clang-format reflow that came with those, and the tab-to-space changes in wasm-jit.
  • Keep the real fixes: bitset calc_num_blocks, [[fallthrough]], ignored const return qualifiers, redundant std::move, the clio format argument, sign comparisons, unused constants, deprecated timestamp calls, FC_LOG_AND_DROP_NOEXCEPT, and the defaulted assignments.

The appbase bump can stay.

Two notes on the description: the bitset overflow is not reachable from fc::raw::unpack (the packed bit count is 32 bits), and the new exception tests also pass with the old FC_LOG_AND_DROP.

@heifner heifner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is what I asked for.

Please squash the three commits into one before merge. I'll approve after the force-push, since it dismisses approvals anyway.

One nit while you are there: in unittests/snapshot_tests.cpp the benchmark instructions now have a /** \code ... \endcode */ block in the middle of a // comment. Putting the command on one line (or dropping the trailing backslash) fixes the multi-line comment warning with less.

Permit unused parameters and partial aggregate initializers in the GCC CI
image. Fix bitset storage rounding at extreme sizes, make the existing
noexcept cleanup allocation policy explicit, and retain targeted fixes for
fallthrough, qualifiers, moves, signed comparisons, formatting, deprecated
accessors, assignments, and account-query construction. Advance appbase to
its merged unused-parameter cleanup.

Add bitset and exception-policy regressions. Keep snapshot benchmark
instructions on one line to avoid comment splicing.

Change-Id: Ia4266943e5016ab11125860089cac3ddc917653f
@huangminghuang
huangminghuang force-pushed the chore/sysio-compiler-warnings branch from a5ae984 to 349ad4b Compare November 10, 2026 18:27
@huangminghuang
huangminghuang merged commit 3cf2567 into master Nov 10, 2026
14 checks passed
heifner added a commit that referenced this pull request Nov 11, 2026
…ys-cpu

Brings in #758, #759, #760 and #761. `sysio.system` is rebuilt with the wire-cdt of #132, so `sysio.system.wasm` carries #761's changes and the new parameter. Every other contract and ABI built with that wire-cdt matches master's byte for byte. `get_cached_descriptor`, which this branch adds to the sys-vm-oc code cache, drops the top-level `const` on its return type, as #758 does for the functions beside it. The deep-mind log, the snapshot compatibility files, the consensus blockchain data and the sys-util snapshot info expectations still pass unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants