Conversation
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).
|
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. |
|
Which warning is GCC emitting? Is it about the section attribute, the I had originally tried to go with |
|
It was error |
|
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. |
|
Gentle ping. Chris, please, let me know if there is anything I can help with, to get this merged. |
|
I think I understand dropping the section attribute, but can we keep the |
|
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. |
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).