On Thu, 22 Feb 2024 14:11:54 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> Well, I hope that can be done, but it depends... As I see it, there are >> three possible ways to get rid of this `HIDDEN`. >> >> 1) Accept that we export these functions on gcc, and just remove the >> attribute. (Kind of capitulating to the bug.) >> >> 2) See if there is a way we can work around this problem, by changing >> something in how these functions are defined, that does not trigger this bug. >> >> 3) Have this bug fixed in gcc, and move to a minimum version of gcc that >> includes this fix. >> >> I'm not terribly keen on any of these solutions. I think 2, if it is >> possible, would be the simplest and cleanest. Or 1, if we really think this >> HIDDEN workaround is worse than the problem it solves (unintentionally >> exporting symbols). > > I see, seems like it's an unfortunate situation where a fix is hard or even > impossible. If we file a gcc bug for it now, it'll get fixed in some insanely > new gcc version (such as gcc 15 or 16) and we'd have to jump to that version, > which I'm sure no one is going to like :( > > Nevertheless, since there are so many instances of it, I suggest #pragma GCC > visibility push(hidden) instead of adding a new macro to this file, in my > opinion it's somewhat cleaner and also makes the change smaller. > > #pragma GCC visibility push(hidden) > // Junk that needs hidden visibility to work properly > #pragma GCC visibility pop > > Also, why TARGET_COMPILER_gcc instead of __GNUC__? Does this have to work > with Apple Clang on macOS as well? This is not an issue with clang, only gcc. My understanding was that the proper way to test for compiler in Hotspot code was by using `TARGET_COMPILER_gcc`. I can try using the `#pragma` route instead; if it works I agree it is slightly better than to sprinkle `HIDDEN` all over the place. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1499427455
