RFR: 8224221: add memprotect calls to event log

[David Holmes]

Hi Matthias,

On 21/05/2019 1:08 am, Baesken, Matthias wrote:
> Hello,  please review  this small changeset .
> 
> The current event log holds the last n events of various interesting operations.
> It is printed in the hs_err file and helps to analyze various issues / bugs .
> 
> I would like to add memory protection events to the current set or events added by calling Events::log .
> (we had this for a long time in our  proprietary  VM and  would like to have it in Open JDK too )

So extra events will now be generated so you doubled the size of the 
buffer - any info on how this impacts the events that may get captured? 
How often do mprotect events happen? Might we always see 20 mprotect 
events and nothing else of interest? Just wondering (I never use this 
event stuff myself).

> Bug/webrev :
> 
> https://bugs.openjdk.java.net/browse/JDK-8224221
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8224221.0/

Nit: "with protections %d" doesn't read right to me as "protection" is 
not a noun. How about "with protection modes %d" or "with protection 
bits %d"? (It would be nice if you could print a symbolic representation 
of the bits rather than just the raw value - and %x may be better than 
%d for the raw value.)

---

src/hotspot/os/bsd/os_bsd.cpp

The log event should be using bottom and bottom+size for its range.

You didn't log these additional ::mprotect calls:

1936   if (::mprotect(addr, size, prot) == 0) {
2020   return ::mprotect(addr, size, PROT_NONE) == 0;

---

src/hotspot/os/linux/os_linux.cpp

The log event should be using bottom and bottom+size for its range.

---

src/hotspot/os/solaris/os_solaris.cpp

There is no "size" variable, it's called "bytes".

It's odd that the Solaris polling_page routines call mprotect directly 
rather than solaris_mprotect. That gave you the opportunity to be more 
specific about logging those events on Solaris, but the other platforms 
miss out. Overall the use of mprotect/<os>_mprotect/guard_memory is a 
bit of a mess. :(

---

src/hotspot/share/runtime/thread.cpp

  jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
+   eventlog_init();

You've made eventlog_init() the very first thing we now do in VM 
initialization which raises a number of issues for me:

- is everything it does actually safe to do with ZERO other VM 
initialization having occurred? I don't see how as it uses a Mutex when 
constructing EventLogBase and mutex initialization has not yet occurred!

- we will always initialize the Event logs because we won't yet have 
processed the parameters that may turn of LogEvents.

I think the initialization placement needs re-examination. When does the 
first mprotect call occur? I'd take a guess it would be for the guard 
pages of the main thread when it attaches.

Thanks,
David
----

> 
> Thanks, Matthias
>