Skip to content

unbreak compilation with GCC 14#289

Open
alk wants to merge 1 commit intogoogle:masterfrom
alk:master
Open

unbreak compilation with GCC 14#289
alk wants to merge 1 commit intogoogle:masterfrom
alk:master

Conversation

@alk
Copy link

@alk alk commented Feb 1, 2026

With previous code gcc ended up giving odd error about section mismatch for kInvalidSpan. Not sure what it was, but kInvalidSpan is only used for it's address. So there is never need for constexpr it. This change turns it into "classic" constant which is declared in the header and defined in the .cc (with whatever section attribute it needs).

With previous code gcc ended up giving odd error about section
mismatch for kInvalidSpan. Not sure what it was, but kInvalidSpan is
only used for it's address. So there is never need for constexpr
it. This change turns it into "classic" constant which is declared in
the header and defined in the .cc (with whatever section attribute it
needs).
@alk
Copy link
Author

alk commented Feb 1, 2026

Also note that in gperftools I finally killed this attribute section madness. In your code I see that you've embraced it instead. AFAIR the original need for this section was for hook's GetCallerStackTrace. Which was only ever used by the heap checker code for it's super-hard-to-comprehend heuristics. For us, mere mortals, pprof filters the backtraces as/when needed. So it is effectively unnecessary complication. Do consider dropping it as well.

@ckennelly
Copy link
Collaborator

Which warning is GCC emitting? Is it about the section attribute, the constexpr, or both? Ideally we use constexpr/constinit on all of our globals to ensure we don't have to think about initializers.

I had originally tried to go with .data.rel.ro for this, so that us accidentally mutating it--if we failed to check the address first--would cause the program to blow up rather than continue in the presence of UB, but I can see dropping that at this point.

@alk
Copy link
Author

alk commented Feb 1, 2026

It was error

user@big:~/src/External/abseil-tcmalloc# USE_BAZEL_VERSION=8.5.1 CC=gcc-13 bazel build -c opt //tcmalloc:tcmalloc
INFO: Analyzed target //tcmalloc:tcmalloc (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //tcmalloc:tcmalloc up-to-date:
  bazel-bin/tcmalloc/libtcmalloc.lo
  bazel-bin/tcmalloc/libtcmalloc.pic.lo
INFO: Elapsed time: 0.047s, Critical Path: 0.00s
INFO: 1 process: 34 action cache hit, 1 internal.
INFO: Build completed successfully, 1 total action
user@big:~/src/External/abseil-tcmalloc# USE_BAZEL_VERSION=8.5.1 CC=gcc-14 bazel build -c opt //tcmalloc:tcmalloc
INFO: Analyzed target //tcmalloc:tcmalloc (1 packages loaded, 169 targets configured).
INFO: From Compiling tcmalloc/internal/percpu_rseq_asm.S:
/tmp/ccJfLMAq.s: Assembler messages:
tcmalloc/internal/percpu_rseq_asm.S:74: Warning: ignoring incorrect section type for .note.GNU-stack
tcmalloc/internal/percpu_rseq_asm.S:74: Warning: ignoring changed section type for .note.GNU-stack
ERROR: /home/me/src/External/abseil-tcmalloc/tcmalloc/BUILD:96:11: Compiling tcmalloc/tcmalloc.cc failed: (Exit 1): gcc-14 failed: error executing CppCompile command (from target //tcmalloc:tcmalloc) /usr/bin/gcc-14 -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 36 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
tcmalloc/tcmalloc.cc:2079:1: error: section type conflict with 'tcmalloc::tcmalloc_internal::Static::kInvalidSpan'
 2079 | }  // extern "C"
      | ^
In file included from ./tcmalloc/error_reporting.h:21,
                 from ./tcmalloc/allocation_sampling.h:24,
                 from tcmalloc/tcmalloc.cc:88:
./tcmalloc/static_vars.h:260:7: note: 'tcmalloc::tcmalloc_internal::Static::kInvalidSpan' was declared here
  260 |       kInvalidSpan{};
      |       ^~~~~~~~~~~~
cc1plus: note: unrecognized command-line option '-Wno-nullability-completeness' may have been intended to silence earlier diagnostics
Target //tcmalloc:tcmalloc failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 9.619s, Critical Path: 6.86s
INFO: 164 processes: 3 internal, 161 linux-sandbox.
ERROR: Build did NOT complete successfully

@alk
Copy link
Author

alk commented Feb 1, 2026

Ah I just realized that I've haven't communicated clearly. By section madness I am referring to .text.google_malloc stuff. All if it is gone in gperftools and I advise you to consider the same.

@alk
Copy link
Author

alk commented Feb 13, 2026

Gentle ping. Chris, please, let me know if there is anything I can help with, to get this merged.

@ckennelly
Copy link
Collaborator

I think I understand dropping the section attribute, but can we keep the constexpr?

@alk
Copy link
Author

alk commented Feb 14, 2026

I don't know if we can keep constexpr there.

Let me know if I am misunderstanding anything. I am keeping the .ro section thing. This is useful not only to avoid runtime initialization but also in various places where invalid span is used, we're making sure no one is actually mutating this thing at run-time.

For why constexpr removal helps, here is my understanding. So constexpr is basically "inline" for the data. It means compilers will insert it into every .cc file that includes this header. And then linker with deduplicate as needed. Somewhere along this path gcc managed to produce different sections for this thing in different object files or something like that. It may be related to tcmalloc_section thingy or not (but I suspect it is).

Also constexpr isn't really helpful for the kInvalidSpan, because you never use this guy for it's value, only for it's address. Classic "non-inline" variable is great for that. And doesn't break whatever gcc is doing there.

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