Add gcc-12 workaround by Bronek · Pull Request #5554 · XRPLF/rippled

Bronek · 2025-07-11T12:18:35Z

High Level Overview of Change

Silence a dummy warning in GCC 12

Context of Change

This warning is breaking build with GCC 12 (but not newer versions of GCC) in release mode only.

Type of Change

Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)

codecov · 2025-07-11T12:49:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.1%. Comparing base (b8626ea) to head (9a30e72).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5554     +/-   ##
=========================================
- Coverage     79.1%   79.1%   -0.0%     
=========================================
  Files          816     816             
  Lines        71632   71632             
  Branches      8240    8240             
=========================================
- Hits         56647   56644      -3     
- Misses       14985   14988      +3

Files with missing lines	Coverage Δ
include/xrpl/beast/utility/rngfill.h	`100.0% <ø> (ø)`

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ximinez · 2025-07-11T15:32:31Z

@Bronek Can you post the text of the warning so I can take a look at it before making any decisions?

Bronek · 2025-07-11T15:45:24Z

@Bronek Can you post the text of the warning so I can take a look at it before making any decisions?

Here's example

In file included from ../src/xrpld/rpc/handlers/Random.cpp:21:
In function 'void beast::rngfill(void*, std::size_t, Generator&) [with Generator = ripple::csprng_engine]',
    inlined from 'Json::Value ripple::doRandom(RPC::JsonContext&)' at ../src/xrpld/rpc/handlers/Random.cpp:46:23:
modules/xrpl.libxrpl.beast/xrpl/beast/utility/rngfill.h:58:20: error: 'void* memcpy(void*, const void*, size_t)' writing between 1 and 24 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
   58 |         std::memcpy(buffer, &v, bytes);
      |         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
../src/xrpld/rpc/handlers/Random.cpp: In function 'Json::Value ripple::doRandom(RPC::JsonContext&)':
../src/xrpld/rpc/handlers/Random.cpp:45:17: note: at offset 32 into destination object 'rand' of size 32
   45 |         uint256 rand;
      |                 ^~~~
cc1plus: all warnings being treated as errors

The warning is in context of

Json::Value
doRandom(RPC::JsonContext& context)
{
    // TODO(tom): the try/catch is almost certainly redundant, we catch at the
    // top level too.
    try
    {
        uint256 rand;
        beast::rngfill(rand.begin(), rand.size(), crypto_prng()); // < HERE

ximinez · 2025-07-11T17:57:52Z

modules/xrpl.libxrpl.beast/xrpl/beast/utility/rngfill.h:58:20: error: 'void* memcpy(void*, const void*, size_t)' writing between 1 and 24 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]

Thanks!

So the compiler is confused because buffer is a void*, and it doesn't know how much space is actually available, or something like that. Weird that it doesn't also complain about line 41, which is more or less doing the same thing, but I've never claimed to understand compiler eccentricities.

lmaisons · 2025-07-11T21:30:56Z

So the compiler is confused because buffer is a void*, and it doesn't know how much space is actually available, or something like that. Weird that it doesn't also complain about line 41, which is more or less doing the same thing, but I've never claimed to understand compiler eccentricities.

For completeness sake, and because I've been bitten by similar scenarios in the past, though admittedly on a different compiler, from my reading, it does know about the size of buffer: it correctly determines that there are zero bytes left available to memcpy into after the first memcpy loop, even realizing it will have written 32 bytes by that point. Where it appears to get confused is with the set of values it believes bytes capable of holding, believing to have found a potential discrepancy of 1-24 between the number of bytes in the area pointed to by buffer and the value passed in bytes.

I wonder if it would do a better job of analysis if the loop induction variable were made more explicit^1, though academic at this point. On a [potentially?] less academic note, depending on how aggressive our optimizations are, it might be worth verifying the generated code somehow to make sure that whatever analytical weirdness that raises the warning doesn't pollute the output.

^1:

template <class Generator>
void
rngfill(void* const buffer, const std::size_t bytes, Generator& g)
{
    using result_type = typename Generator::result_type;
    
    std::uint8_t * const buffer_start = static_cast<uint8_t *>(buffer);
    const std::size_t complete_iterations = bytes / sizeof(result_type);
    const std::size_t bytes_remaining = bytes % sizeof(result_type);

    for (std::size_t count = 0; count < complete_iterations; ++count)
    {
        auto const v = g();
        const std::size_t offset = count * sizeof(result_type);
        std::memcpy(buffer_start + offset, &v, sizeof(result_type));
    }

    if (bytes_remaining > 0)
    {
        auto const v = g();
        const std::size_t offset = complete_iterations * sizeof(result_type);
        std::memcpy(buffer_start + offset, &v, bytes_remaining);
    }
}

* XRPLF/develop: fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513) chore: Add gcc-12 workaround (#5554) Add MPT related txns into issuer's account history (#5530) chore: Remove unused headers (#5526)

Bronek · 2025-07-14T15:59:34Z

@lmaisons this is nice cleanup, do you mind turning it into a proper PR ? Or I can do it, if you prefer.

lmaisons · 2025-07-14T17:01:10Z

@Bronek Sure, I'll open a PR for it.

lmaisons · 2025-07-14T18:20:00Z

@Bronek Opened #5563

…refactoring-1 * upstream/develop: (56 commits) Remove `include(default)` from libxrpl profile (#5587) refactor: Change boost::shared_mutex to std::shared_mutex (#5576) Fix macos runner (#5585) Remove the type filter from "ledger" RPC command (#4934) refactor: Update date, libarchive, nudb, openssl, sqlite3, xxhash packages (#5567) test: Run unit tests regardless of 'Supported' amendment status (#5537) Retire Flow Cross amendment (#5562) chore: Update CI to use Conan 2 (#5556) fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513) chore: Add gcc-12 workaround (#5554) Add MPT related txns into issuer's account history (#5530) chore: Remove unused headers (#5526) fix: add allowTrustLineLocking flag for account_info (#5525) Downgrade required CMake version for Antithesis SDK (#5548) fix: Link with boost libraries explicitly (#5546) chore: Fix compilation error with clang-20 and cleanup (#5543) test: Remove circular jtx.h dependencies (#5544) Decouple CredentialHelpers from xrpld/app/tx (#5487) fix: crash when trace-logging in tests (#5529) test: switch some unit tests to doctest (#5383) ...

…actoring-2 * ximinez/lending-refactoring-1: (57 commits) Fix formatting Remove `include(default)` from libxrpl profile (#5587) refactor: Change boost::shared_mutex to std::shared_mutex (#5576) Fix macos runner (#5585) Remove the type filter from "ledger" RPC command (#4934) refactor: Update date, libarchive, nudb, openssl, sqlite3, xxhash packages (#5567) test: Run unit tests regardless of 'Supported' amendment status (#5537) Retire Flow Cross amendment (#5562) chore: Update CI to use Conan 2 (#5556) fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513) chore: Add gcc-12 workaround (#5554) Add MPT related txns into issuer's account history (#5530) chore: Remove unused headers (#5526) fix: add allowTrustLineLocking flag for account_info (#5525) Downgrade required CMake version for Antithesis SDK (#5548) fix: Link with boost libraries explicitly (#5546) chore: Fix compilation error with clang-20 and cleanup (#5543) test: Remove circular jtx.h dependencies (#5544) Decouple CredentialHelpers from xrpld/app/tx (#5487) fix: crash when trace-logging in tests (#5529) ...

…actoring-3 * ximinez/lending-refactoring-2: (57 commits) Fix formatting Remove `include(default)` from libxrpl profile (#5587) refactor: Change boost::shared_mutex to std::shared_mutex (#5576) Fix macos runner (#5585) Remove the type filter from "ledger" RPC command (#4934) refactor: Update date, libarchive, nudb, openssl, sqlite3, xxhash packages (#5567) test: Run unit tests regardless of 'Supported' amendment status (#5537) Retire Flow Cross amendment (#5562) chore: Update CI to use Conan 2 (#5556) fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513) chore: Add gcc-12 workaround (#5554) Add MPT related txns into issuer's account history (#5530) chore: Remove unused headers (#5526) fix: add allowTrustLineLocking flag for account_info (#5525) Downgrade required CMake version for Antithesis SDK (#5548) fix: Link with boost libraries explicitly (#5546) chore: Fix compilation error with clang-20 and cleanup (#5543) test: Remove circular jtx.h dependencies (#5544) Decouple CredentialHelpers from xrpld/app/tx (#5487) fix: crash when trace-logging in tests (#5529) ...

…actoring-4 * ximinez/lending-refactoring-3: (61 commits) fixup! Rename Transactor preflight functions Rename Transactor preflight functions fixup! Make preflight1 and preflight2 private static Transactor functions Make preflight1 and preflight2 private static Transactor functions Fix formatting Remove `include(default)` from libxrpl profile (#5587) refactor: Change boost::shared_mutex to std::shared_mutex (#5576) Fix macos runner (#5585) Remove the type filter from "ledger" RPC command (#4934) refactor: Update date, libarchive, nudb, openssl, sqlite3, xxhash packages (#5567) test: Run unit tests regardless of 'Supported' amendment status (#5537) Retire Flow Cross amendment (#5562) chore: Update CI to use Conan 2 (#5556) fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513) chore: Add gcc-12 workaround (#5554) Add MPT related txns into issuer's account history (#5530) chore: Remove unused headers (#5526) fix: add allowTrustLineLocking flag for account_info (#5525) Downgrade required CMake version for Antithesis SDK (#5548) fix: Link with boost libraries explicitly (#5546) ...

…actoring-4 * ximinez/lending-refactoring-3: (61 commits) fixup! Rename Transactor preflight functions Rename Transactor preflight functions fixup! Make preflight1 and preflight2 private static Transactor functions Make preflight1 and preflight2 private static Transactor functions Fix formatting Remove `include(default)` from libxrpl profile (#5587) refactor: Change boost::shared_mutex to std::shared_mutex (#5576) Fix macos runner (#5585) Remove the type filter from "ledger" RPC command (#4934) refactor: Update date, libarchive, nudb, openssl, sqlite3, xxhash packages (#5567) test: Run unit tests regardless of 'Supported' amendment status (#5537) Retire Flow Cross amendment (#5562) chore: Update CI to use Conan 2 (#5556) fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513) chore: Add gcc-12 workaround (#5554) Add MPT related txns into issuer's account history (#5530) chore: Remove unused headers (#5526) fix: add allowTrustLineLocking flag for account_info (#5525) Downgrade required CMake version for Antithesis SDK (#5548) fix: Link with boost libraries explicitly (#5546) ... Co-authored-by: Bronek Kozicki <brok@incorrekt.com>

Add gcc-12 workaround

d1ae28e

Bronek assigned ximinez and bthomee Jul 11, 2025

Bronek added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Jul 11, 2025

bthomee approved these changes Jul 11, 2025

View reviewed changes

Bronek unassigned ximinez and bthomee Jul 11, 2025

Bronek requested a review from ximinez July 11, 2025 15:30

Bronek mentioned this pull request Jul 11, 2025

Update CI to use Conan 2 #5556

Merged

2 tasks

legleux self-requested a review July 11, 2025 15:48

legleux approved these changes Jul 11, 2025

View reviewed changes

ximinez approved these changes Jul 11, 2025

View reviewed changes

Merge branch 'develop' into Bronek/gcc_workaround

9a30e72

bthomee enabled auto-merge (squash) July 11, 2025 18:17

Bronek added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Jul 11, 2025

bthomee merged commit 258ba71 into develop Jul 11, 2025
3 checks passed

bthomee deleted the Bronek/gcc_workaround branch July 11, 2025 18:57

This was referenced Aug 27, 2025

Release 2.6.0 #5741

Merged

Release 2.6.0 #5742

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gcc-12 workaround#5554

Add gcc-12 workaround#5554
bthomee merged 2 commits intodevelopfrom
Bronek/gcc_workaround

Bronek commented Jul 11, 2025

Uh oh!

codecov bot commented Jul 11, 2025 •

edited

Loading

Uh oh!

ximinez commented Jul 11, 2025

Uh oh!

Bronek commented Jul 11, 2025

Uh oh!

ximinez commented Jul 11, 2025

Uh oh!

Uh oh!

lmaisons commented Jul 11, 2025 •

edited

Loading

Uh oh!

Bronek commented Jul 14, 2025

Uh oh!

lmaisons commented Jul 14, 2025

Uh oh!

lmaisons commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Bronek commented Jul 11, 2025

High Level Overview of Change

Context of Change

Type of Change

Uh oh!

codecov bot commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ximinez commented Jul 11, 2025

Uh oh!

Bronek commented Jul 11, 2025

Uh oh!

ximinez commented Jul 11, 2025

Uh oh!

Uh oh!

lmaisons commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bronek commented Jul 14, 2025

Uh oh!

lmaisons commented Jul 14, 2025

Uh oh!

lmaisons commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Jul 11, 2025 •

edited

Loading

lmaisons commented Jul 11, 2025 •

edited

Loading