Bug 1676292 - mozjs60: fails when built with O2 on rawhide/f30 on aarch64
Summary: mozjs60: fails when built with O2 on rawhide/f30 on aarch64
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: mozjs60
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: František Zatloukal
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F30FTBFS
TreeView+ depends on / blocked
 
Reported: 2019-02-11 22:30 UTC by Fedora Release Engineering
Modified: 2019-04-15 19:48 UTC (History)
5 users (show)

Fixed In Version: mozjs60-60.6.1-2.fc31
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-04-15 19:48:39 UTC


Attachments (Terms of Use)
build.log (1.00 KB, text/plain)
2019-02-11 22:30 UTC, Fedora Release Engineering
no flags Details
root.log (1.00 KB, text/plain)
2019-02-11 22:30 UTC, Fedora Release Engineering
no flags Details
state.log (625 bytes, text/plain)
2019-02-11 22:30 UTC, Fedora Release Engineering
no flags Details


Links
System ID Priority Status Summary Last Updated
Mozilla Foundation 1527311 None None None 2019-07-12 12:03:10 UTC

Description Fedora Release Engineering 2019-02-11 22:30:45 UTC
mozjs60 failed to build from source in Fedora rawhide/f30

https://koji.fedoraproject.org/koji/taskinfo?taskID=32616800


For details on the mass rebuild see:

https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild
Please fix mozjs60 at your earliest convenience and set the bug's status to
ASSIGNED when you start fixing it. If the bug remains in NEW state for 8 weeks,
mozjs60 will be orphaned. Before branching of Fedora 31,
mozjs60 will be retired, if it still fails to build.

For more details on the FTBFS policy, please visit:
https://fedoraproject.org/wiki/Fails_to_build_from_source

Comment 1 Fedora Release Engineering 2019-02-11 22:30:48 UTC
Created attachment 1533808 [details]
build.log

file build.log too big, will only attach last 1024 bytes

Comment 2 Fedora Release Engineering 2019-02-11 22:30:50 UTC
Created attachment 1533809 [details]
root.log

file root.log too big, will only attach last 1024 bytes

Comment 3 Fedora Release Engineering 2019-02-11 22:30:51 UTC
Created attachment 1533810 [details]
state.log

Comment 4 Jakub Jelinek 2019-02-13 15:53:18 UTC
Does it work if you build with -O0 instead of -O2?  If so, can you do one -O2 build that fails, another one in another directory with -O0, make a copy of one of the trees and bisect which object file matters
(as in, ideally if all objects are built with -O0 and one with -O2 the result fails, and if that same is built with -O0 and all other -O2 it succeeds)?
If yes, preprocessed source + command line options for that source file would be appreciated.

Comment 5 František Zatloukal 2019-02-14 12:03:46 UTC
So, building with O0 fixed it on aarch64. I'll push the change to fix the FTBFS state for now.

Scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=32801426

Comment 6 Jakub Jelinek 2019-02-14 12:07:35 UTC
Well, that is not the fix, that is just a workaround.  This needs to be analyzed, it can be a package bug (undefined behavior somewhere) or compiler bug, the above method allows a brute force way to determine where the problem roughly is.
As I'm not really familiar with this package, it is better if it is done by somebody who is familiar with it.

Comment 7 František Zatloukal 2019-02-14 12:12:34 UTC
Don't worry, I am planning to continue debugging it :) . It just takes some time to compile on my laptop (especially because it's broken only on aarch64). I'll post here the results when I am done.

Comment 8 Igor Gnatenko 2019-02-16 17:19:30 UTC
There has been at least one successfull build after mass rebuild.

mozjs60-60.4.0-3.fc30: https://koji.fedoraproject.org/koji/buildinfo?buildID=1209941

Comment 9 Jakub Jelinek 2019-02-18 11:35:38 UTC
Just because that build forces -O0 on aarch64.

Comment 10 Jakub Jelinek 2019-02-18 11:40:12 UTC
I've reproduced it, but have hard time finding out what actually aborts.
If I try to run manually what I saw in strace, like:
LD_LIBRARY_PATH=/builddir/build/BUILD/firefox-60.4.0/js/src/js/src/shell /builddir/build/BUILD/firefox-60.4.0/js/src/js/src/shell/js --no-baseline -e 'var xulRuntime = { OS: "Linux", XPCOMABI: "aarch64-gcc3", shell: true };var release_or_beta = getBuildConfiguration().release_or_beta;var isDebugBuild=false; var Android=false; var browserIsRemote=false'
then it works and finishes almost instantly, while when /usr/bin/python2 tests/jstests.py -d -s -t 1800 ../../js/src/js/src/shell/js is run, it fails as you wrote.

Comment 11 Jakub Jelinek 2019-02-18 15:28:27 UTC
Bisection points to js/src/js/src/Unified_cpp_js_src37.o, if it is from -O0 build, tests work even if all other objects are from -O2 build, if it is from -O2 build, then it fails even if all other objects are from -O0 build.

Comment 12 Jakub Jelinek 2019-02-18 22:10:18 UTC
Seems this has changed on that file with http://gcc.gnu.org/r266385 .  Further investigation tomorrow.

Comment 13 Jakub Jelinek 2019-02-19 14:46:05 UTC
In-file bisection points to a difference in js::RegExpShared::execute(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*).
.s7 is what "works", .s6 is what breaks.  I'm afraid I can't progress further without hints on what actually is called during the testing and how can I step through it in the debugger,
watch how many times this function is called and which of the calls is problematic.
The register allocation is slightly different, sure, but I haven't spotted any obvious issues.

--- Unified_cpp_js_src37.s7	2019-02-19 15:37:00.446116127 +0100
+++ Unified_cpp_js_src37.s6	2019-02-19 15:30:19.171655019 +0100
@@ -6016,15 +6016,15 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	stp	x25, x26, [sp, 64]
 .LCFI309:
 	mov	x25, x5
-	cset	w26, eq
+	mov	x23, x1
 	stp	x27, x28, [sp, 80]
 .LCFI310:
-	mov	x23, x1
-	mov	w3, w26
+	cset	w27, eq
+	mov	w3, w27
 	ldr	x5, [x4]
 	str	x5, [sp, 248]
 	mov	x5,0
-	mov	w28, 0
+	mov	w26, 0
 	mov	w4, 0
 	bl	_ZN2js12RegExpShared18compileIfNecessaryEP9JSContextN2JS13MutableHandleIPS0_EENS3_6HandleIP14JSLinearStringEENS0_15CompilationModeENS0_17ForceByteCodeEnumE
 	tst	w0, 255
@@ -6039,11 +6039,11 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	beq	.L1482
 .L1408:
 	ldr	x2, [x21]
-	add	x27, sp, 128
-	mov	x0, x27
+	add	x28, sp, 128
+	mov	x0, x28
 	mov	x1, x24
-	ldr	w28, [x2, 4]
-	str	x28, [sp, 96]
+	ldr	w26, [x2, 4]
+	str	x26, [sp, 96]
 	bl	_ZN2js8irregexp16RegExpStackScopeC1EP9JSContext
 	ldr	x0, [x23]
 	ldrb	w1, [x0, 9]
@@ -6054,16 +6054,16 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	uxtw	x23, w24
 	tbz	x0, 3, .L1410
 	adds	x23, x22, x23
-	ccmp	x23, x28, 2, cc
+	ccmp	x23, x26, 2, cc
 	bls	.L1413
 .L1415:
-	mov	w28, 2
+	mov	w26, 2
 .L1414:
-	mov	x0, x27
+	mov	x0, x28
 	bl	_ZN2js8irregexp16RegExpStackScopeD1Ev
 .L1406:
 	ldr	x20, [x20, #:got_lo12:__stack_chk_guard]
-	mov	w0, w28
+	mov	w0, w26
 	ldr	x2, [sp, 248]
 	ldr	x1, [x20]
 	eor	x1, x2, x1
@@ -6083,38 +6083,38 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	ldr	w1, [x1]
 	and	w1, w1, 64
 	cmp	w1, 0
-	cset	w3, eq
+	cset	w26, eq
 	cbnz	x19, .L1420
-	add	w3, w3, 2
+	add	w26, w26, 2
 .L1420:
-	add	x28, x0, x3, sxtw 4
-	ldr	x4, [x28, 24]
-	cbz	x4, .L1421
-	and	x8, x4, -4096
-	ldr	x7, [x8, 8]
-	ldr	w0, [x7, 16]
+	add	x26, x0, x26, sxtw 4
+	ldr	x3, [x26, 24]
+	cbz	x3, .L1421
+	and	x8, x3, -4096
+	ldr	x6, [x8, 8]
+	ldr	w0, [x6, 16]
 	cbnz	w0, .L1484
 .L1422:
-	and	x0, x4, -1048576
+	and	x0, x3, -1048576
 	mov	x1, 49312
 	movk	x1, 0xf, lsl 16
-	ubfx	x2, x4, 9, 11
+	ubfx	x2, x3, 9, 11
 	orr	x0, x0, x1
-	ubfx	w1, w4, 3, 6
-	mov	x5, 1
-	ubfx	x6, x4, 3, 17
-	lsl	x1, x5, x1
+	ubfx	w1, w3, 3, 6
+	mov	x4, 1
+	ubfx	x5, x3, 3, 17
+	lsl	x1, x4, x1
 	ldr	x2, [x0, x2, lsl 3]
 	tst	x1, x2
 	bne	.L1481
-	add	x1, x6, 1
+	add	x1, x5, 1
 	lsr	x2, x1, 6
-	lsl	x5, x5, x1
+	lsl	x4, x4, x1
 	ldr	x0, [x0, x2, lsl 3]
-	tst	x5, x0
+	tst	x4, x0
 	bne	.L1425
 .L1481:
-	ldr	x1, [x28, 24]
+	ldr	x1, [x26, 24]
 	cbz	x1, .L1421
 	ldr	x2, [x21]
 	ldr	w0, [x2]
@@ -6134,7 +6134,7 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	cmp	w0, 2
 	beq	.L1415
 .L1417:
-	mov	w28, 1
+	mov	w26, 1
 	b	.L1414
 	.align	2
 .L1486:
@@ -6143,7 +6143,7 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	tst	w0, 255
 	beq	.L1434
 .L1421:
-	mov	w3, w26
+	mov	w3, w27
 	mov	x2, x21
 	mov	x1, x23
 	mov	x0, x24
@@ -6152,7 +6152,7 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	tst	w0, 255
 	bne	.L1487
 .L1434:
-	mov	w28, 0
+	mov	w26, 0
 	b	.L1414
 	.align	2
 .L1413:
@@ -6164,7 +6164,7 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	cbz	x19, .L1416
 	ldr	x0, [x19, 8]
 	add	w24, w24, w22
-	mov	w28, 1
+	mov	w26, 1
 	str	w22, [x0]
 	ldr	x0, [x19, 8]
 	str	w24, [x0, 4]
@@ -6179,7 +6179,7 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	cbz	x19, .L1418
 	ldr	x1, [x19, 8]
 	add	w24, w0, w24
-	mov	w28, 1
+	mov	w26, 1
 	str	w0, [x1]
 	ldr	x0, [x19, 8]
 	str	w24, [x0, 4]
@@ -6216,15 +6216,15 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	add	w1, w1, 2
 .L1436:
 	add	x1, x2, x1, sxtw 4
-	add	x26, sp, 144
+	add	x23, sp, 144
 	ldr	x3, [x24, 64]
 	add	x4, x24, 64
-	ldr	x23, [x1, 32]
-	str	x26, [x24, 64]
-	mov	x0, x26
+	ldr	x27, [x1, 32]
+	str	x23, [x24, 64]
+	mov	x0, x23
 	mov	x1, x24
 	ldr	x2, [x21]
-	mov	w28, 0
+	mov	w26, 0
 	stp	x4, x3, [sp, 144]
 	str	xzr, [sp, 160]
 	strb	wzr, [sp, 232]
@@ -6237,13 +6237,13 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	mov	x5, x19
 	mov	x3, x22
 	cmp	w0, 1
-	mov	x1, x23
+	mov	x1, x27
 	mov	x0, x24
 	ldr	x4, [sp, 96]
 	ldr	x2, [sp, 168]
 	beq	.L1489
 	bl	_ZN2js8irregexp13InterpretCodeIDsEENS_15RegExpRunStatusEP9JSContextPKhPKT_mmPNS_10MatchPairsEPm
-	mov	w28, w0
+	mov	w26, w0
 .L1437:
 	ldrb	w0, [sp, 232]
 	cbnz	w0, .L1490
@@ -6253,58 +6253,58 @@ _ZN2js12RegExpShared7executeEP9JSContext
 	b	.L1414
 	.align	2
 .L1425:
-	mov	x0, x7
-	stp	x4, x8, [sp, 104]
+	mov	x0, x6
+	stp	x3, x8, [sp, 104]
 	bl	_ZN2js55RuntimeFromActiveCooperatingThreadIsHeapMajorCollectingEPN2JS6shadow4ZoneE
 	tst	w0, 255
-	ldp	x4, x8, [sp, 104]
+	ldp	x3, x8, [sp, 104]
 	bne	.L1481
 	ldrb	w1, [x8, 24]
 	adrp	x0, .LANCHOR2
 	add	x0, x0, :lo12:.LANCHOR2
 	ldr	w0, [x0, x1, lsl 2]
 	and	x0, x0, 7
-	orr	x0, x4, x0
+	orr	x0, x3, x0
 	bl	_ZN2JS28UnmarkGrayGCThingRecursivelyENS_9GCCellPtrE
 	b	.L1481
 	.align	2
 .L1484:
-	ldr	x0, [x7, 8]
+	ldr	x0, [x6, 8]
 	adrp	x2, .LANCHOR0
 	add	x2, x2, :lo12:.LANCHOR0
 	add	x1, sp, 136
 	add	x2, x2, 272
-	stp	x4, x7, [sp, 104]
+	stp	x3, x6, [sp, 104]
 	str	x8, [sp, 120]
-	str	x4, [sp, 136]
+	str	x3, [sp, 136]
 	bl	_ZN2js40TraceManuallyBarrieredGenericPointerEdgeEP8JSTracerPPNS_2gc4CellEPKc
-	ldp	x4, x7, [sp, 104]
+	ldp	x3, x6, [sp, 104]
 	ldr	x8, [sp, 120]
 	b	.L1422
 	.align	2
 .L1418:
 	cbz	x25, .L1417
 	add	x0, x23, x0, sxtw
-	mov	w28, 1
+	mov	w26, 1
 	str	x0, [x25]
 	b	.L1414
 	.align	2
 .L1416:
 	cbz	x25, .L1417
-	mov	w28, 1
+	mov	w26, 1
 	str	x23, [x25]
 	b	.L1414
 	.align	2
 .L1490:
 	ldr	x0, [sp, 184]
-	add	x26, x26, 64
-	cmp	x0, x26
+	add	x23, x23, 64
+	cmp	x0, x23
 	beq	.L1439
 	bl	free
 	b	.L1439
 .L1489:
 	bl	_ZN2js8irregexp13InterpretCodeIhEENS_15RegExpRunStatusEP9JSContextPKhPKT_mmPNS_10MatchPairsEPm
-	mov	w28, w0
+	mov	w26, w0
 	b	.L1437
 .L1483:
 	bl	__stack_chk_fail

The above has been created with gcc r266385 patched with following patch to make it bisection friendly with old vs. new ira-color.c changes:
--- gcc/ira-costs.c.jj	2019-02-19 12:48:20.630639363 +0100
+++ gcc/ira-costs.c	2019-02-19 12:52:04.252968637 +0100
@@ -1276,6 +1276,8 @@ record_address_regs (machine_mode mode,
 }
 
 and
IRA_COSTS=80 ./cc1plus -fpreprocessed Unified_cpp_js_src37.ii -quiet -dumpbase Unified_cpp_js_src37.ii -mlittle-endian -mabi=lp64 -auxbase-strip Unified_cpp_js_src37.o -g -grecord-gcc-switches -g -O2 -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wwrite-strings -Wno-invalid-offsetof -Wc++17-compat -Wduplicated-cond -Wimplicit-fallthrough=3 -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat=1 -Wformat-overflow=2 -Wno-noexcept-type -Wall -Werror=format-security -Wno-shadow -Werror=format -version -fPIC -fno-sized-deallocation -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-tree-vrp -fno-delete-null-pointer-checks -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -fno-omit-frame-pointer -fno-strict-aliasing -o Unified_cpp_js_src37.s7 -g0 -fno-asynchronous-unwind-tables
for works version,
IRA_COSTS=79 ./cc1plus -fpreprocessed Unified_cpp_js_src37.ii -quiet -dumpbase Unified_cpp_js_src37.ii -mlittle-endian -mabi=lp64 -auxbase-strip Unified_cpp_js_src37.o -g -grecord-gcc-switches -g -O2 -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wwrite-strings -Wno-invalid-offsetof -Wc++17-compat -Wduplicated-cond -Wimplicit-fallthrough=3 -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat=1 -Wformat-overflow=2 -Wno-noexcept-type -Wall -Werror=format-security -Wno-shadow -Werror=format -version -fPIC -fno-sized-deallocation -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-tree-vrp -fno-delete-null-pointer-checks -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -fno-omit-frame-pointer -fno-strict-aliasing -o Unified_cpp_js_src37.s6 -g0 -fno-asynchronous-unwind-tables
for doesn't work version.

+static int xcntx = -1;
+static int xlimitx = -1;
 
 /* Calculate the costs of insn operands.  */
 static void
@@ -1283,10 +1285,13 @@ record_operand_costs (rtx_insn *insn, en
 {
   const char *constraints[MAX_RECOG_OPERANDS];
   machine_mode modes[MAX_RECOG_OPERANDS];
+  rtx ops[MAX_RECOG_OPERANDS];
   rtx set;
   int i;
+  bool old = xcntx < xlimitx;
 
-  if ((set = single_set (insn)) != NULL_RTX
+  if (!old
+      && (set = single_set (insn)) != NULL_RTX
       /* In rare cases the single set insn might have less 2 operands
 	 as the source can be a fixed special reg.  */
       && recog_data.n_operands > 1
@@ -1392,6 +1397,7 @@ record_operand_costs (rtx_insn *insn, en
     {
       memcpy (op_costs[i], init_cost, struct_costs_size);
 
+      ops[i] = recog_data.operand[i];
       if (GET_CODE (recog_data.operand[i]) == SUBREG)
 	recog_data.operand[i] = SUBREG_REG (recog_data.operand[i]);
 
@@ -1431,6 +1437,70 @@ record_operand_costs (rtx_insn *insn, en
   record_reg_classes (recog_data.n_alternatives, recog_data.n_operands,
 		      recog_data.operand, modes,
 		      constraints, insn, pref);
+
+  /* If this insn is a single set copying operand 1 to operand 0 and
+     one operand is an allocno with the other a hard reg or an allocno
+     that prefers a hard register that is in its own register class
+     then we may want to adjust the cost of that register class to -1.
+
+     Avoid the adjustment if the source does not die to avoid
+     stressing of register allocator by preferencing two colliding
+     registers into single class.
+
+     Also avoid the adjustment if a copy between hard registers of the
+     class is expensive (ten times the cost of a default copy is
+     considered arbitrarily expensive).  This avoids losing when the
+     preferred class is very expensive as the source of a copy
+     instruction.  */
+  if (old
+      && (set = single_set (insn)) != NULL_RTX
+      /* In rare cases the single set insn might have less 2 operands
+	 as the source can be a fixed special reg.  */
+      && recog_data.n_operands > 1
+      && ops[0] == SET_DEST (set) && ops[1] == SET_SRC (set))
+    {
+      int regno, other_regno;
+      rtx dest = SET_DEST (set);
+      rtx src = SET_SRC (set);
+
+      if (GET_CODE (dest) == SUBREG
+	  && known_eq (GET_MODE_SIZE (GET_MODE (dest)),
+		       GET_MODE_SIZE (GET_MODE (SUBREG_REG (dest)))))
+	dest = SUBREG_REG (dest);
+      if (GET_CODE (src) == SUBREG
+	  && known_eq (GET_MODE_SIZE (GET_MODE (src)),
+		       GET_MODE_SIZE (GET_MODE (SUBREG_REG (src)))))
+	src = SUBREG_REG (src);
+      if (REG_P (src) && REG_P (dest)
+	  && find_regno_note (insn, REG_DEAD, REGNO (src))
+	  && (((regno = REGNO (src)) >= FIRST_PSEUDO_REGISTER
+	       && (other_regno = REGNO (dest)) < FIRST_PSEUDO_REGISTER)
+	      || ((regno = REGNO (dest)) >= FIRST_PSEUDO_REGISTER
+		  && (other_regno = REGNO (src)) < FIRST_PSEUDO_REGISTER)))
+	{
+	  machine_mode mode = GET_MODE (src);
+	  cost_classes_t cost_classes_ptr = regno_cost_classes[regno];
+	  enum reg_class *cost_classes = cost_classes_ptr->classes;
+	  reg_class_t rclass;
+	  int k;
+
+	  i = regno == (int) REGNO (src) ? 1 : 0;
+	  for (k = cost_classes_ptr->num - 1; k >= 0; k--)
+	    {
+	      rclass = cost_classes[k];
+	      if (TEST_HARD_REG_BIT (reg_class_contents[rclass], other_regno)
+		  && (reg_class_size[(int) rclass]
+		      == ira_reg_class_max_nregs [(int) rclass][(int) mode]))
+		{
+		  if (reg_class_size[rclass] == 1)
+		    op_costs[i]->cost[k] = -frequency;
+		  else if (in_hard_reg_set_p (reg_class_contents[rclass],
+					      mode, other_regno))
+		    op_costs[i]->cost[k] = -frequency;
+		}
+	    }
+	}
+    }
 }
 
 

@@ -2273,6 +2343,8 @@ finish_costs (void)
 void
 ira_costs (void)
 {
+  if (xlimitx == -1 && getenv ("IRA_COSTS")) xlimitx = atoi (getenv ("IRA_COSTS"));
+  xcntx++;
   allocno_p = true;
   cost_elements_num = ira_allocnos_num;
   init_costs ();

Comment 14 Jakub Jelinek 2019-02-19 15:50:17 UTC
Seems it is
/builddir/build/BUILD/firefox-60.4.0/js/src/js/src/shell/js -f shell.js -f test262/shell.js -f test262/intl402/shell.js -f test262/intl402/DateTimeFormat/shell.js -f test262/intl402/DateTimeFormat/prototype/shell.js -f test262/intl402/DateTimeFormat/prototype/resolvedOptions/shell.js -f test262/intl402/DateTimeFormat/prototype/resolvedOptions/basic.js
that crashes and _ZN2js12RegExpShared7executeEP9JSContextN2JS13MutableHandleIPS0_EENS3_6HandleIP14JSLinearStringEEmPNS_16VectorMatchPairsEPm is called there just once.

Comment 15 Jakub Jelinek 2019-02-19 16:32:36 UTC
valgrind says:
==11791== Command: /builddir/build/BUILD/firefox-60.4.0/js/src/js/src/shell/js.bad -f shell.js -f test262/shell.js -f test262/intl402/shell.js -f test262/intl402/DateTimeFormat/shell.js -f test262/intl402/DateTimeFormat/prototype/shell.js -f test262/intl402/DateTimeFormat/prototype/resolvedOptions/shell.js -f test262/intl402/DateTimeFormat/prototype/resolvedOptions/basic.js
==11791== 
==11791== Warning: set address range perms: large range [0x34bfae521000, 0x34bfee521000) (noaccess)
==11791== Invalid read of size 8
==11791==    at 0x43F310: js::irregexp::RegExpStackScope::~RegExpStackScope() (RegExpStack.cpp:44)
==11791==    by 0xA54EDB: js::RegExpShared::execute(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*) (RegExpObject.cpp:1091)
==11791==    by 0x27BF63: ExecuteRegExpImpl(JSContext*, js::RegExpStatics*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*) (RegExp.cpp:126)
==11791==    by 0x2809BF: ExecuteRegExp(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSString*>, int, js::VectorMatchPairs*, unsigned long*) (RegExp.cpp:964)
==11791==    by 0x2812AF: js::RegExpTester(JSContext*, unsigned int, JS::Value*) (RegExp.cpp:1137)
==11791==    by 0x31FD63: CallJSNative (JSContext-inl.h:290)
==11791==    by 0x31FD63: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:468)
==11791==    by 0x320037: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:517)
==11791==    by 0x32007F: js::CallFromStack(JSContext*, JS::CallArgs const&) (Interpreter.cpp:523)
==11791==    by 0x32E5D3: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:3115)
==11791==    by 0x31F62B: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:418)
==11791==    by 0x31FE97: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:490)
==11791==    by 0x320037: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:517)
==11791==  Address 0x1ffeffaa30 is on thread 1's stack
==11791==  128 bytes below stack pointer
==11791== 
==11791== Invalid free() / delete / delete[] / realloc()
==11791==    at 0x48594E4: realloc (vg_replace_malloc.c:836)
==11791==    by 0x2449CB: SystemMalloc::realloc(void*, unsigned long) (malloc_decls.h:39)
==11791==    by 0x244BB3: DummyArenaAllocator<SystemMalloc>::moz_arena_realloc(unsigned long, void*, unsigned long) (malloc_decls.h:39)
==11791==    by 0x244AC7: moz_arena_realloc (malloc_decls.h:117)
==11791==    by 0x42947B: js_realloc(void*, unsigned long) (Utility.h:411)
==11791==    by 0x43F48B: js::irregexp::RegExpStack::reset() (RegExpStack.cpp:81)
==11791==    by 0x43F317: js::irregexp::RegExpStackScope::~RegExpStackScope() (RegExpStack.cpp:44)
==11791==    by 0xA54EDB: js::RegExpShared::execute(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*) (RegExpObject.cpp:1091)
==11791==    by 0x27BF63: ExecuteRegExpImpl(JSContext*, js::RegExpStatics*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*) (RegExp.cpp:126)
==11791==    by 0x2809BF: ExecuteRegExp(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSString*>, int, js::VectorMatchPairs*, unsigned long*) (RegExp.cpp:964)
==11791==    by 0x2812AF: js::RegExpTester(JSContext*, unsigned int, JS::Value*) (RegExp.cpp:1137)
==11791==    by 0x31FD63: CallJSNative (JSContext-inl.h:290)
==11791==    by 0x31FD63: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:468)
==11791==  Address 0x1ffeffabd0 is on thread 1's stack
==11791==  in frame #8, created by ExecuteRegExpImpl(JSContext*, js::RegExpStatics*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*) (RegExp.cpp:125)
==11791== 
while js.good (same except for the single function) doesn't show any errors.

Comment 16 Jakub Jelinek 2019-02-19 17:08:14 UTC
   0x000000000043f300 <js::irregexp::RegExpStackScope::~RegExpStackScope()+0>:  stp     x29, x30, [sp, #-32]!
   0x000000000043f304 <js::irregexp::RegExpStackScope::~RegExpStackScope()+4>:  mov     x29, sp
   0x000000000043f308 <js::irregexp::RegExpStackScope::~RegExpStackScope()+8>:  str     x0, [sp, #24]
   0x000000000043f30c <js::irregexp::RegExpStackScope::~RegExpStackScope()+12>: ldr     x0, [sp, #24]
=> 0x000000000043f310 <js::irregexp::RegExpStackScope::~RegExpStackScope()+16>: ldr     x0, [x0]
   0x000000000043f314 <js::irregexp::RegExpStackScope::~RegExpStackScope()+20>: bl      0x43f460 <js::irregexp::RegExpStack::reset()>
   0x000000000043f318 <js::irregexp::RegExpStackScope::~RegExpStackScope()+24>: nop
   0x000000000043f31c <js::irregexp::RegExpStackScope::~RegExpStackScope()+28>: ldp     x29, x30, [sp], #32
   0x000000000043f320 <js::irregexp::RegExpStackScope::~RegExpStackScope()+32>: ret
   0x000000000043f324 <js::irregexp::GrowBacktrackStack(JSRuntime*)+0>: stp     x29, x30, [sp, #-64]!
   0x000000000043f328 <js::irregexp::GrowBacktrackStack(JSRuntime*)+4>: mov     x29, sp
   0x000000000043f32c <js::irregexp::GrowBacktrackStack(JSRuntime*)+8>: str     x19, [sp, #16]
End of assembler dump.
(gdb) p/x $x0
$1 = 0x1ffeffa5e0
(gdb) p/x $sp
$2 = 0x1ffeffa660

is where the read from below sp is reported.

Comment 17 Jakub Jelinek 2019-02-19 20:25:11 UTC
So far this doesn't look like a gcc bug to me.
The code has:
       add     x28, sp, 128
       mov     x0, x28
       mov     x1, x24
       ldr     w26, [x2, 4]
       str     x26, [sp, 96]
       bl      _ZN2js8irregexp16RegExpStackScopeC1EP9JSContext
// Here x28 is still correct
...
// Code where x28 (call saved register) is not modified
...
       mov     x0, x28
       bl      _ZN2js8irregexp16RegExpStackScopeD1Ev
The problem is that x28 is clobbered and not saved/restored around that.
From what I can see, it happens in some JITted code:
js::irregexp::ExecuteCode<unsigned char> (cx=0xaaaaacb2c700, codeBlock=0x700001b9998, chars=0x7000014464
    8 "en-GB", start=0, length=5, matches=0x0, endIndex
    [m=0xffffffff9978) at /builddir/build/BUILD/firefox-60.4.0/js/src/irregexp/RegExpEngine.cpp:1868
1868            CALL_GENERATED_1(function, &data);
1: /x $x28 = 0xffffffff9750
(gdb) 
0x0000aaaaab7c6684      1868            CALL_GENERATED_1(function, &data);
1: /x $x28 = 0xffffffff9750
(gdb) 
0x0000aaaaab7c6688      1868            CALL_GENERATED_1(function, &data);
1: /x $x28 = 0xffffffff9750
(gdb) 
0x000033385da61a00 in ?? ()
1: /x $x28 = 0xffffffff9750
(gdb) 
0x000033385da66340 in ?? ()
1: /x $x28 = 0xffffffff9750
(gdb) 
0x000033385da66344 in ?? ()
1: /x $x28 = 0xffffffff9630
(gdb) 
0x000033385da66348 in ?? ()
1: /x $x28 = 0xffffffff9630
(gdb) 
0x000033385da6634c in ?? ()
1: /x $x28 = 0xffffffff9628
(gdb) 
0x000033385da66350 in ?? ()
1: /x $x28 = 0xffffffff9628
(gdb) 
0x000033385da66354 in ?? ()
1: /x $x28 = 0xffffffff9620
(gdb) 
0x000033385da66358 in ?? ()
1: /x $x28 = 0xffffffff93a0
where 0xffffffff9750 is the correct value.
(gdb) disas 0x000033385da61a00,0x000033385da61a00+4
Dump of assembler code from 0x33385da61a00 to 0x33385da61a04:
   0x000033385da61a00:  b       0x33385da66340
End of assembler dump.
(gdb) disas 0x000033385da66340,0x000033385da66358
Dump of assembler code from 0x33385da66340 to 0x33385da66358:
   0x000033385da66340:  mov     x28, sp
   0x000033385da66344:  sub     sp, x28, #0x8
   0x000033385da66348:  str     x30, [x28, #-8]!
   0x000033385da6634c:  sub     sp, x28, #0x8
   0x000033385da66350:  str     x0, [x28, #-8]!
   0x000033385da66354:  sub     x28, x28, #0x280
From the comments, it seems the JIT is using x28 as "PseudoStackRegister", whatever it is.
That is fine, but it needs to save it on entry to the JIT and restore it on return, which is not what happens here.
So, please report this to upstream and let them fix it.

The only change on the GCC side is that while it saved before something important in x26 or other registers across the JIT, it now saves something importaint in x28 across the JIT.

Comment 18 Jakub Jelinek 2019-02-19 20:34:22 UTC
Of course, another possibility is ask the compiler in the caller of the JIT code to save/restore it for you, but that would need to be done explicitly, e.g. through inline asm, or local register vars or something similar.
In any case, not looking into this further, the JIT Aarch64 maintainers need to look at their code.

Comment 19 Zbigniew Jędrzejewski-Szmek 2019-03-03 15:40:25 UTC
A build was successfull: mozjs60-60.4.0-5.fc30 [1].
Closing the bug.

[1] https://koji.fedoraproject.org/koji/buildinfo?buildID=1214412

Comment 20 Jakub Jelinek 2019-03-05 19:25:29 UTC
The upstream mozilla bugzilla now lists two patches that need to be backported to mozjs60.

Comment 21 František Zatloukal 2019-04-15 19:48:39 UTC
The patches are now backported and -O2 on aarch64 is enabled in Rawhide/mozjs60-60.6.1-2.fc31, F30 will follow soon.


Note You need to log in before you can comment on or make changes to this bug.