RFR: Improve macros for libjdwp memory function overrides

[David Holmes]

On 12/04/2017 11:07 AM, Mikael Vidstedt wrote:
>
> The memory management functions (malloc, free, …) should not be used directly in the libjdwp library. To catch any incorrect uses of the functions, the is a handful of macros in libjdwp/util.h which override the raw functions with something which will produce a build time error.
>
> Because of some the internal details of how the musl header files work, some system header files get included *after* the macro overrides, and the result is a bunch of (rather confusing) build time errors. I believe what happens is something along the lines of the following:
>
> libjdwp/util.h does this:
>
> #define calloc(p) Do not use this interface.
>
> Later, through some include chain, one of the system header files (sched.h) gets included. It contains some references to calloc, along the lines of:
>
> ...
> void *calloc(size_t, size_t);
> …
> #define CPU_ALLOC(n) ((cpu_set_t *)calloc(1, CPU_ALLOC_SIZE(n)))
> ...
>
> But since calloc has been redefined to a bunch of individual words/symbols, the result is an error pointing back to the original macro override definition in libjdwp/util.h:
>
> jdk/src/jdk.jdwp.agent/share/native/libjdwp/util.h:42:28: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'not'
>      #define calloc(p,s) Do not use this interface.
>
> In either case, this change updates the macros to use a macro value which will still induce a build time error if the system memory functions are used in libjdwp (something like “implicit declaration of function ‘do_not_use_this_interface_malloc’), but which doesn’t produce a build time error in the normal/happy case.
>
> That said, this change is incomplete/fishy in the sense that there may well be cases where it’s the wrong thing to do. Specifically, the system header files may well expect to be able to do CPU_ALLOC/calloc implicitly, as part of some system function call or something else. I’ve tried to think of alternative solutions, but haven’t come up with anything so far. Taking suggestions, but meanwhile I’ll push this change.

I would suggest ensuring that util.h is always the last include in the 
.c files - that will avoid any possible interference with system 
headers. If that is not possible then I suggest moving this set of 
defines to a new header file which can be included last.

The current approach is potentially broken, as you note, if any of the 
affected system headers actually use the redefined functions in their 
own macro expansions.

Thanks,
David

> http://cr.openjdk.java.net/~mikael/webrevs/portola/memmacros/webrev.00/jdk/webrev/
>
> Cheers,
> Mikael
>