my-ship-it commented on issue #1677:
URL: https://github.com/apache/cloudberry/issues/1677#issuecomment-4278175916

   Hi @leborchuk, thank you for the detailed log and for pushing the Rocky 
Linux 10 port forward (tracking in #1432) — the compiler output you captured 
made this very easy to diagnose. Here's a summary of what's happening and what 
we could do about it, so we can discuss and pick a direction together.
   
   ## TL;DR
   
   These warnings are **not a PAX bug**, and they do **not break the build**. 
They're a long-standing false positive from `protobuf 3.19.6`'s internal 
layout, surfaced more aggressively by the newer GCC (14.x) that ships with 
Rocky Linux 10. PAX already anticipates this class of warning — 
`contrib/pax_storage/CMakeLists.txt:27` sets `-Wno-error=array-bounds` — which 
is why compilation continues past the warnings to completion. That said, the 
noise is real, and there are a few clean ways to reduce it.
   
   ## Root cause
   
   Every warning in your log traces back to the same two lines in 
`/usr/include/google/protobuf/repeated_ptr_field.h`:
   
   \`\`\`cpp
   // line 857 (Get) and 879 (Mutable)
   return cast<TypeHandler>(rep_->elements[index]);
   \`\`\`
   
   where \`elements\` is declared as:
   
   \`\`\`cpp
   // line 339
   void* elements[(std::numeric_limits<int>::max() - 2 * sizeof(int)) / 
sizeof(void*)];
   \`\`\`
   
   This is protobuf's older \"huge-declared flexible array\" trick for 
\`RepeatedPtrFieldBase::Rep\`. When PAX calls something like 
\`info->mutable_columnstats(i)\` or \`stripe.colstats(i)\`, the compiler 
inlines the chain down into \`Get(int)\` / \`Mutable(int)\`. Inside that 
inlined body, GCC's value-range propagation cannot see the caller-side 
invariant that \`i >= 0\`, so it conservatively keeps \`-1\` in the possible 
range for \`index\`, which triggers \`-Warray-bounds\` against the declared 
\`void* [268435454]\`.
   
   **Why Rocky 9 / older hosts don't complain:**
   - GCC 14 (Rocky 10 default) does significantly more aggressive cross-inline 
VRP than GCC 11/12.
   - Rocky 10's default \`protobuf-devel\` is still 3.19.6, which retains the 
old \`Rep\` layout.
   - protobuf upstream reworked \`RepeatedPtrFieldBase\` in 3.21+ / 4.x 
(dynamic allocation, tighter bounds expressions), so modern protobuf no longer 
trips this diagnostic.
   
   So the warning is the intersection of: **old protobuf headers + new GCC 
analysis**. It is not evidence of an actual out-of-bounds access.
   
   ## Build impact
   
   - The log shows compilation progressing from 26% → 42% → 49% 
(\`paxformat.so\` linked successfully) → 72% and beyond, with the warnings 
sprinkled in between. Nothing aborts.
   - PAX's top-level CMake intentionally uses \`-Werror 
-Wno-error=array-bounds\` so this category stays non-fatal. It looks like the 
project has seen this before on other GCC/protobuf combinations.
   
   In other words: your build should complete successfully. If it doesn't, 
please share the final failure line so we can look separately — that would be a 
different issue.
   
   ## Options to reduce the noise
   
   Listed from most surgical to most invasive. Happy to hear preferences before 
sending a patch.
   
   ### Option 1 — \`#pragma GCC diagnostic\` around protobuf includes in PAX 
(preferred)
   
   Wrap the protobuf includes in PAX's own headers (e.g. 
\`storage/proto/proto_wrappers.h\`) with:
   
   \`\`\`cpp
   #pragma GCC diagnostic push
   #pragma GCC diagnostic ignored \"-Warray-bounds\"
   #include <google/protobuf/repeated_field.h>
   #include <google/protobuf/repeated_ptr_field.h>
   // ... other protobuf headers ...
   #pragma GCC diagnostic pop
   \`\`\`
   
   **Pros:** silences only protobuf's own headers; PAX code keeps full 
\`-Warray-bounds\` coverage, so any genuine out-of-bounds regression we 
introduce still gets caught.
   **Cons:** needs one or two include sites touched; pragma needs a 
Clang-compatible guard (\`#if defined(__GNUC__) && !defined(__clang__)\` or 
equivalent) to stay portable.
   
   ### Option 2 — Per-target compile flag for protobuf-generated translation 
units
   
   In the CMake target that builds the generated \`*.pb.cc\` files, add 
\`target_compile_options(<tgt> PRIVATE -Wno-array-bounds)\`.
   
   **Pros:** single place, no source changes.
   **Cons:** any \`Get\`/\`Mutable\` call site inside PAX's own \`.cc\` (not in 
\`.pb.cc\`) still produces the warning, because the warning lives in the header 
chain, not in \`.pb.cc\` specifically. In your log several triggers come from 
\`micro_partition_stats.cc\`, \`orc_format_reader.cc\`, etc. — so this option 
alone won't fully silence the noise.
   
   ### Option 3 — Require newer protobuf on Rocky 10
   
   Update \`FindDependencies.cmake\` / docs to recommend protobuf 3.21+ (e.g. 
from EPEL or a locally built version) on Rocky 10.
   
   **Pros:** root-cause fix.
   **Cons:** users may not have a ready 3.21+ package on Rocky 10; this raises 
the bar for a supposedly minimal port.
   
   ## My recommendation
   
   **Option 1 + a one-line mention in the Rocky 10 build notes.** Option 1 
cleans the build log without weakening diagnostics for PAX's own code, and a 
note in the docs helps anyone who stumbles on leftover warnings during 
transitions (e.g. clang builds, or users on older protobuf that we can't wrap).
   
   If you're happy with that direction, I'd be glad to prepare a PR — it should 
be a small, reviewable change.
   
   Also happy to hear if you prefer another option or if there's context about 
Rocky 10 packaging I'm missing. Thanks again for the careful bug report.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to