NativeGeneralJump::insert_unconditional ResourceMark - was : RE: RFR [XS] : 8219112: name_and_sig_as_C_string usages in frame_s390 miss ResourceMark

[Doerr, Martin]

Hi Matthias,

aarch64 looks ok. The MacroAssembler is allocated on stack. So I can't find anything which gets allocated in resource area.
Actually, I think this would be a better fix for PPC64 than adding a ResourceMark (see below).

Best regards,
Martin



diff -r 140ecd460080 src/hotspot/cpu/ppc/nativeInst_ppc.cpp
--- a/src/hotspot/cpu/ppc/nativeInst_ppc.cpp    Tue Feb 19 12:52:28 2019 +0100
+++ b/src/hotspot/cpu/ppc/nativeInst_ppc.cpp    Tue Feb 19 12:56:50 2019 +0100
@@ -362,8 +362,8 @@

 void NativeGeneralJump::insert_unconditional(address code_pos, address entry) {
   CodeBuffer cb(code_pos, BytesPerInstWord + 1);
-  MacroAssembler* a = new MacroAssembler(&cb);
-  a->b(entry);
+  MacroAssembler a(&cb);
+  a.b(entry);
   ICache::ppc64_flush_icache_bytes(code_pos, NativeGeneralJump::instruction_size);
 }



-----Original Message-----
From: Baesken, Matthias 
Sent: Dienstag, 19. Februar 2019 09:47
To: Doerr, Martin <martin.doerr at sap.com>; Aleksey Shipilev <shade at redhat.com>; 'hotspot-dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
Subject: NativeGeneralJump::insert_unconditional ResourceMark - was : RE: RFR [XS] : 8219112: name_and_sig_as_C_string usages in frame_s390 miss ResourceMark

Hi Martin, thanks for commenting .

> 
> In the example below (nativeInst_ppc.cpp), I think we should add one. At
> least, I couldn't find one at a good place.
>

NativeGeneralJump::insert_unconditional   for aarch64 looks similar  to what we see in   nativeInst_ppc.cpp  ,   should we add a ResourceMark   here as well ?
 (maybe someone from  the aarch64  developers could comment).  


src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp

490void NativeGeneralJump::insert_unconditional(address code_pos, address entry) {
491  NativeGeneralJump* n_jump = (NativeGeneralJump*)code_pos;
492
493  CodeBuffer cb(code_pos, instruction_size);
494  MacroAssembler a(&cb);
495
496  a.movptr(rscratch1, (uintptr_t)entry);
497  a.br(rscratch1);
498
499  ICache::invalidate_range(code_pos, instruction_size);
500}




Best regards, Matthias


> -----Original Message-----
> From: Doerr, Martin
> Sent: Montag, 18. Februar 2019 15:52
> To: Baesken, Matthias <matthias.baesken at sap.com>; Aleksey Shipilev
> <shade at redhat.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
> dev at openjdk.java.net>
> Subject: RE: RFR [XS] : 8219112: name_and_sig_as_C_string usages in
> frame_s390 miss ResourceMark
> 
> Hi Matthias,
> 
> we would get "fatal error: memory leak: allocating without ResourceMark" in
> the debug build if there was no ResourceMark at all on the stack. However,
> the existing one may be at a bad position where its scope never gets left and
> the resources never get freed. This situation can't get detected
> automatically.
> 
> In the example below (nativeInst_ppc.cpp), I think we should add one. At
> least, I couldn't find one at a good place.
> 
> Yes, the current process is error prone. ResourceMarks can also easily get
> placed at wrong positions leading to "assert... re-allocated with a different
> ResourceMark".
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> Baesken, Matthias
> Sent: Montag, 18. Februar 2019 14:40
> To: Aleksey Shipilev <shade at redhat.com>; 'hotspot-dev at openjdk.java.net'
> <hotspot-dev at openjdk.java.net>
> Subject: RE: RFR [XS] : 8219112: name_and_sig_as_C_string usages in
> frame_s390 miss ResourceMark
> 
> Hi Aleksey and Lutz , thanks  for  the reviews.
> 
> 
> One (maybe stupid)  general   question  -  is there an easy way to find out  all
> locations  of missing ResourceMark usages ?
> Currently  it looks to me  like a very  error prone process .
> 
> The name_and_sig_as_C_string()  issue  in this  CR   seems to be an easy and
> simple case .
> 
> 
> But in  other cases  it is  (for me at least)   not so easy to say - is the
> ResourceMark missing  or  is it  somewhere located  in the calling functions ?
> One example :
> 
> src/hotspot/cpu/ppc/nativeInst_ppc.cpp
> 
> 363  void NativeGeneralJump::insert_unconditional(address code_pos,
> address entry) {
> 364    CodeBuffer cb(code_pos, BytesPerInstWord + 1);
> 365    MacroAssembler* a = new MacroAssembler(&cb);
> 366    a->b(entry);
> 367    ICache::ppc64_flush_icache_bytes(code_pos,
> NativeGeneralJump::instruction_size);
> 368  }
> 
>  Do we need the ResouceMark here or not ?
>