RFR(S) 8209950: SIGBUS in CodeHeapState::print_names()

[Schmidt, Lutz]

Hi Tobias, 

I observed a NULL pointer for method->signature() in sharedRuntime.cpp, but never saw garbage (which would need is_readable_poiner() to detect). To that end, the is_readable_pointer() check is not necessary. I added is anyway out of "bad experience". Is_readable_pointer() isn't a cheap check, so there always is a tradeoff. I can add the check to method->name() as well if you require it. 

A more general remark on the situation we are dealing with here:
In codeHeapState.cpp, I know we are running without any safety net: the CodeCache_lock is not held, opening the door to arbitrary concurrent state changes in the code cache. We need extra precautions to compensate for the missing safety net. 
In sharedRuntime.cpp, on the other hand, we are protected by the CodeCache_lock. CodeCache::nmethods_do() asserts that. The lock obviously doesn't protect us from seeing inconsistent nmethod states. That could be subject of a separate investigation.

Thanks, 
Lutz 


On 28.08.18, 09:58, "Tobias Hartmann" <tobias.hartmann at oracle.com> wrote:

    Hi Lutz,
    
    On 28.08.2018 09:45, Schmidt, Lutz wrote:
    > In sharedRuntime.cpp, method->signature is used in line
    >    2142:  ArgumentCount args(method->signature());
    > The ArgumentCount CTOR isn't ready to handle NULL pointers. I hit this issue while testing for JDK-8207343, after first applying the so far sufficient fix for JDK-8209588.
    
    Sorry, I was actually referring to the change in codeHeapState.cpp (not sharedRuntime.cpp). I was
    wondering why you added a os::is_readable_pointer() check for method->signature() but not for
    method->name().
    
    > In codeHeapState.cpp, method->name() is not checked in the "big if statement", but a few lines further down (@ line 2198). Why? I did not want to skip the output, just because the name is unavailable. Plus, method->name() is only called if the method is active ("inuse" or "notused"), making it very unlikely for it'S name to be NULL. 
    
    Yes but method->name() is only checked for NULL and not for os::is_readable_pointer(), right? Is
    that because the pointer must be valid for the inuse/notused cases?
    
    Thanks,
    Tobias