Address review comments (#15)

* Address review comments

Author: asgibbons

src/hotspot/cpu/x86/macroAssembler_x86.cpp

@@ -5965,7 +5965,7 @@ void MacroAssembler::generate_fill(BasicType t, bool aligned,
     orl(value, rtmp);
   }
 
-  cmpl(count, 2<<shift); // Short arrays (< 8 bytes) fill by element
+  cmpptr(count, 2<<shift); // Short arrays (< 8 bytes) fill by element
   jcc(Assembler::below, L_fill_4_bytes); // use unsigned cmp
   if (!UseUnalignedLoadStores && !aligned && (t == T_BYTE || t == T_SHORT)) {
     Label L_skip_align2;
@@ -6042,7 +6042,7 @@ void MacroAssembler::generate_fill(BasicType t, bool aligned,
           Label L_fill_64_bytes_loop_avx3, L_check_fill_64_bytes_avx2;
 
           // If number of bytes to fill < VM_Version::avx3_threshold(), perform fill using AVX2
-          cmpl(count, VM_Version::avx3_threshold());
+          cmpptr(count, VM_Version::avx3_threshold());
           jccb(Assembler::below, L_check_fill_64_bytes_avx2);
 
           vpbroadcastd(xtmp, xtmp, Assembler::AVX_512bit);

src/hotspot/cpu/x86/stubGenerator_x86_64.cpp

@@ -4013,6 +4013,11 @@ void StubGenerator::generate_initial_stubs() {
     UnsafeCopyMemory::create_table(16);
   }
 
+  // Initialize table for unsafe set memeory check.
+  if (UnsafeSetMemory::_table == nullptr) {
+    UnsafeSetMemory::create_table(16);
+  }
+
   // entry points that exist in all platforms Note: This is code
   // that could be shared among different platforms - however the
   // benefit seems to be smaller than the disadvantage of having a

src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp

@@ -732,6 +732,7 @@ address StubGenerator::generate_disjoint_copy_avx3_masked(address* entry, const
   __ ret(0);
 
   if (MaxVectorSize == 64) {
+    UnsafeCopyMemoryMark ucmm(this, !is_oop && !aligned, false, ucme_exit_pc);
     __ BIND(L_copy_large);
     arraycopy_avx3_large(to, from, temp1, temp2, temp3, temp4, count, xmm1, xmm2, xmm3, xmm4, shift);
     __ jmp(L_finish);
@@ -2547,6 +2548,7 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
     // Fill words
     {
       Label L_wordsTail, L_wordsLoop, L_wordsTailLoop;
+      UnsafeSetMemoryMark usmm(this, true, true);
       //////  Set words
       __ leaq(rScratch2, Address(size, 1));
       __ movq(rScratch1, rScratch2);
@@ -2584,14 +2586,15 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
       __ incrementq(rScratch1);
       __ cmpq(size, rScratch1);
       __ jccb(Assembler::notEqual, L_wordsTailLoop);
-      __ jmp(L_exit);
     }
+    __ jmp(L_exit);
 
     __ BIND(L_fillQuadwords);
 
     // Fill QUADWORDs
     {
       Label L_qwordLoop, L_qwordsTail, L_qwordsTailLoop;
+      UnsafeSetMemoryMark usmm(this, true, true);
 
       __ leaq(rScratch2, Address(size, 7));
       __ movq(rScratch1, rScratch2);
@@ -2628,14 +2631,18 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
       __ incrementq(rScratch1);
       __ cmpq(size, rScratch1);
       __ jccb(Assembler::notEqual, L_qwordsTailLoop);
-      __ jmp(L_exit);
     }
+    __ BIND(L_exit);
+
+    restore_arg_regs();
+    __ ret(0);
 
     __ BIND(L_fillDwords);
 
     // Fill DWORDs
     {
       Label L_dwordLoop, L_dwordsTail, L_dwordsTailLoop;
+      UnsafeSetMemoryMark usmm(this, true, true);
 
       __ leaq(rScratch2, Address(size, 3));
       __ movq(rScratch1, rScratch2);
@@ -2675,13 +2682,14 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
       __ incrementq(rScratch1);
       __ cmpq(size, rScratch1);
       __ jccb(Assembler::notEqual, L_dwordsTailLoop);
-      __ jmpb(L_exit);
     }
+    __ jmpb(L_exit);
 
     __ BIND(L_fillBytes);
 #ifdef MUSL_LIBC
     {
       Label L_byteLoop, L_longByteLoop, L_byteTail, L_byteTailLoop;
+      UnsafeSetMemoryMark usmm(this, true, true);
 
 #undef wide_value
 #define savedSize rax
@@ -2738,13 +2746,22 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
       __ movq(c_rarg2, rax);
 
       __ xchgq(c_rarg1, c_rarg2);
-      __ jump(RuntimeAddress(byte_fill_entry));
+      // generate_unsafe_fill(T_BYTE, false, "unsafe_set_memory");
+      __ mov(r11, c_rarg2);
+
+      __ enter(); // required for proper stackwalking of RuntimeStub frame
+
+      {
+        UnsafeSetMemoryMark usmm(this, true, true);
+
+        __ generate_fill(T_BYTE, false, c_rarg0, c_rarg1, r11, rax, xmm0);
+      }
+
+      __ vzeroupper();
+      __ leave(); // required for proper stackwalking of RuntimeStub frame
+      __ ret(0);
     }
 #endif  // MUSL_LIBC
-    __ BIND(L_exit);
-
-    restore_arg_regs();
-    __ ret(0);
 
 #undef dest
 #undef size

src/hotspot/os/windows/os_windows.cpp

@@ -2788,12 +2788,16 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) {
       }
 
       bool is_unsafe_arraycopy = (in_native || in_java) && UnsafeCopyMemory::contains_pc(pc);
-      if (((in_vm || in_native || is_unsafe_arraycopy) && thread->doing_unsafe_access()) ||
+      bool is_unsafe_setmemory = (in_native || in_java) && UnsafeSetMemory::contains_pc(pc);
+      if (((in_vm || in_native || is_unsafe_arraycopy || is_unsafe_setmemory) && thread->doing_unsafe_access()) ||
           (nm != nullptr && nm->has_unsafe_access())) {
         address next_pc =  Assembler::locate_next_instruction(pc);
         if (is_unsafe_arraycopy) {
           next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
         }
+        if (is_unsafe_setmemory) {
+          next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
+        }
         return Handle_Exception(exceptionInfo, SharedRuntime::handle_unsafe_access(thread, next_pc));
       }
     }

src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp

@@ -442,18 +442,21 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
         CodeBlob* cb = CodeCache::find_blob(pc);
         CompiledMethod* nm = (cb != nullptr) ? cb->as_compiled_method_or_null() : nullptr;
         bool is_unsafe_arraycopy = thread->doing_unsafe_access() && UnsafeCopyMemory::contains_pc(pc);
-        if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy) {
+        bool is_unsafe_setmemory = thread->doing_unsafe_access() && UnsafeSetMemory::contains_pc(pc);
+        if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy ||
+            is_unsafe_setmemory) {
           address next_pc = Assembler::locate_next_instruction(pc);
           if (is_unsafe_arraycopy) {
             next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
           }
+          if (is_unsafe_setmemory) {
+            next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
+          }
           stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
         }
-      }
-      else
-
+      } else
 #ifdef AMD64
-      if (sig == SIGFPE  &&
+      if (sig == SIGFPE &&
           (info->si_code == FPE_INTDIV || info->si_code == FPE_FLTDIV
            // Workaround for macOS ARM incorrectly reporting FPE_FLTINV for "div by 0"
            // instead of the expected FPE_FLTDIV when running x86_64 binary under Rosetta emulation
@@ -526,6 +529,9 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
         if (UnsafeCopyMemory::contains_pc(pc)) {
           next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
         }
+        if (UnsafeSetMemory::contains_pc(pc)) {
+          next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
+        }
         stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
     }
 

src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp

@@ -261,18 +261,21 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
         CodeBlob* cb = CodeCache::find_blob(pc);
         CompiledMethod* nm = (cb != nullptr) ? cb->as_compiled_method_or_null() : nullptr;
         bool is_unsafe_arraycopy = thread->doing_unsafe_access() && UnsafeCopyMemory::contains_pc(pc);
-        if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy) {
+        bool is_unsafe_setmemory = thread->doing_unsafe_access() && UnsafeSetMemory::contains_pc(pc);
+        if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy ||
+            is_unsafe_setmemory) {
           address next_pc = Assembler::locate_next_instruction(pc);
           if (is_unsafe_arraycopy) {
             next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
           }
+          if (is_unsafe_setmemory) {
+            next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
+          }
           stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
         }
-      }
-      else
-
+      } else
 #ifdef AMD64
-      if (sig == SIGFPE  &&
+      if (sig == SIGFPE &&
           (info->si_code == FPE_INTDIV || info->si_code == FPE_FLTDIV)) {
         stub =
           SharedRuntime::
@@ -320,6 +323,9 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
         if (UnsafeCopyMemory::contains_pc(pc)) {
           next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
         }
+        if (UnsafeSetMemory::contains_pc(pc)) {
+          next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
+        }
         stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
     }
 

src/hotspot/share/opto/library_call.cpp

@@ -4951,9 +4951,9 @@ bool LibraryCallKit::inline_unsafe_setMemory() {
   null_check_receiver();  // null-check receiver
   if (stopped())  return true;
 
-  C->set_has_unsafe_access(true);  // Mark eventual nmethod as "unsafe".
+  if (StubRoutines::unsafe_setmemory() == nullptr) return false;
 
-  // printf("In inline_unsafe_setMemory\n");
+  C->set_has_unsafe_access(true);  // Mark eventual nmethod as "unsafe".
 
   Node* dst_base =         argument(1);  // type: oop
   Node* dst_off  = ConvL2X(argument(2)); // type: long

src/hotspot/share/runtime/stubRoutines.cpp

@@ -45,6 +45,11 @@ int UnsafeCopyMemory::_table_length                             = 0;
 int UnsafeCopyMemory::_table_max_length                         = 0;
 address UnsafeCopyMemory::_common_exit_stub_pc                  = nullptr;
 
+UnsafeSetMemory* UnsafeSetMemory::_table                       = nullptr;
+int UnsafeSetMemory::_table_length                             = 0;
+int UnsafeSetMemory::_table_max_length                         = 0;
+address UnsafeSetMemory::_common_exit_stub_pc                  = nullptr;
+
 // Implementation of StubRoutines - for a description
 // of how to extend it, see the header file.
 
@@ -226,6 +231,31 @@ address UnsafeCopyMemory::page_error_continue_pc(address pc) {
   return nullptr;
 }
 
+void UnsafeSetMemory::create_table(int max_size) {
+  UnsafeSetMemory::_table = new UnsafeSetMemory[max_size];
+  UnsafeSetMemory::_table_max_length = max_size;
+}
+
+bool UnsafeSetMemory::contains_pc(address pc) {
+  for (int i = 0; i < UnsafeSetMemory::_table_length; i++) {
+    UnsafeSetMemory* entry = &UnsafeSetMemory::_table[i];
+    if (pc >= entry->start_pc() && pc < entry->end_pc()) {
+      return true;
+    }
+  }
+  return false;
+}
+
+address UnsafeSetMemory::page_error_continue_pc(address pc) {
+  for (int i = 0; i < UnsafeSetMemory::_table_length; i++) {
+    UnsafeSetMemory* entry = &UnsafeSetMemory::_table[i];
+    if (pc >= entry->start_pc() && pc < entry->end_pc()) {
+      return entry->error_exit_pc();
+    }
+  }
+  return nullptr;
+}
+
 
 static BufferBlob* initialize_stubs(StubCodeGenerator::StubsKind kind,
                                     int code_size, int max_aligned_stubs,
@@ -539,3 +569,25 @@ UnsafeCopyMemoryMark::~UnsafeCopyMemoryMark() {
     }
   }
 }
+
+UnsafeSetMemoryMark::UnsafeSetMemoryMark(StubCodeGenerator* cgen, bool add_entry, bool continue_at_scope_end, address error_exit_pc) {
+  _cgen = cgen;
+  _ucm_entry = nullptr;
+  if (add_entry) {
+    address err_exit_pc = nullptr;
+    if (!continue_at_scope_end) {
+      err_exit_pc = error_exit_pc != nullptr ? error_exit_pc : UnsafeSetMemory::common_exit_stub_pc();
+    }
+    assert(err_exit_pc != nullptr || continue_at_scope_end, "error exit not set");
+    _ucm_entry = UnsafeSetMemory::add_to_table(_cgen->assembler()->pc(), nullptr, err_exit_pc);
+  }
+}
+
+UnsafeSetMemoryMark::~UnsafeSetMemoryMark() {
+  if (_ucm_entry != nullptr) {
+    _ucm_entry->set_end_pc(_cgen->assembler()->pc());
+    if (_ucm_entry->error_exit_pc() == nullptr) {
+      _ucm_entry->set_error_exit_pc(_cgen->assembler()->pc());
+    }
+  }
+}

src/hotspot/share/runtime/stubRoutines.hpp

@@ -122,6 +122,52 @@ class UnsafeCopyMemoryMark : public StackObj {
   ~UnsafeCopyMemoryMark();
 };
 
+class UnsafeSetMemory : public CHeapObj<mtCode> {
+ private:
+  address _start_pc;
+  address _end_pc;
+  address _error_exit_pc;
+ public:
+  static address           _common_exit_stub_pc;
+  static UnsafeSetMemory* _table;
+  static int               _table_length;
+  static int               _table_max_length;
+  UnsafeSetMemory() : _start_pc(nullptr), _end_pc(nullptr), _error_exit_pc(nullptr) {}
+  void    set_start_pc(address pc)      { _start_pc = pc; }
+  void    set_end_pc(address pc)        { _end_pc = pc; }
+  void    set_error_exit_pc(address pc) { _error_exit_pc = pc; }
+  address start_pc()      const { return _start_pc; }
+  address end_pc()        const { return _end_pc; }
+  address error_exit_pc() const { return _error_exit_pc; }
+
+  static void    set_common_exit_stub_pc(address pc) { _common_exit_stub_pc = pc; }
+  static address common_exit_stub_pc()               { return _common_exit_stub_pc; }
+
+  static UnsafeSetMemory* add_to_table(address start_pc, address end_pc, address error_exit_pc) {
+    guarantee(_table_length < _table_max_length, "Incorrect UnsafeSetMemory::_table_max_length");
+    UnsafeSetMemory* entry = &_table[_table_length];
+    entry->set_start_pc(start_pc);
+    entry->set_end_pc(end_pc);
+    entry->set_error_exit_pc(error_exit_pc);
+
+    _table_length++;
+    return entry;
+  }
+
+  static bool    contains_pc(address pc);
+  static address page_error_continue_pc(address pc);
+  static void    create_table(int max_size);
+};
+
+class UnsafeSetMemoryMark : public StackObj {
+ private:
+  UnsafeSetMemory*  _ucm_entry;
+  StubCodeGenerator* _cgen;
+ public:
+  UnsafeSetMemoryMark(StubCodeGenerator* cgen, bool add_entry, bool continue_at_scope_end, address error_exit_pc = nullptr);
+  ~UnsafeSetMemoryMark();
+};
+
 class StubRoutines: AllStatic {
 
  public: