Add option to set mimalloc symbols to visibility=hidden

[tiran]

mimalloc.h uses __attribute__((visibility("default")) or __declspec() to export public mimalloc symbols. For embedding and vendoring of mimalloc it would be very helpful to be able to hide all symbols. In the CPython case, we don't want to export mimalloc symbols to consumers of libpython3.x.so. Compiler flags like -fvisibility=hidden don't hide the symbols. It seems like the __attribute__ takes precedence and overrides the compiler flag.

As a quick hack we came up with this solution in python/cpython#29123

#if defined(MIMALLOC_VENDOR)
  // hide symbols when vendoring mimalloc
  #undef mi_decl_export
  #define mi_decl_export mi_decl_restrict
#endif

[corona10]

yeah we need it :)

[daanx]

Hi Christian -- thanks for the work on integrating mimalloc, and feel free to contact me if you would like to chat about mimalloc sometime.

The visibility is done this way as the cmake passes -fvisibility=hidden to make all other extern definitions hidden (these start with an underscore and are declared in include/mimalloc-internal.h).
This way we can build mimalloc as separate source files into one library (although, with src/static.c this is not really necessary anymore).

Redefining mi_decl_export is indeed the right way to change this, but it should not be made equal to mi_decl_restrict, Just an empty define (or using __attribute__(("internal")) may work? ).

Perhaps the best change is something along the lines of:

#ifndef MI_EXPORT_VISIBILITY
#define MI_EXPORT_VISIBILITY "default"
#endif
#define mi_decl_export                __attribute__((visibility(MI_EXPORT_VISIBILITY)))

and then you could define MI_EXPORT_VISIBILITY to "internal" or "hidden" as needed? Would that work for you?

[tiran]

Hi Daan,

some background: in CPython we also use -fvisibility=hidden to hide symbols. The Python library libpythonX.Y.so and archive libpythonX.Y.a are embedded often. To avoid symbol clashes we try to most symbols that are not part of our public or internal API. Our main library must only export symbols that are prefix with either Py or _Py. With src/static.c and __attribute__((visibility("default"))) the mimalloc symbols appeared as exported symbols in our shared library. The attribute directive has a higher precedence than the command line option.

Your proposed solution works for us. The modified patch is already in my experimental pull request. Because the value is a string, it's a bit annoying to pass the flag on the command line: -DMI_EXPORT_VISIBILITY=\"hidden\". Not a major problem, though.

Dankjewel! :)

[daanx]

Thanks for explaining how CPython uses this. I actually don't like my proposed fix as it is so gcc/clang specific. Since you only need "hidden" perhaps we should specialize it so it could potentially also work for, say, MSVC (in the future):

#ifdef MI_NO_EXPORT
  #define mi_decl_export               
#else
  #define mi_decl_export                __attribute__((visibility("default")))
#endif

This would just make the mi_decl_export empty which I think is already enough for you? (or should it be __attribute__((visibility("hidden"))) ? ). This also gets rid of the quotes when passing the define on the command line -- -DMi_NO_EXPORT would do.

[tiran]

I agree, your new proposal is easier to use. The GCC man page describes other two possible options for:

protected and internal are pretty useless in real-world usage so the only other commonly used option is hidden.

An empty mi_decl_export works for us. It lets us specify the visibility on the command line.

[daanx]

Hi @tiran , apologies for keeping to come back to this; I read again the docs for -fvisibility and I think it only has an effect for shared libraries. Already, mimalloc defines MI_SHARED_LIB and MI_SHARED_LIB_EXPORT in that case when building the .so (or .dylib or .dll) file. I think we can do the same as for MSVC and just check for shared library building. You are linking statically with mimalloc right? In that case it should work without any extra options. i.e.:

#if defined(MI_SHARED_LIB) && defined(MI_SHARED_LIB_EXPORT)
    #define mi_decl_export              __attribute__((visibility("default")))
  #else
    #define mi_decl_export
  #endif

If you are linking statically with mimalloc, the above would work as neither MI_SHARED_LIB and MI_SHARED_LIB_EXPORT will be defined. Let me know what you think, Best , Daan

[tiran]

This is much better than the previous solutions. Thanks!