RFR: 8263185: Mallinfo deprecated in glibc 2.33 [v2]

Thomas Stuefe stuefe at openjdk.java.net
Mon Mar 15 14:48:07 UTC 2021


On Mon, 15 Mar 2021 12:10:19 GMT, Christoph Göttschkes <cgo at openjdk.org> wrote:

> Hi Thomas,
> 
> > Also please use CAST_TO_FN_PTR instead of manually casting.
> 
> Sure thing, changed in the last commit.
> 
> About moving the symbol resolution. Before changing this, I would like to get your feedback, because the reason I didn't do it where you are suggesting was the amount of GLIBC version dependent code I would introduce. If I move the the resolution (and keep the current logic), I would add 3 more `ifdefs` which check the glibc version:
> 
> 1. Delcaration of the two function pointers
> 2. Initialization of the two function pointer
> 3. Symbol resolution
> 
> This seemed a lot of dependent code, all of which need two `#if`:
> 
> ```
> #ifdef GLIBC
> // declare (os_linux.hpp)  / intialize (os_linux.cpp) / resolve (os_linux.cpp os::init) mallinfo function pointer
> #if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 33)
> // declare / intialize / resolve mallinfo2 function pointer
> #endif
> #endif
> ```
> 
> We always need two pointers, since we need to know which one we are calling because of the return type. We could only have one and do something like the following, but then we loose the ability to call `mallinfo`, if the JVM has been linked against glibc >= 2.33:
> 
> ```
> #ifdef GLIBC
> #if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 33)
> // declare / intialize / resolve mallinfo2 function pointer
> #else
> // declare / intialize / resolve mallinfo function pointer
> #endif
> #endif
> ```
> 
> I wanted to avoid that, that's why I did the resolution where it is right now.
> 
> We could also ignore `mallinfo2` for the moment, and always call `mallinfo`, regardless of the glibc version. I think the important part right now is, two get rid of the compile time warning (by resolving `mallinfo` at runtime using `dlsym`), not enhancing the error reporting.

The code does not have to be complicated. You do not need any ifdefs. In fact, it would be better if you would avoid them.

1) locally define struct mallinfo2. Actually all we need is the correct offset for urordblks so in principle all other members could be called dummy1, dummy2...
2) define mallinfo2* (mallinfo2_fun)() = NULL;
3) in init, resolve mallinfo2
4) when reporting, if mallinfo2_fun=NULL, call it, otherwise call (statically linked) mallinfo. 

That way you don't have any #ifs; a VM compiled on an older machine will still show correct values when running on a machine with glibc2.33 and later. VM would be back-and-forward compatible. Patch should be quite small. 

You could even do completely without defining mallinfo2 structure:
1) define (void*)(mallinfo2_fun)() = NULL
2) resolve it
3) use it like this:

if (mallinfo2_fun != NULL) {
	void* p = mallinfo2_fun();
	size_t total_allocated = ((size_t*)p)[7];
}

which is perfectly fine since mallinfo2 is just a sequence of size_t values. Bit less code than the first alternative; the first alternative is a bit cleaner and more obvious.

All that said, I read David proposed to just mute the warning. I'm fine with that too though I would prefer to use mallinfo2. I leave it up to you.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2964


More information about the hotspot-runtime-dev mailing list