Skip to content

Commit ef50373

Browse files
randall77randall77
committed
reflect: ensure correct scanning of return values
During a call to a reflect-generated function or method (via makeFuncStub or methodValueCall), when should we scan the return values? When we're starting a reflect call, the space on the stack for the return values is not initialized yet, as it contains whatever junk was on the stack of the caller at the time. The return space must not be scanned during a GC. When we're finishing a reflect call, the return values are initialized, and must be scanned during a GC to make sure that any pointers in the return values are found and their referents retained. When the GC stack walk comes across a reflect call in progress on the stack, it needs to know whether to scan the results or not. It doesn't know the progress of the reflect call, so it can't decide by itself. The reflect package needs to tell it. This CL adds another slot in the frame of makeFuncStub and methodValueCall so we can put a boolean in there which tells the runtime whether to scan the results or not. This CL also adds the args length to reflectMethodValue so the runtime can restrict its scanning to only the args section (not the results) if the reflect package says the results aren't ready yet. Do a delicate dance in the reflect package to set the "results are valid" bit. We need to make sure we set the bit only after we've copied the results back to the stack. But we must set the bit before we drop reflect's copy of the results. Otherwise, we might have a state where (temporarily) no one has a live copy of the results. That's the state we were observing in issue #27695 before this CL. The bitmap used by the runtime currently contains only the args. (Actually, it contains all the bits, but the size is set so we use only the args portion.) This is safe for early in a reflect call, but unsafe late in a reflect call. The test issue27695.go demonstrates this unsafety. We change the bitmap to always include both args and results, and decide at runtime which portion to use. issue27695.go only has a test for method calls. Function calls were ok because there wasn't a safepoint between when reflect dropped its copy of the return values and when the caller is resumed. This may change when we introduce safepoints everywhere. This truncate-to-only-the-args was part of CL 9888 (in 2015). That part of the CL fixed the problem demonstrated in issue27695b.go but introduced the problem demonstrated in issue27695.go. TODO, in another CL: simplify FuncLayout and its test. stack return value is now identical to frametype.ptrdata + frametype.gcdata. Fixes #27695 Change-Id: I2d49b34e34a82c6328b34f02610587a291b25c5f Reviewed-on: https://go-review.googlesource.com/137440 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent a0e7f12 commit ef50373

19 files changed

+288
-65
lines changed

‎src/cmd/compile/internal/types/type.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ func (t *Type) ChanArgs() *Type {
817817
return t.Extra.(ChanArgs).T
818818
}
819819

820-
// FuncArgs returns the channel type for TFUNCARGS type t.
820+
// FuncArgs returns the func type for TFUNCARGS type t.
821821
func (t *Type) FuncArgs() *Type {
822822
t.wantEtype(TFUNCARGS)
823823
return t.Extra.(FuncArgs).T

‎src/reflect/all_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5844,7 +5844,7 @@ func clobber() {
58445844
type funcLayoutTest struct {
58455845
rcvr, t Type
58465846
size, argsize, retOffset uintptr
5847-
stack []byte // pointer bitmap: 1 is pointer, 0 is scalar (or uninitialized)
5847+
stack []byte // pointer bitmap: 1 is pointer, 0 is scalar
58485848
gc []byte
58495849
}
58505850

@@ -5866,7 +5866,7 @@ func init() {
58665866
6 * PtrSize,
58675867
4 * PtrSize,
58685868
4 * PtrSize,
5869-
[]byte{1, 0, 1},
5869+
[]byte{1, 0, 1, 0, 1},
58705870
[]byte{1, 0, 1, 0, 1},
58715871
})
58725872

‎src/reflect/asm_386.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,28 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No argsize here, gc generates argsize info at call site.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$8
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
1313
NO_LOCAL_POINTERS
1414
MOVL DX, 0(SP)
1515
LEAL argframe+0(FP), CX
1616
MOVL CX, 4(SP)
17+
MOVB $0, 12(SP)
18+
LEAL 12(SP), AX
19+
MOVL AX, 8(SP)
1720
CALL ·callReflect(SB)
1821
RET
1922

2023
// methodValueCall is the code half of the function returned by makeMethodValue.
2124
// See the comment on the declaration of methodValueCall in makefunc.go
2225
// for more details.
2326
// No argsize here, gc generates argsize info at call site.
24-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$8
27+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
2528
NO_LOCAL_POINTERS
2629
MOVL DX, 0(SP)
2730
LEAL argframe+0(FP), CX
2831
MOVL CX, 4(SP)
32+
MOVB $0, 12(SP)
33+
LEAL 12(SP), AX
34+
MOVL AX, 8(SP)
2935
CALL ·callMethod(SB)
3036
RET

‎src/reflect/asm_amd64.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,28 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No arg size here; runtime pulls arg map out of the func value.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$32
1313
NO_LOCAL_POINTERS
1414
MOVQ DX, 0(SP)
1515
LEAQ argframe+0(FP), CX
1616
MOVQ CX, 8(SP)
17+
MOVB $0, 24(SP)
18+
LEAQ 24(SP), AX
19+
MOVQ AX, 16(SP)
1720
CALL ·callReflect(SB)
1821
RET
1922

2023
// methodValueCall is the code half of the function returned by makeMethodValue.
2124
// See the comment on the declaration of methodValueCall in makefunc.go
2225
// for more details.
2326
// No arg size here; runtime pulls arg map out of the func value.
24-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
27+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$32
2528
NO_LOCAL_POINTERS
2629
MOVQ DX, 0(SP)
2730
LEAQ argframe+0(FP), CX
2831
MOVQ CX, 8(SP)
32+
MOVB $0, 24(SP)
33+
LEAQ 24(SP), AX
34+
MOVQ AX, 16(SP)
2935
CALL ·callMethod(SB)
3036
RET

‎src/reflect/asm_amd64p32.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,28 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No argsize here, gc generates argsize info at call site.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$8
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
1313
NO_LOCAL_POINTERS
1414
MOVL DX, 0(SP)
1515
LEAL argframe+0(FP), CX
1616
MOVL CX, 4(SP)
17+
MOVB $0, 12(SP)
18+
LEAL 12(SP), AX
19+
MOVL AX, 8(SP)
1720
CALL ·callReflect(SB)
1821
RET
1922

2023
// methodValueCall is the code half of the function returned by makeMethodValue.
2124
// See the comment on the declaration of methodValueCall in makefunc.go
2225
// for more details.
2326
// No argsize here, gc generates argsize info at call site.
24-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$8
27+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
2528
NO_LOCAL_POINTERS
2629
MOVL DX, 0(SP)
2730
LEAL argframe+0(FP), CX
2831
MOVL CX, 4(SP)
32+
MOVB $0, 12(SP)
33+
LEAL 12(SP), AX
34+
MOVL AX, 8(SP)
2935
CALL ·callMethod(SB)
3036
RET

‎src/reflect/asm_arm.s

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,30 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No argsize here, gc generates argsize info at call site.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$8
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
1313
NO_LOCAL_POINTERS
1414
MOVW R7, 4(R13)
1515
MOVW $argframe+0(FP), R1
1616
MOVW R1, 8(R13)
17+
MOVW $0, R1
18+
MOVB R1, 16(R13)
19+
ADD $16, R13, R1
20+
MOVW R1, 12(R13)
1721
BL ·callReflect(SB)
1822
RET
1923

2024
// methodValueCall is the code half of the function returned by makeMethodValue.
2125
// See the comment on the declaration of methodValueCall in makefunc.go
2226
// for more details.
2327
// No argsize here, gc generates argsize info at call site.
24-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$8
28+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
2529
NO_LOCAL_POINTERS
2630
MOVW R7, 4(R13)
2731
MOVW $argframe+0(FP), R1
2832
MOVW R1, 8(R13)
33+
MOVW $0, R1
34+
MOVB R1, 16(R13)
35+
ADD $16, R13, R1
36+
MOVW R1, 12(R13)
2937
BL ·callMethod(SB)
3038
RET

‎src/reflect/asm_arm64.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,28 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No arg size here, runtime pulls arg map out of the func value.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$24
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$40
1313
NO_LOCAL_POINTERS
1414
MOVD R26, 8(RSP)
1515
MOVD $argframe+0(FP), R3
1616
MOVD R3, 16(RSP)
17+
MOVB $0, 32(RSP)
18+
ADD $32, RSP, R3
19+
MOVD R3, 24(RSP)
1720
BL ·callReflect(SB)
1821
RET
1922

2023
// methodValueCall is the code half of the function returned by makeMethodValue.
2124
// See the comment on the declaration of methodValueCall in makefunc.go
2225
// for more details.
2326
// No arg size here; runtime pulls arg map out of the func value.
24-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$24
27+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$40
2528
NO_LOCAL_POINTERS
2629
MOVD R26, 8(RSP)
2730
MOVD $argframe+0(FP), R3
2831
MOVD R3, 16(RSP)
32+
MOVB $0, 32(RSP)
33+
ADD $32, RSP, R3
34+
MOVD R3, 24(RSP)
2935
BL ·callMethod(SB)
3036
RET

‎src/reflect/asm_mips64x.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,28 @@
1313
// See the comment on the declaration of makeFuncStub in makefunc.go
1414
// for more details.
1515
// No arg size here, runtime pulls arg map out of the func value.
16-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
16+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$32
1717
NO_LOCAL_POINTERS
1818
MOVV REGCTXT, 8(R29)
1919
MOVV $argframe+0(FP), R1
2020
MOVV R1, 16(R29)
21+
MOVB R0, 32(R29)
22+
ADDV $32, R29, R1
23+
MOVV R1, 24(R29)
2124
JAL ·callReflect(SB)
2225
RET
2326

2427
// methodValueCall is the code half of the function returned by makeMethodValue.
2528
// See the comment on the declaration of methodValueCall in makefunc.go
2629
// for more details.
2730
// No arg size here; runtime pulls arg map out of the func value.
28-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
31+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$32
2932
NO_LOCAL_POINTERS
3033
MOVV REGCTXT, 8(R29)
3134
MOVV $argframe+0(FP), R1
3235
MOVV R1, 16(R29)
36+
MOVB R0, 32(R29)
37+
ADDV $32, R29, R1
38+
MOVV R1, 24(R29)
3339
JAL ·callMethod(SB)
3440
RET

‎src/reflect/asm_mipsx.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,28 @@
1313
// See the comment on the declaration of makeFuncStub in makefunc.go
1414
// for more details.
1515
// No arg size here, runtime pulls arg map out of the func value.
16-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$8
16+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
1717
NO_LOCAL_POINTERS
1818
MOVW REGCTXT, 4(R29)
1919
MOVW $argframe+0(FP), R1
2020
MOVW R1, 8(R29)
21+
MOVB R0, 16(R29)
22+
ADD $16, R29, R1
23+
MOVW R1, 12(R29)
2124
JAL ·callReflect(SB)
2225
RET
2326

2427
// methodValueCall is the code half of the function returned by makeMethodValue.
2528
// See the comment on the declaration of methodValueCall in makefunc.go
2629
// for more details.
2730
// No arg size here; runtime pulls arg map out of the func value.
28-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$8
31+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
2932
NO_LOCAL_POINTERS
3033
MOVW REGCTXT, 4(R29)
3134
MOVW $argframe+0(FP), R1
3235
MOVW R1, 8(R29)
36+
MOVB R0, 16(R29)
37+
ADD $16, R29, R1
38+
MOVW R1, 12(R29)
3339
JAL ·callMethod(SB)
3440
RET

‎src/reflect/asm_ppc64x.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,28 @@
1212
// See the comment on the declaration of makeFuncStub in makefunc.go
1313
// for more details.
1414
// No arg size here, runtime pulls arg map out of the func value.
15-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
15+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$32
1616
NO_LOCAL_POINTERS
1717
MOVD R11, FIXED_FRAME+0(R1)
1818
MOVD $argframe+0(FP), R3
1919
MOVD R3, FIXED_FRAME+8(R1)
20+
MOVB R0, FIXED_FRAME+24(R1)
21+
ADD $FIXED_FRAME+24, R1, R3
22+
MOVD R3, FIXED_FRAME+16(R1)
2023
BL ·callReflect(SB)
2124
RET
2225

2326
// methodValueCall is the code half of the function returned by makeMethodValue.
2427
// See the comment on the declaration of methodValueCall in makefunc.go
2528
// for more details.
2629
// No arg size here; runtime pulls arg map out of the func value.
27-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
30+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$32
2831
NO_LOCAL_POINTERS
2932
MOVD R11, FIXED_FRAME+0(R1)
3033
MOVD $argframe+0(FP), R3
3134
MOVD R3, FIXED_FRAME+8(R1)
35+
MOVB R0, FIXED_FRAME+24(R1)
36+
ADD $FIXED_FRAME+24, R1, R3
37+
MOVD R3, FIXED_FRAME+16(R1)
3238
BL ·callMethod(SB)
3339
RET

‎src/reflect/asm_s390x.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,28 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No arg size here, runtime pulls arg map out of the func value.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$32
1313
NO_LOCAL_POINTERS
1414
MOVD R12, 8(R15)
1515
MOVD $argframe+0(FP), R3
1616
MOVD R3, 16(R15)
17+
MOVB R0, 32(R15)
18+
ADD $32, R15, R3
19+
MOVD R3, 24(R15)
1720
BL ·callReflect(SB)
1821
RET
1922

2023
// methodValueCall is the code half of the function returned by makeMethodValue.
2124
// See the comment on the declaration of methodValueCall in makefunc.go
2225
// for more details.
2326
// No arg size here; runtime pulls arg map out of the func value.
24-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
27+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$32
2528
NO_LOCAL_POINTERS
2629
MOVD R12, 8(R15)
2730
MOVD $argframe+0(FP), R3
2831
MOVD R3, 16(R15)
32+
MOVB R0, 32(R15)
33+
ADD $32, R15, R3
34+
MOVD R3, 24(R15)
2935
BL ·callMethod(SB)
3036
RET

‎src/reflect/asm_wasm.s

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// See the comment on the declaration of makeFuncStub in makefunc.go
1010
// for more details.
1111
// No arg size here; runtime pulls arg map out of the func value.
12-
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
12+
TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$32
1313
NO_LOCAL_POINTERS
1414

1515
MOVD CTXT, 0(SP)
@@ -21,14 +21,17 @@ TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
2121
I64Add
2222
I64Store $8
2323

24+
MOVB $0, 24(SP)
25+
MOVD $24(SP), 16(SP)
26+
2427
CALL ·callReflect(SB)
2528
RET
2629

2730
// methodValueCall is the code half of the function returned by makeMethodValue.
2831
// See the comment on the declaration of methodValueCall in makefunc.go
2932
// for more details.
3033
// No arg size here; runtime pulls arg map out of the func value.
31-
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
34+
TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$32
3235
NO_LOCAL_POINTERS
3336

3437
MOVD CTXT, 0(SP)
@@ -40,5 +43,8 @@ TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
4043
I64Add
4144
I64Store $8
4245

46+
MOVB $0, 24(SP)
47+
MOVD $24(SP), 16(SP)
48+
4349
CALL ·callMethod(SB)
4450
RET

‎src/reflect/export_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ func FuncLayout(t Type, rcvr Type) (frametype Type, argSize, retOffset uintptr,
2525
var ft *rtype
2626
var s *bitVector
2727
if rcvr != nil {
28-
ft, argSize, retOffset, s, _ = funcLayout(t.(*rtype), rcvr.(*rtype))
28+
ft, argSize, retOffset, s, _ = funcLayout((*funcType)(unsafe.Pointer(t.(*rtype))), rcvr.(*rtype))
2929
} else {
30-
ft, argSize, retOffset, s, _ = funcLayout(t.(*rtype), nil)
30+
ft, argSize, retOffset, s, _ = funcLayout((*funcType)(unsafe.Pointer(t.(*rtype))), nil)
3131
}
3232
frametype = ft
3333
for i := uint32(0); i < s.n; i++ {

0 commit comments

Comments
 (0)