[CF] Avoid using malloc introspection. #2626
Conversation
CFBasicHashGetSize has an argument, total, to enable whether to presumably calculate the size its dependent data structures. CFBasicHashGetSize does not appear outside CFBasicHash.c, and this total argument is true only when ENABLE_MEMORY_COUNTERS is true or transitively when ENABLE_DTRACE_PROBES is true. These preprocessor symbols appear to be to control debugging and other informational purposes. malloc introspection, as afforded by malloc_size, malloc_usable_size, or msize, is nonportable (and thus not guaranteed to be present on all platforms), not guaranteed to be accurate (though certainly an upper bound), and is generally recommended for debugging. For those reasons, only expose the call to malloc_size when either of ENABLE_MEMORY_COUNTERS or ENABLE_DTRACE_PROBES is true. This avoids an undefined symbol issue when malloc introspection is unavailable, but does mean these counters and other information is not available on platforms when malloc_size is not present.
|
It seems that the standard library uses wraps |
|
Yes, the Swift stdlib also uses malloc_size, but that is something of a separate issue[1], because CF ought to be able to be built standalone outside of Swift. [1] This is really unfortunate, since it's basically used at the heart of ManagedBuffer.capacity (and somewhere in ContiguousArray also, from memory): it's exposed as part of a public API and thus isn't something that is only exposed during a debugging rebuild like here in this PR. But trying to deal with that in the right way is a PR for another day. |
|
CC: @parkera @millenomi I believe that |
|
@swift-ci please test |
|
@3405691582 would you mind updating the commit message for the second change to remove the statement that the function is vestigial, as I believe it to be a debugger-only debugging aid function. |
|
@swift-ci please test macOS platform |
This function is tagged with __attribute__((unused)) and certainly appears to be unused. It may be a function intended for use in a debugger context. As it is a static function, the reference to this function and malloc_size will not necessarily be present when built, _but_ this does cause a compiler warning when building on systems where malloc_size or alternatives are not available. malloc introspection, as afforded by malloc_size, malloc_usable_size, or msize, is nonportable (and thus not guaranteed to be present on all platforms), not guaranteed to be accurate (though certainly an upper bound), and is generally recommended for debugging. For those reasons, remove this function.
3405691582
force-pushed
the
PleaseAvoidMallocIntrospection
branch
from
cd2eb7a to
74cd0cf
Compare
|
Actually, after looking closer at On the one hand, that's good, because the symbol reference isn't actually emitted here, but it does cause a compiler warning when I've therefore reworded that commit accordingly. If this analysis is somehow off base, please let me know and I'll update the commit description further. |
|
It should be preserved at |
|
@swift-ci please test |
This appears to have gotten lost in the recent Catalina merge.
[CF] Reapply #2626 (do not call malloc_size).
malloc introspection, as afforded by
malloc_size,malloc_usable_size, or_msize, is nonportable (and thus not guaranteed to be present on all platforms), not guaranteed to be accurate (though certainly an upper bound), and is generally recommended for debugging.For those reasons, the following commits are introduced in this PR:
CFBasicHashGetSize'stotalbranch when one or both of the debugging symbolsENABLE_MEMORY_COUNTERSorENABLE_DTRACE_PROBESare true__CFStorageEstimateTotalAllocatedSize. If this is necessary for something, please let me know what constraints are required to avoid exposing themalloc_sizecall.