Skip to content

Commit 9227a8d

Browse files
Milad FaV8 LUCI CQ
Milad Fa
authored and
V8 LUCI CQ
committed
PPC/s390: [regexp] Fix stack growth for global regexps
Port 3e3a027 Original Commit Message: Irregexp reentrancy (crrev.com/c/3162604) introduced a bug for global regexp execution in which each iteration would use a new stack region (i.e. we forgot to pop the regexp stack pointer when starting a new iteration). This CL fixes that by popping the stack pointer on the loop backedge. At a high level: - Initialize the backtrack_stackpointer earlier and avoid clobbering it by setup code. - Pop it on the loop backedge. - Slightly refactor Push/Pop operations to avoid unneeded memory accesses. [email protected], [email protected], [email protected], [email protected] BUG= LOG=N Change-Id: Iafe6814d3695e83fced6a46209accf5e712d56f6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3198391 Reviewed-by: Junliang Yan <[email protected]> Commit-Queue: Milad Fa <[email protected]> Cr-Commit-Position: refs/heads/main@{#77180}
1 parent 3cfb930 commit 9227a8d

4 files changed

Lines changed: 140 additions & 119 deletions

File tree

‎src/regexp/ppc/regexp-macro-assembler-ppc.cc‎

Lines changed: 69 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -665,26 +665,26 @@ void RegExpMacroAssemblerPPC::StoreRegExpStackPointerToMemory(
665665
__ StoreU64(src, MemOperand(scratch));
666666
}
667667

668-
void RegExpMacroAssemblerPPC::PushRegExpBasePointer(Register scratch1,
669-
Register scratch2) {
670-
LoadRegExpStackPointerFromMemory(scratch1);
668+
void RegExpMacroAssemblerPPC::PushRegExpBasePointer(Register stack_pointer,
669+
Register scratch) {
671670
ExternalReference ref =
672671
ExternalReference::address_of_regexp_stack_memory_top_address(isolate());
673-
__ mov(scratch2, Operand(ref));
674-
__ LoadU64(scratch2, MemOperand(scratch2));
675-
__ SubS64(scratch2, scratch1, scratch2);
676-
__ StoreU64(scratch2, MemOperand(frame_pointer(), kRegExpStackBasePointer));
672+
__ mov(scratch, Operand(ref));
673+
__ LoadU64(scratch, MemOperand(scratch));
674+
__ SubS64(scratch, stack_pointer, scratch);
675+
__ StoreU64(scratch, MemOperand(frame_pointer(), kRegExpStackBasePointer));
677676
}
678677

679-
void RegExpMacroAssemblerPPC::PopRegExpBasePointer(Register scratch1,
680-
Register scratch2) {
678+
void RegExpMacroAssemblerPPC::PopRegExpBasePointer(Register stack_pointer_out,
679+
Register scratch) {
681680
ExternalReference ref =
682681
ExternalReference::address_of_regexp_stack_memory_top_address(isolate());
683-
__ LoadU64(scratch1, MemOperand(frame_pointer(), kRegExpStackBasePointer));
684-
__ mov(scratch2, Operand(ref));
685-
__ LoadU64(scratch2, MemOperand(scratch2));
686-
__ AddS64(scratch1, scratch1, scratch2);
687-
StoreRegExpStackPointerToMemory(scratch1, scratch2);
682+
__ LoadU64(stack_pointer_out,
683+
MemOperand(frame_pointer(), kRegExpStackBasePointer));
684+
__ mov(scratch, Operand(ref));
685+
__ LoadU64(scratch, MemOperand(scratch));
686+
__ AddS64(stack_pointer_out, stack_pointer_out, scratch);
687+
StoreRegExpStackPointerToMemory(stack_pointer_out, scratch);
688688
}
689689

690690
Handle<HeapObject> RegExpMacroAssemblerPPC::GetCode(Handle<String> source) {
@@ -743,37 +743,44 @@ Handle<HeapObject> RegExpMacroAssemblerPPC::GetCode(Handle<String> source) {
743743
kBacktrackCount - kSystemPointerSize);
744744
__ push(r3); // The regexp stack base ptr.
745745

746+
// Initialize backtrack stack pointer. It must not be clobbered from here
747+
// on. Note the backtrack_stackpointer is callee-saved.
748+
STATIC_ASSERT(backtrack_stackpointer() == r29);
749+
LoadRegExpStackPointerFromMemory(backtrack_stackpointer());
750+
746751
// Store the regexp base pointer - we'll later restore it / write it to
747752
// memory when returning from this irregexp code object.
748-
PushRegExpBasePointer(r3, r4);
749-
750-
// Check if we have space on the stack for registers.
751-
Label stack_limit_hit;
752-
Label stack_ok;
753-
754-
ExternalReference stack_limit =
755-
ExternalReference::address_of_jslimit(isolate());
756-
__ mov(r3, Operand(stack_limit));
757-
__ LoadU64(r3, MemOperand(r3));
758-
__ sub(r3, sp, r3, LeaveOE, SetRC);
759-
// Handle it if the stack pointer is already below the stack limit.
760-
__ ble(&stack_limit_hit, cr0);
761-
// Check if there is room for the variable number of registers above
762-
// the stack limit.
763-
__ CmpU64(r3, Operand(num_registers_ * kSystemPointerSize), r0);
764-
__ bge(&stack_ok);
765-
// Exit with OutOfMemory exception. There is not enough space on the stack
766-
// for our working registers.
767-
__ li(r3, Operand(EXCEPTION));
768-
__ b(&return_r3);
769-
770-
__ bind(&stack_limit_hit);
771-
CallCheckStackGuardState(r3);
772-
__ cmpi(r3, Operand::Zero());
773-
// If returned value is non-zero, we exit with the returned value as result.
774-
__ bne(&return_r3);
753+
PushRegExpBasePointer(backtrack_stackpointer(), r4);
754+
755+
{
756+
// Check if we have space on the stack for registers.
757+
Label stack_limit_hit, stack_ok;
758+
759+
ExternalReference stack_limit =
760+
ExternalReference::address_of_jslimit(isolate());
761+
__ mov(r3, Operand(stack_limit));
762+
__ LoadU64(r3, MemOperand(r3));
763+
__ sub(r3, sp, r3, LeaveOE, SetRC);
764+
// Handle it if the stack pointer is already below the stack limit.
765+
__ ble(&stack_limit_hit, cr0);
766+
// Check if there is room for the variable number of registers above
767+
// the stack limit.
768+
__ CmpU64(r3, Operand(num_registers_ * kSystemPointerSize), r0);
769+
__ bge(&stack_ok);
770+
// Exit with OutOfMemory exception. There is not enough space on the stack
771+
// for our working registers.
772+
__ li(r3, Operand(EXCEPTION));
773+
__ b(&return_r3);
774+
775+
__ bind(&stack_limit_hit);
776+
CallCheckStackGuardState(r3);
777+
__ cmpi(r3, Operand::Zero());
778+
// If returned value is non-zero, we exit with the returned value as
779+
// result.
780+
__ bne(&return_r3);
775781

776-
__ bind(&stack_ok);
782+
__ bind(&stack_ok);
783+
}
777784

778785
// Allocate space on stack for registers.
779786
__ AddS64(sp, sp, Operand(-num_registers_ * kSystemPointerSize), r0);
@@ -800,18 +807,21 @@ Handle<HeapObject> RegExpMacroAssemblerPPC::GetCode(Handle<String> source) {
800807
// Initialize code pointer register
801808
__ mov(code_pointer(), Operand(masm_->CodeObject()));
802809

803-
Label load_char_start_regexp, start_regexp;
804-
// Load newline if index is at start, previous character otherwise.
805-
__ cmpi(r4, Operand::Zero());
806-
__ bne(&load_char_start_regexp);
807-
__ li(current_character(), Operand('\n'));
808-
__ b(&start_regexp);
809-
810-
// Global regexp restarts matching here.
811-
__ bind(&load_char_start_regexp);
812-
// Load previous char as initial value of current character register.
813-
LoadCurrentCharacterUnchecked(-1, 1);
814-
__ bind(&start_regexp);
810+
Label load_char_start_regexp;
811+
{
812+
Label start_regexp;
813+
// Load newline if index is at start, previous character otherwise.
814+
__ cmpi(r4, Operand::Zero());
815+
__ bne(&load_char_start_regexp);
816+
__ li(current_character(), Operand('\n'));
817+
__ b(&start_regexp);
818+
819+
// Global regexp restarts matching here.
820+
__ bind(&load_char_start_regexp);
821+
// Load previous char as initial value of current character register.
822+
LoadCurrentCharacterUnchecked(-1, 1);
823+
__ bind(&start_regexp);
824+
}
815825

816826
// Initialize on-stack registers.
817827
if (num_saved_registers_ > 0) { // Always is, if generated from a regexp.
@@ -833,9 +843,6 @@ Handle<HeapObject> RegExpMacroAssemblerPPC::GetCode(Handle<String> source) {
833843
}
834844
}
835845

836-
// Initialize backtrack stack pointer.
837-
LoadRegExpStackPointerFromMemory(backtrack_stackpointer());
838-
839846
__ b(&start_label_);
840847

841848
// Exit code:
@@ -906,6 +913,10 @@ Handle<HeapObject> RegExpMacroAssemblerPPC::GetCode(Handle<String> source) {
906913
// Prepare r3 to initialize registers with its value in the next run.
907914
__ LoadU64(r3, MemOperand(frame_pointer(), kStringStartMinusOne));
908915

916+
// Restore the original regexp stack pointer value (effectively, pop the
917+
// stored base pointer).
918+
PopRegExpBasePointer(backtrack_stackpointer(), r5);
919+
909920
if (global_with_zero_length_check()) {
910921
// Special case for zero-length matches.
911922
// r25: capture start index
@@ -938,7 +949,7 @@ Handle<HeapObject> RegExpMacroAssemblerPPC::GetCode(Handle<String> source) {
938949
__ bind(&return_r3);
939950
// Restore the original regexp stack pointer value (effectively, pop the
940951
// stored base pointer).
941-
PopRegExpBasePointer(r4, r5);
952+
PopRegExpBasePointer(backtrack_stackpointer(), r5);
942953

943954
// Skip sp past regexp registers and local variables..
944955
__ mr(sp, frame_pointer());

‎src/regexp/ppc/regexp-macro-assembler-ppc.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerPPC
179179

180180
void LoadRegExpStackPointerFromMemory(Register dst);
181181
void StoreRegExpStackPointerToMemory(Register src, Register scratch);
182-
void PushRegExpBasePointer(Register scratch1, Register scratch2);
183-
void PopRegExpBasePointer(Register scratch1, Register scratch2);
182+
void PushRegExpBasePointer(Register stack_pointer, Register scratch);
183+
void PopRegExpBasePointer(Register stack_pointer_out, Register scratch);
184184

185185
Isolate* isolate() const { return masm_->isolate(); }
186186

‎src/regexp/s390/regexp-macro-assembler-s390.cc‎

Lines changed: 67 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -642,26 +642,26 @@ void RegExpMacroAssemblerS390::StoreRegExpStackPointerToMemory(
642642
__ StoreU64(src, MemOperand(scratch));
643643
}
644644

645-
void RegExpMacroAssemblerS390::PushRegExpBasePointer(Register scratch1,
646-
Register scratch2) {
647-
LoadRegExpStackPointerFromMemory(scratch1);
645+
void RegExpMacroAssemblerS390::PushRegExpBasePointer(Register stack_pointer,
646+
Register scratch) {
648647
ExternalReference ref =
649648
ExternalReference::address_of_regexp_stack_memory_top_address(isolate());
650-
__ mov(scratch2, Operand(ref));
651-
__ LoadU64(scratch2, MemOperand(scratch2));
652-
__ SubS64(scratch2, scratch1, scratch2);
653-
__ StoreU64(scratch2, MemOperand(frame_pointer(), kRegExpStackBasePointer));
649+
__ mov(scratch, Operand(ref));
650+
__ LoadU64(scratch, MemOperand(scratch));
651+
__ SubS64(scratch, stack_pointer, scratch);
652+
__ StoreU64(scratch, MemOperand(frame_pointer(), kRegExpStackBasePointer));
654653
}
655654

656-
void RegExpMacroAssemblerS390::PopRegExpBasePointer(Register scratch1,
657-
Register scratch2) {
655+
void RegExpMacroAssemblerS390::PopRegExpBasePointer(Register stack_pointer_out,
656+
Register scratch) {
658657
ExternalReference ref =
659658
ExternalReference::address_of_regexp_stack_memory_top_address(isolate());
660-
__ LoadU64(scratch1, MemOperand(frame_pointer(), kRegExpStackBasePointer));
661-
__ mov(scratch2, Operand(ref));
662-
__ LoadU64(scratch2, MemOperand(scratch2));
663-
__ AddS64(scratch1, scratch1, scratch2);
664-
StoreRegExpStackPointerToMemory(scratch1, scratch2);
659+
__ LoadU64(stack_pointer_out,
660+
MemOperand(frame_pointer(), kRegExpStackBasePointer));
661+
__ mov(scratch, Operand(ref));
662+
__ LoadU64(scratch, MemOperand(scratch));
663+
__ AddS64(stack_pointer_out, stack_pointer_out, scratch);
664+
StoreRegExpStackPointerToMemory(stack_pointer_out, scratch);
665665
}
666666

667667
Handle<HeapObject> RegExpMacroAssemblerS390::GetCode(Handle<String> source) {
@@ -728,37 +728,43 @@ Handle<HeapObject> RegExpMacroAssemblerS390::GetCode(Handle<String> source) {
728728
kBacktrackCount - kSystemPointerSize);
729729
__ push(r1); // The regexp stack base ptr.
730730

731+
// Initialize backtrack stack pointer. It must not be clobbered from here on.
732+
// Note the backtrack_stackpointer is callee-saved.
733+
STATIC_ASSERT(backtrack_stackpointer() == r13);
734+
LoadRegExpStackPointerFromMemory(backtrack_stackpointer());
735+
731736
// Store the regexp base pointer - we'll later restore it / write it to
732737
// memory when returning from this irregexp code object.
733-
PushRegExpBasePointer(r2, r3);
734-
735-
// Check if we have space on the stack for registers.
736-
Label stack_limit_hit;
737-
Label stack_ok;
738+
PushRegExpBasePointer(backtrack_stackpointer(), r3);
739+
740+
{
741+
// Check if we have space on the stack for registers.
742+
Label stack_limit_hit, stack_ok;
743+
744+
ExternalReference stack_limit =
745+
ExternalReference::address_of_jslimit(isolate());
746+
__ mov(r2, Operand(stack_limit));
747+
__ LoadU64(r2, MemOperand(r2));
748+
__ SubS64(r2, sp, r2);
749+
// Handle it if the stack pointer is already below the stack limit.
750+
__ ble(&stack_limit_hit);
751+
// Check if there is room for the variable number of registers above
752+
// the stack limit.
753+
__ CmpU64(r2, Operand(num_registers_ * kSystemPointerSize));
754+
__ bge(&stack_ok);
755+
// Exit with OutOfMemory exception. There is not enough space on the stack
756+
// for our working registers.
757+
__ mov(r2, Operand(EXCEPTION));
758+
__ b(&return_r2);
738759

739-
ExternalReference stack_limit =
740-
ExternalReference::address_of_jslimit(isolate());
741-
__ mov(r2, Operand(stack_limit));
742-
__ LoadU64(r2, MemOperand(r2));
743-
__ SubS64(r2, sp, r2);
744-
// Handle it if the stack pointer is already below the stack limit.
745-
__ ble(&stack_limit_hit);
746-
// Check if there is room for the variable number of registers above
747-
// the stack limit.
748-
__ CmpU64(r2, Operand(num_registers_ * kSystemPointerSize));
749-
__ bge(&stack_ok);
750-
// Exit with OutOfMemory exception. There is not enough space on the stack
751-
// for our working registers.
752-
__ mov(r2, Operand(EXCEPTION));
753-
__ b(&return_r2);
754-
755-
__ bind(&stack_limit_hit);
756-
CallCheckStackGuardState(r2);
757-
__ CmpS64(r2, Operand::Zero());
758-
// If returned value is non-zero, we exit with the returned value as result.
759-
__ bne(&return_r2);
760+
__ bind(&stack_limit_hit);
761+
CallCheckStackGuardState(r2);
762+
__ CmpS64(r2, Operand::Zero());
763+
// If returned value is non-zero, we exit with the returned value as result.
764+
__ bne(&return_r2);
760765

761-
__ bind(&stack_ok);
766+
__ bind(&stack_ok);
767+
}
762768

763769
// Allocate space on stack for registers.
764770
__ lay(sp, MemOperand(sp, (-num_registers_ * kSystemPointerSize)));
@@ -786,18 +792,21 @@ Handle<HeapObject> RegExpMacroAssemblerS390::GetCode(Handle<String> source) {
786792
// Initialize code pointer register
787793
__ mov(code_pointer(), Operand(masm_->CodeObject()));
788794

789-
Label load_char_start_regexp, start_regexp;
790-
// Load newline if index is at start, previous character otherwise.
791-
__ CmpS64(r3, Operand::Zero());
792-
__ bne(&load_char_start_regexp);
793-
__ mov(current_character(), Operand('\n'));
794-
__ b(&start_regexp);
795-
796-
// Global regexp restarts matching here.
797-
__ bind(&load_char_start_regexp);
798-
// Load previous char as initial value of current character register.
799-
LoadCurrentCharacterUnchecked(-1, 1);
800-
__ bind(&start_regexp);
795+
Label load_char_start_regexp;
796+
{
797+
Label start_regexp;
798+
// Load newline if index is at start, previous character otherwise.
799+
__ CmpS64(r3, Operand::Zero());
800+
__ bne(&load_char_start_regexp);
801+
__ mov(current_character(), Operand('\n'));
802+
__ b(&start_regexp);
803+
804+
// Global regexp restarts matching here.
805+
__ bind(&load_char_start_regexp);
806+
// Load previous char as initial value of current character register.
807+
LoadCurrentCharacterUnchecked(-1, 1);
808+
__ bind(&start_regexp);
809+
}
801810

802811
// Initialize on-stack registers.
803812
if (num_saved_registers_ > 0) { // Always is, if generated from a regexp.
@@ -819,9 +828,6 @@ Handle<HeapObject> RegExpMacroAssemblerS390::GetCode(Handle<String> source) {
819828
}
820829
}
821830

822-
// Initialize backtrack stack pointer.
823-
LoadRegExpStackPointerFromMemory(backtrack_stackpointer());
824-
825831
__ b(&start_label_);
826832

827833
// Exit code:
@@ -914,6 +920,10 @@ Handle<HeapObject> RegExpMacroAssemblerS390::GetCode(Handle<String> source) {
914920
// Prepare r2 to initialize registers with its value in the next run.
915921
__ LoadU64(r2, MemOperand(frame_pointer(), kStringStartMinusOne));
916922

923+
// Restore the original regexp stack pointer value (effectively, pop the
924+
// stored base pointer).
925+
PopRegExpBasePointer(backtrack_stackpointer(), r4);
926+
917927
if (global_with_zero_length_check()) {
918928
// Special case for zero-length matches.
919929
// r6: capture start index
@@ -945,7 +955,7 @@ Handle<HeapObject> RegExpMacroAssemblerS390::GetCode(Handle<String> source) {
945955
__ bind(&return_r2);
946956
// Restore the original regexp stack pointer value (effectively, pop the
947957
// stored base pointer).
948-
PopRegExpBasePointer(r3, r4);
958+
PopRegExpBasePointer(backtrack_stackpointer(), r4);
949959

950960
// Skip sp past regexp registers and local variables..
951961
__ mov(sp, frame_pointer());

‎src/regexp/s390/regexp-macro-assembler-s390.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerS390
177177

178178
void LoadRegExpStackPointerFromMemory(Register dst);
179179
void StoreRegExpStackPointerToMemory(Register src, Register scratch);
180-
void PushRegExpBasePointer(Register scratch1, Register scratch2);
181-
void PopRegExpBasePointer(Register scratch1, Register scratch2);
180+
void PushRegExpBasePointer(Register stack_pointer, Register scratch);
181+
void PopRegExpBasePointer(Register stack_pointer_out, Register scratch);
182182

183183
Isolate* isolate() const { return masm_->isolate(); }
184184

0 commit comments

Comments
 (0)