From 3050a0a7d2bccede88c56828609902469328e15f Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Mon, 30 Jun 2014 11:06:47 -0700 Subject: [PATCH] encoding/gob: remove unsafe, use reflection. This removes a major unsafe thorn in our side, a perennial obstacle to clean garbage collection. Not coincidentally: In cleaning this up, several bugs were found, including code that reached inside by-value interfaces to create pointers for pointer-receiver methods. Unsafe code is just as advertised. Performance of course suffers, but not too badly. The Pipe number is more indicative, since it's doing I/O that simulates a network connection. Plus these are end-to-end, so each end suffers only half of this pain. The edit is pretty much a line-by-line conversion, with a few simplifications and a couple of new tests. There may be more performance to gain. BenchmarkEndToEndByteBuffer 2493 3033 +21.66% BenchmarkEndToEndPipe 4953 5597 +13.00% Fixes #5159. LGTM=rsc R=rsc CC=golang-codereviews, khr https://golang.org/cl/102680045 --- src/pkg/encoding/gob/codec_test.go | 311 ++++++------- src/pkg/encoding/gob/debug.go | 6 +- src/pkg/encoding/gob/decode.go | 588 +++++++++++-------------- src/pkg/encoding/gob/encode.go | 324 +++++--------- src/pkg/encoding/gob/encoder_test.go | 76 +++- src/pkg/encoding/gob/gobencdec_test.go | 7 +- src/pkg/encoding/gob/timing_test.go | 6 +- 7 files changed, 593 insertions(+), 725 deletions(-) diff --git a/src/pkg/encoding/gob/codec_test.go b/src/pkg/encoding/gob/codec_test.go index fa57f3761d..a6012f55e0 100644 --- a/src/pkg/encoding/gob/codec_test.go +++ b/src/pkg/encoding/gob/codec_test.go @@ -14,7 +14,6 @@ import ( "strings" "testing" "time" - "unsafe" ) var doFuzzTests = flag.Bool("gob.fuzz", false, "run the fuzz tests, which are large and very slow") @@ -140,10 +139,10 @@ func TestScalarEncInstructions(t *testing.T) { // bool { - data := struct{ a bool }{true} - instr := &encInstr{encBool, 6, 0, 0} + var data bool = true + instr := &encInstr{encBool, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(boolResult, b.Bytes()) { t.Errorf("bool enc instructions: expected % x got % x", boolResult, b.Bytes()) } @@ -152,10 +151,10 @@ func TestScalarEncInstructions(t *testing.T) { // int { b.Reset() - data := struct{ a int }{17} - instr := &encInstr{encInt, 6, 0, 0} + var data int = 17 + instr := &encInstr{encInt, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(signedResult, b.Bytes()) { t.Errorf("int enc instructions: expected % x got % x", signedResult, b.Bytes()) } @@ -164,10 +163,10 @@ func TestScalarEncInstructions(t *testing.T) { // uint { b.Reset() - data := struct{ a uint }{17} - instr := &encInstr{encUint, 6, 0, 0} + var data uint = 17 + instr := &encInstr{encUint, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(unsignedResult, b.Bytes()) { t.Errorf("uint enc instructions: expected % x got % x", unsignedResult, b.Bytes()) } @@ -176,10 +175,10 @@ func TestScalarEncInstructions(t *testing.T) { // int8 { b.Reset() - data := struct{ a int8 }{17} - instr := &encInstr{encInt8, 6, 0, 0} + var data int8 = 17 + instr := &encInstr{encInt, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(signedResult, b.Bytes()) { t.Errorf("int8 enc instructions: expected % x got % x", signedResult, b.Bytes()) } @@ -188,10 +187,10 @@ func TestScalarEncInstructions(t *testing.T) { // uint8 { b.Reset() - data := struct{ a uint8 }{17} - instr := &encInstr{encUint8, 6, 0, 0} + var data uint8 = 17 + instr := &encInstr{encUint, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(unsignedResult, b.Bytes()) { t.Errorf("uint8 enc instructions: expected % x got % x", unsignedResult, b.Bytes()) } @@ -200,10 +199,10 @@ func TestScalarEncInstructions(t *testing.T) { // int16 { b.Reset() - data := struct{ a int16 }{17} - instr := &encInstr{encInt16, 6, 0, 0} + var data int16 = 17 + instr := &encInstr{encInt, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(signedResult, b.Bytes()) { t.Errorf("int16 enc instructions: expected % x got % x", signedResult, b.Bytes()) } @@ -212,10 +211,10 @@ func TestScalarEncInstructions(t *testing.T) { // uint16 { b.Reset() - data := struct{ a uint16 }{17} - instr := &encInstr{encUint16, 6, 0, 0} + var data uint16 = 17 + instr := &encInstr{encUint, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(unsignedResult, b.Bytes()) { t.Errorf("uint16 enc instructions: expected % x got % x", unsignedResult, b.Bytes()) } @@ -224,10 +223,10 @@ func TestScalarEncInstructions(t *testing.T) { // int32 { b.Reset() - data := struct{ a int32 }{17} - instr := &encInstr{encInt32, 6, 0, 0} + var data int32 = 17 + instr := &encInstr{encInt, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(signedResult, b.Bytes()) { t.Errorf("int32 enc instructions: expected % x got % x", signedResult, b.Bytes()) } @@ -236,10 +235,10 @@ func TestScalarEncInstructions(t *testing.T) { // uint32 { b.Reset() - data := struct{ a uint32 }{17} - instr := &encInstr{encUint32, 6, 0, 0} + var data uint32 = 17 + instr := &encInstr{encUint, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(unsignedResult, b.Bytes()) { t.Errorf("uint32 enc instructions: expected % x got % x", unsignedResult, b.Bytes()) } @@ -248,10 +247,10 @@ func TestScalarEncInstructions(t *testing.T) { // int64 { b.Reset() - data := struct{ a int64 }{17} - instr := &encInstr{encInt64, 6, 0, 0} + var data int64 = 17 + instr := &encInstr{encInt, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(signedResult, b.Bytes()) { t.Errorf("int64 enc instructions: expected % x got % x", signedResult, b.Bytes()) } @@ -260,10 +259,10 @@ func TestScalarEncInstructions(t *testing.T) { // uint64 { b.Reset() - data := struct{ a uint64 }{17} - instr := &encInstr{encUint64, 6, 0, 0} + var data uint64 = 17 + instr := &encInstr{encUint, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(unsignedResult, b.Bytes()) { t.Errorf("uint64 enc instructions: expected % x got % x", unsignedResult, b.Bytes()) } @@ -272,10 +271,10 @@ func TestScalarEncInstructions(t *testing.T) { // float32 { b.Reset() - data := struct{ a float32 }{17} - instr := &encInstr{encFloat32, 6, 0, 0} + var data float32 = 17 + instr := &encInstr{encFloat, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(floatResult, b.Bytes()) { t.Errorf("float32 enc instructions: expected % x got % x", floatResult, b.Bytes()) } @@ -284,10 +283,10 @@ func TestScalarEncInstructions(t *testing.T) { // float64 { b.Reset() - data := struct{ a float64 }{17} - instr := &encInstr{encFloat64, 6, 0, 0} + var data float64 = 17 + instr := &encInstr{encFloat, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(floatResult, b.Bytes()) { t.Errorf("float64 enc instructions: expected % x got % x", floatResult, b.Bytes()) } @@ -296,10 +295,10 @@ func TestScalarEncInstructions(t *testing.T) { // bytes == []uint8 { b.Reset() - data := struct{ a []byte }{[]byte("hello")} - instr := &encInstr{encUint8Array, 6, 0, 0} + data := []byte("hello") + instr := &encInstr{encUint8Array, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(bytesResult, b.Bytes()) { t.Errorf("bytes enc instructions: expected % x got % x", bytesResult, b.Bytes()) } @@ -308,23 +307,23 @@ func TestScalarEncInstructions(t *testing.T) { // string { b.Reset() - data := struct{ a string }{"hello"} - instr := &encInstr{encString, 6, 0, 0} + var data string = "hello" + instr := &encInstr{encString, 6, nil, 0} state := newEncoderState(b) - instr.op(instr, state, unsafe.Pointer(&data)) + instr.op(instr, state, reflect.ValueOf(data)) if !bytes.Equal(bytesResult, b.Bytes()) { t.Errorf("string enc instructions: expected % x got % x", bytesResult, b.Bytes()) } } } -func execDec(typ string, instr *decInstr, state *decoderState, t *testing.T, p unsafe.Pointer) { +func execDec(typ string, instr *decInstr, state *decoderState, t *testing.T, value reflect.Value) { defer testError(t) v := int(state.decodeUint()) if v+state.fieldnum != 6 { t.Fatalf("decoding field number %d, got %d", 6, v+state.fieldnum) } - instr.op(instr, state, decIndirect(p, instr.indir)) + instr.op(instr, state, decIndirect(value, instr.indir)) state.fieldnum = 6 } @@ -342,234 +341,198 @@ func TestScalarDecInstructions(t *testing.T) { // bool { - var data struct { - a bool - } - instr := &decInstr{decBool, 6, 0, 0, ovfl} + var data bool + instr := &decInstr{decBool, 6, nil, 1, ovfl} state := newDecodeStateFromData(boolResult) - execDec("bool", instr, state, t, unsafe.Pointer(&data)) - if data.a != true { - t.Errorf("bool a = %v not true", data.a) + execDec("bool", instr, state, t, reflect.ValueOf(&data)) + if data != true { + t.Errorf("bool a = %v not true", data) } } // int { - var data struct { - a int - } - instr := &decInstr{decOpTable[reflect.Int], 6, 0, 0, ovfl} + var data int + instr := &decInstr{decOpTable[reflect.Int], 6, nil, 1, ovfl} state := newDecodeStateFromData(signedResult) - execDec("int", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("int a = %v not 17", data.a) + execDec("int", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("int a = %v not 17", data) } } // uint { - var data struct { - a uint - } - instr := &decInstr{decOpTable[reflect.Uint], 6, 0, 0, ovfl} + var data uint + instr := &decInstr{decOpTable[reflect.Uint], 6, nil, 1, ovfl} state := newDecodeStateFromData(unsignedResult) - execDec("uint", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("uint a = %v not 17", data.a) + execDec("uint", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("uint a = %v not 17", data) } } // int8 { - var data struct { - a int8 - } - instr := &decInstr{decInt8, 6, 0, 0, ovfl} + var data int8 + instr := &decInstr{decInt8, 6, nil, 1, ovfl} state := newDecodeStateFromData(signedResult) - execDec("int8", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("int8 a = %v not 17", data.a) + execDec("int8", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("int8 a = %v not 17", data) } } // uint8 { - var data struct { - a uint8 - } - instr := &decInstr{decUint8, 6, 0, 0, ovfl} + var data uint8 + instr := &decInstr{decUint8, 6, nil, 1, ovfl} state := newDecodeStateFromData(unsignedResult) - execDec("uint8", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("uint8 a = %v not 17", data.a) + execDec("uint8", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("uint8 a = %v not 17", data) } } // int16 { - var data struct { - a int16 - } - instr := &decInstr{decInt16, 6, 0, 0, ovfl} + var data int16 + instr := &decInstr{decInt16, 6, nil, 1, ovfl} state := newDecodeStateFromData(signedResult) - execDec("int16", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("int16 a = %v not 17", data.a) + execDec("int16", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("int16 a = %v not 17", data) } } // uint16 { - var data struct { - a uint16 - } - instr := &decInstr{decUint16, 6, 0, 0, ovfl} + var data uint16 + instr := &decInstr{decUint16, 6, nil, 1, ovfl} state := newDecodeStateFromData(unsignedResult) - execDec("uint16", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("uint16 a = %v not 17", data.a) + execDec("uint16", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("uint16 a = %v not 17", data) } } // int32 { - var data struct { - a int32 - } - instr := &decInstr{decInt32, 6, 0, 0, ovfl} + var data int32 + instr := &decInstr{decInt32, 6, nil, 1, ovfl} state := newDecodeStateFromData(signedResult) - execDec("int32", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("int32 a = %v not 17", data.a) + execDec("int32", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("int32 a = %v not 17", data) } } // uint32 { - var data struct { - a uint32 - } - instr := &decInstr{decUint32, 6, 0, 0, ovfl} + var data uint32 + instr := &decInstr{decUint32, 6, nil, 1, ovfl} state := newDecodeStateFromData(unsignedResult) - execDec("uint32", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("uint32 a = %v not 17", data.a) + execDec("uint32", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("uint32 a = %v not 17", data) } } // uintptr { - var data struct { - a uintptr - } - instr := &decInstr{decOpTable[reflect.Uintptr], 6, 0, 0, ovfl} + var data uintptr + instr := &decInstr{decOpTable[reflect.Uintptr], 6, nil, 1, ovfl} state := newDecodeStateFromData(unsignedResult) - execDec("uintptr", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("uintptr a = %v not 17", data.a) + execDec("uintptr", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("uintptr a = %v not 17", data) } } // int64 { - var data struct { - a int64 - } - instr := &decInstr{decInt64, 6, 0, 0, ovfl} + var data int64 + instr := &decInstr{decInt64, 6, nil, 1, ovfl} state := newDecodeStateFromData(signedResult) - execDec("int64", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("int64 a = %v not 17", data.a) + execDec("int64", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("int64 a = %v not 17", data) } } // uint64 { - var data struct { - a uint64 - } - instr := &decInstr{decUint64, 6, 0, 0, ovfl} + var data uint64 + instr := &decInstr{decUint64, 6, nil, 1, ovfl} state := newDecodeStateFromData(unsignedResult) - execDec("uint64", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("uint64 a = %v not 17", data.a) + execDec("uint64", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("uint64 a = %v not 17", data) } } // float32 { - var data struct { - a float32 - } - instr := &decInstr{decFloat32, 6, 0, 0, ovfl} + var data float32 + instr := &decInstr{decFloat32, 6, nil, 1, ovfl} state := newDecodeStateFromData(floatResult) - execDec("float32", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("float32 a = %v not 17", data.a) + execDec("float32", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("float32 a = %v not 17", data) } } // float64 { - var data struct { - a float64 - } - instr := &decInstr{decFloat64, 6, 0, 0, ovfl} + var data float64 + instr := &decInstr{decFloat64, 6, nil, 1, ovfl} state := newDecodeStateFromData(floatResult) - execDec("float64", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17 { - t.Errorf("float64 a = %v not 17", data.a) + execDec("float64", instr, state, t, reflect.ValueOf(&data)) + if data != 17 { + t.Errorf("float64 a = %v not 17", data) } } // complex64 { - var data struct { - a complex64 - } - instr := &decInstr{decOpTable[reflect.Complex64], 6, 0, 0, ovfl} + var data complex64 + instr := &decInstr{decOpTable[reflect.Complex64], 6, nil, 1, ovfl} state := newDecodeStateFromData(complexResult) - execDec("complex", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17+19i { - t.Errorf("complex a = %v not 17+19i", data.a) + execDec("complex", instr, state, t, reflect.ValueOf(&data)) + if data != 17+19i { + t.Errorf("complex a = %v not 17+19i", data) } } // complex128 { - var data struct { - a complex128 - } - instr := &decInstr{decOpTable[reflect.Complex128], 6, 0, 0, ovfl} + var data complex128 + instr := &decInstr{decOpTable[reflect.Complex128], 6, nil, 1, ovfl} state := newDecodeStateFromData(complexResult) - execDec("complex", instr, state, t, unsafe.Pointer(&data)) - if data.a != 17+19i { - t.Errorf("complex a = %v not 17+19i", data.a) + execDec("complex", instr, state, t, reflect.ValueOf(&data)) + if data != 17+19i { + t.Errorf("complex a = %v not 17+19i", data) } } // bytes == []uint8 { - var data struct { - a []byte - } - instr := &decInstr{decUint8Slice, 6, 0, 0, ovfl} + var data []byte + instr := &decInstr{decUint8Slice, 6, nil, 1, ovfl} state := newDecodeStateFromData(bytesResult) - execDec("bytes", instr, state, t, unsafe.Pointer(&data)) - if string(data.a) != "hello" { - t.Errorf(`bytes a = %q not "hello"`, string(data.a)) + execDec("bytes", instr, state, t, reflect.ValueOf(&data)) + if string(data) != "hello" { + t.Errorf(`bytes a = %q not "hello"`, string(data)) } } // string { - var data struct { - a string - } - instr := &decInstr{decString, 6, 0, 0, ovfl} + var data string + instr := &decInstr{decString, 6, nil, 1, ovfl} state := newDecodeStateFromData(bytesResult) - execDec("bytes", instr, state, t, unsafe.Pointer(&data)) - if data.a != "hello" { - t.Errorf(`bytes a = %q not "hello"`, data.a) + execDec("bytes", instr, state, t, reflect.ValueOf(&data)) + if data != "hello" { + t.Errorf(`bytes a = %q not "hello"`, data) } } } diff --git a/src/pkg/encoding/gob/debug.go b/src/pkg/encoding/gob/debug.go index 6117eb0837..536bbdb5ac 100644 --- a/src/pkg/encoding/gob/debug.go +++ b/src/pkg/encoding/gob/debug.go @@ -306,7 +306,7 @@ func (deb *debugger) common() CommonType { // Id typeId id = deb.typeId() default: - errorf("corrupted CommonType") + errorf("corrupted CommonType, delta is %d fieldNum is %d", delta, fieldNum) } } return CommonType{name, id} @@ -598,11 +598,11 @@ func (deb *debugger) printBuiltin(indent tab, id typeId) { fmt.Fprintf(os.Stderr, "%s%d\n", indent, x) case tFloat: x := deb.uint64() - fmt.Fprintf(os.Stderr, "%s%g\n", indent, floatFromBits(x)) + fmt.Fprintf(os.Stderr, "%s%g\n", indent, float64FromBits(x)) case tComplex: r := deb.uint64() i := deb.uint64() - fmt.Fprintf(os.Stderr, "%s%g+%gi\n", indent, floatFromBits(r), floatFromBits(i)) + fmt.Fprintf(os.Stderr, "%s%g+%gi\n", indent, float64FromBits(r), float64FromBits(i)) case tBytes: x := int(deb.uint64()) b := make([]byte, x) diff --git a/src/pkg/encoding/gob/decode.go b/src/pkg/encoding/gob/decode.go index d8513148ec..14e01ae866 100644 --- a/src/pkg/encoding/gob/decode.go +++ b/src/pkg/encoding/gob/decode.go @@ -4,9 +4,6 @@ package gob -// TODO(rsc): When garbage collector changes, revisit -// the allocations in this file that use unsafe.Pointer. - import ( "bytes" "encoding" @@ -14,7 +11,6 @@ import ( "io" "math" "reflect" - "unsafe" ) var ( @@ -128,15 +124,15 @@ func (state *decoderState) decodeInt() int64 { } // decOp is the signature of a decoding operator for a given type. -type decOp func(i *decInstr, state *decoderState, p unsafe.Pointer) +type decOp func(i *decInstr, state *decoderState, v reflect.Value) // The 'instructions' of the decoding machine type decInstr struct { - op decOp - field int // field number of the wire type - indir int // how many pointer indirections to reach the value in the struct - offset uintptr // offset in the structure of the field to encode - ovfl error // error message for overflow/underflow (for arrays, of the elements) + op decOp + field int // field number of the wire type + index []int // field access indices for destination type + indir int // how many pointer indirections to reach the value in the struct + ovfl error // error message for overflow/underflow (for arrays, of the elements) } // Since the encoder writes no zeros, if we arrive at a decoder we have @@ -146,157 +142,112 @@ type decInstr struct { // with the data structure. If any pointer so reached is nil, allocation must // be done. -// Walk the pointer hierarchy, allocating if we find a nil. Stop one before the end. -func decIndirect(p unsafe.Pointer, indir int) unsafe.Pointer { +// decIndirect walks the pointer hierarchy, allocating if we find a nil. Stop one before the end. +func decIndirect(pv reflect.Value, indir int) reflect.Value { for ; indir > 1; indir-- { - if *(*unsafe.Pointer)(p) == nil { + if pv.IsNil() { // Allocation required - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(unsafe.Pointer)) + pv.Set(reflect.New(pv.Type().Elem())) // New will always allocate a pointer here. } - p = *(*unsafe.Pointer)(p) + pv = pv.Elem() } - return p + return pv } // ignoreUint discards a uint value with no destination. -func ignoreUint(i *decInstr, state *decoderState, p unsafe.Pointer) { +func ignoreUint(i *decInstr, state *decoderState, v reflect.Value) { state.decodeUint() } // ignoreTwoUints discards a uint value with no destination. It's used to skip // complex values. -func ignoreTwoUints(i *decInstr, state *decoderState, p unsafe.Pointer) { +func ignoreTwoUints(i *decInstr, state *decoderState, v reflect.Value) { state.decodeUint() state.decodeUint() } -// decBool decodes a uint and stores it as a boolean through p. -func decBool(i *decInstr, state *decoderState, p unsafe.Pointer) { +// decAlloc takes a value and returns a settable value that can +// be assigned to. If the value is a pointer (i.indir is positive), +// decAlloc guarantees it points to storage. +func (i *decInstr) decAlloc(v reflect.Value) reflect.Value { if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(bool)) + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) } - p = *(*unsafe.Pointer)(p) + v = v.Elem() } - *(*bool)(p) = state.decodeUint() != 0 + return v } -// decInt8 decodes an integer and stores it as an int8 through p. -func decInt8(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(int8)) - } - p = *(*unsafe.Pointer)(p) - } +// decBool decodes a uint and stores it as a boolean in value. +func decBool(i *decInstr, state *decoderState, value reflect.Value) { + i.decAlloc(value).SetBool(state.decodeUint() != 0) +} + +// decInt8 decodes an integer and stores it as an int8 in value. +func decInt8(i *decInstr, state *decoderState, value reflect.Value) { v := state.decodeInt() if v < math.MinInt8 || math.MaxInt8 < v { error_(i.ovfl) - } else { - *(*int8)(p) = int8(v) } + i.decAlloc(value).SetInt(v) } -// decUint8 decodes an unsigned integer and stores it as a uint8 through p. -func decUint8(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(uint8)) - } - p = *(*unsafe.Pointer)(p) - } +// decUint8 decodes an unsigned integer and stores it as a uint8 in value. +func decUint8(i *decInstr, state *decoderState, value reflect.Value) { v := state.decodeUint() if math.MaxUint8 < v { error_(i.ovfl) - } else { - *(*uint8)(p) = uint8(v) } + i.decAlloc(value).SetUint(v) } -// decInt16 decodes an integer and stores it as an int16 through p. -func decInt16(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(int16)) - } - p = *(*unsafe.Pointer)(p) - } +// decInt16 decodes an integer and stores it as an int16 in value. +func decInt16(i *decInstr, state *decoderState, value reflect.Value) { v := state.decodeInt() if v < math.MinInt16 || math.MaxInt16 < v { error_(i.ovfl) - } else { - *(*int16)(p) = int16(v) } + i.decAlloc(value).SetInt(v) } -// decUint16 decodes an unsigned integer and stores it as a uint16 through p. -func decUint16(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(uint16)) - } - p = *(*unsafe.Pointer)(p) - } +// decUint16 decodes an unsigned integer and stores it as a uint16 in value. +func decUint16(i *decInstr, state *decoderState, value reflect.Value) { v := state.decodeUint() if math.MaxUint16 < v { error_(i.ovfl) - } else { - *(*uint16)(p) = uint16(v) } + i.decAlloc(value).SetUint(v) } -// decInt32 decodes an integer and stores it as an int32 through p. -func decInt32(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(int32)) - } - p = *(*unsafe.Pointer)(p) - } +// decInt32 decodes an integer and stores it as an int32 in value. +func decInt32(i *decInstr, state *decoderState, value reflect.Value) { v := state.decodeInt() if v < math.MinInt32 || math.MaxInt32 < v { error_(i.ovfl) - } else { - *(*int32)(p) = int32(v) } + i.decAlloc(value).SetInt(v) } -// decUint32 decodes an unsigned integer and stores it as a uint32 through p. -func decUint32(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(uint32)) - } - p = *(*unsafe.Pointer)(p) - } +// decUint32 decodes an unsigned integer and stores it as a uint32 in value. +func decUint32(i *decInstr, state *decoderState, value reflect.Value) { v := state.decodeUint() if math.MaxUint32 < v { error_(i.ovfl) - } else { - *(*uint32)(p) = uint32(v) } + i.decAlloc(value).SetUint(v) } -// decInt64 decodes an integer and stores it as an int64 through p. -func decInt64(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(int64)) - } - p = *(*unsafe.Pointer)(p) - } - *(*int64)(p) = int64(state.decodeInt()) +// decInt64 decodes an integer and stores it as an int64 in value. +func decInt64(i *decInstr, state *decoderState, value reflect.Value) { + v := state.decodeInt() + i.decAlloc(value).SetInt(v) } -// decUint64 decodes an unsigned integer and stores it as a uint64 through p. -func decUint64(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(uint64)) - } - p = *(*unsafe.Pointer)(p) - } - *(*uint64)(p) = uint64(state.decodeUint()) +// decUint64 decodes an unsigned integer and stores it as a uint64 in value. +func decUint64(i *decInstr, state *decoderState, value reflect.Value) { + v := state.decodeUint() + i.decAlloc(value).SetUint(v) } // Floating-point numbers are transmitted as uint64s holding the bits @@ -304,7 +255,7 @@ func decUint64(i *decInstr, state *decoderState, p unsafe.Pointer) { // the exponent end coming out first, so integer floating point numbers // (for example) transmit more compactly. This routine does the // unswizzling. -func floatFromBits(u uint64) float64 { +func float64FromBits(u uint64) float64 { var v uint64 for i := 0; i < 8; i++ { v <<= 8 @@ -314,10 +265,12 @@ func floatFromBits(u uint64) float64 { return math.Float64frombits(v) } -// storeFloat32 decodes an unsigned integer, treats it as a 32-bit floating-point -// number, and stores it through p. It's a helper function for float32 and complex64. -func storeFloat32(i *decInstr, state *decoderState, p unsafe.Pointer) { - v := floatFromBits(state.decodeUint()) +// float32FromBits decodes an unsigned integer, treats it as a 32-bit floating-point +// number, and returns it. It's a helper function for float32 and complex64. +// It returns a float64 because that's what reflection needs, but its return +// value is known to be accurately representable in a float32. +func float32FromBits(i *decInstr, u uint64) float64 { + v := float64FromBits(u) av := v if av < 0 { av = -av @@ -325,117 +278,105 @@ func storeFloat32(i *decInstr, state *decoderState, p unsafe.Pointer) { // +Inf is OK in both 32- and 64-bit floats. Underflow is always OK. if math.MaxFloat32 < av && av <= math.MaxFloat64 { error_(i.ovfl) - } else { - *(*float32)(p) = float32(v) } + return v } // decFloat32 decodes an unsigned integer, treats it as a 32-bit floating-point -// number, and stores it through p. -func decFloat32(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(float32)) - } - p = *(*unsafe.Pointer)(p) - } - storeFloat32(i, state, p) +// number, and stores it in value. +func decFloat32(i *decInstr, state *decoderState, value reflect.Value) { + i.decAlloc(value).SetFloat(float32FromBits(i, state.decodeUint())) } // decFloat64 decodes an unsigned integer, treats it as a 64-bit floating-point -// number, and stores it through p. -func decFloat64(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(float64)) - } - p = *(*unsafe.Pointer)(p) - } - *(*float64)(p) = floatFromBits(uint64(state.decodeUint())) +// number, and stores it in value. +func decFloat64(i *decInstr, state *decoderState, value reflect.Value) { + i.decAlloc(value).SetFloat(float64FromBits(state.decodeUint())) } // decComplex64 decodes a pair of unsigned integers, treats them as a -// pair of floating point numbers, and stores them as a complex64 through p. +// pair of floating point numbers, and stores them as a complex64 through v. // The real part comes first. -func decComplex64(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(complex64)) - } - p = *(*unsafe.Pointer)(p) - } - storeFloat32(i, state, p) - storeFloat32(i, state, unsafe.Pointer(uintptr(p)+unsafe.Sizeof(float32(0)))) +func decComplex64(i *decInstr, state *decoderState, value reflect.Value) { + real := float32FromBits(i, state.decodeUint()) + imag := float32FromBits(i, state.decodeUint()) + i.decAlloc(value).SetComplex(complex(real, imag)) } // decComplex128 decodes a pair of unsigned integers, treats them as a -// pair of floating point numbers, and stores them as a complex128 through p. +// pair of floating point numbers, and stores them as a complex128 through v. // The real part comes first. -func decComplex128(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(complex128)) - } - p = *(*unsafe.Pointer)(p) - } - real := floatFromBits(uint64(state.decodeUint())) - imag := floatFromBits(uint64(state.decodeUint())) - *(*complex128)(p) = complex(real, imag) +func decComplex128(i *decInstr, state *decoderState, value reflect.Value) { + real := float64FromBits(state.decodeUint()) + imag := float64FromBits(state.decodeUint()) + i.decAlloc(value).SetComplex(complex(real, imag)) } -// decUint8Slice decodes a byte slice and stores through p a slice header +// decUint8Slice decodes a byte slice and stores through v a slice header // describing the data. // uint8 slices are encoded as an unsigned count followed by the raw bytes. -func decUint8Slice(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new([]uint8)) +func decUint8Slice(i *decInstr, state *decoderState, value reflect.Value) { + u := state.decodeUint() + n := int(u) + if n < 0 { + errorf("negative slice length: %d", n) + } + if n > state.b.Len() { + errorf("%s data too long for buffer: %d", value.Type(), n) + } + // Indirect if necessary until we have a settable slice header with enough storage. + typ := value.Type() + switch typ.Kind() { + default: + panic("should be slice " + typ.String()) + case reflect.Slice: + if value.Cap() < n { + value.Set(reflect.MakeSlice(typ, n, n)) } - p = *(*unsafe.Pointer)(p) - } - n := state.decodeUint() - if n > uint64(state.b.Len()) { - errorf("length of []byte exceeds input size (%d bytes)", n) - } - slice := (*[]uint8)(p) - if uint64(cap(*slice)) < n { - *slice = make([]uint8, n) - } else { - *slice = (*slice)[0:n] + case reflect.Ptr: + for typ.Elem().Kind() == reflect.Ptr { + if value.IsNil() { + value.Set(reflect.New(typ.Elem())) + } + value = value.Elem() + typ = typ.Elem() + } + // Value is now a pointer to a slice header. + // It might be nil. If so, allocate the header. + if value.IsNil() { + value.Set(reflect.New(typ.Elem())) + } + if value.Elem().IsNil() || value.Elem().Cap() < n { + value.Elem().Set(reflect.MakeSlice(typ.Elem(), n, n)) + } else { + value.Elem().Set(value.Elem().Slice(0, n)) + } + value = value.Elem() } - if _, err := state.b.Read(*slice); err != nil { + if _, err := state.b.Read(value.Bytes()); err != nil { errorf("error decoding []byte: %s", err) } } -// decString decodes byte array and stores through p a string header +// decString decodes byte array and stores through v a string header // describing the data. // Strings are encoded as an unsigned count followed by the raw bytes. -func decString(i *decInstr, state *decoderState, p unsafe.Pointer) { - if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(new(string)) - } - p = *(*unsafe.Pointer)(p) +func decString(i *decInstr, state *decoderState, value reflect.Value) { + u := state.decodeUint() + n := int(u) + if n < 0 || uint64(n) < u || n > state.b.Len() { + errorf("length of string exceeds input size (%d bytes)", n) } - n := state.decodeUint() - if n > uint64(state.b.Len()) { - errorf("string length exceeds input size (%d bytes)", n) + // Read the data. + data := make([]byte, n) + if _, err := state.b.Read(data); err != nil { + errorf("error decoding string: %s", err) } - b := make([]byte, n) - state.b.Read(b) - // It would be a shame to do the obvious thing here, - // *(*string)(p) = string(b) - // because we've already allocated the storage and this would - // allocate again and copy. So we do this ugly hack, which is even - // even more unsafe than it looks as it depends the memory - // representation of a string matching the beginning of the memory - // representation of a byte slice (a byte slice is longer). - *(*string)(p) = *(*string)(unsafe.Pointer(&b)) + i.decAlloc(value).SetString(string(data)) } // ignoreUint8Array skips over the data for a byte slice value with no destination. -func ignoreUint8Array(i *decInstr, state *decoderState, p unsafe.Pointer) { +func ignoreUint8Array(i *decInstr, state *decoderState, value reflect.Value) { b := make([]byte, state.decodeUint()) state.b.Read(b) } @@ -451,53 +392,50 @@ type decEngine struct { // allocate makes sure storage is available for an object of underlying type rtyp // that is indir levels of indirection through p. -func allocate(rtyp reflect.Type, p unsafe.Pointer, indir int) unsafe.Pointer { +func allocate(rtyp reflect.Type, v reflect.Value, indir int) reflect.Value { if indir == 0 { - return p + return v } - up := p if indir > 1 { - up = decIndirect(up, indir) + v = decIndirect(v, indir) } - if *(*unsafe.Pointer)(up) == nil { + if v.IsNil() { // Allocate object. - *(*unsafe.Pointer)(up) = unsafe.Pointer(reflect.New(rtyp).Pointer()) + v.Set(reflect.New(v.Type().Elem())) } - return *(*unsafe.Pointer)(up) + return v.Elem() } -// decodeSingle decodes a top-level value that is not a struct and stores it through p. +// decodeSingle decodes a top-level value that is not a struct and stores it in value. // Such values are preceded by a zero, making them have the memory layout of a // struct field (although with an illegal field number). -func (dec *Decoder) decodeSingle(engine *decEngine, ut *userTypeInfo, basep unsafe.Pointer) { +func (dec *Decoder) decodeSingle(engine *decEngine, ut *userTypeInfo, value reflect.Value) { state := dec.newDecoderState(&dec.buf) + defer dec.freeDecoderState(state) state.fieldnum = singletonField - delta := int(state.decodeUint()) - if delta != 0 { + if state.decodeUint() != 0 { errorf("decode: corrupted data: non-zero delta for singleton") } instr := &engine.instr[singletonField] if instr.indir != ut.indir { errorf("internal error: inconsistent indirection instr %d ut %d", instr.indir, ut.indir) } - ptr := basep // offset will be zero if instr.indir > 1 { - ptr = decIndirect(ptr, instr.indir) + value = decIndirect(value, instr.indir) } - instr.op(instr, state, ptr) - dec.freeDecoderState(state) + instr.op(instr, state, value) } -// decodeStruct decodes a top-level struct and stores it through p. +// decodeStruct decodes a top-level struct and stores it in value. // Indir is for the value, not the type. At the time of the call it may // differ from ut.indir, which was computed when the engine was built. // This state cannot arise for decodeSingle, which is called directly // from the user's value, not from the innards of an engine. -func (dec *Decoder) decodeStruct(engine *decEngine, ut *userTypeInfo, p unsafe.Pointer, indir int) { - p = allocate(ut.base, p, indir) +func (dec *Decoder) decodeStruct(engine *decEngine, ut *userTypeInfo, value reflect.Value, indir int) { + value = allocate(ut.base, value, indir) state := dec.newDecoderState(&dec.buf) + defer dec.freeDecoderState(state) state.fieldnum = -1 - basep := p for state.b.Len() > 0 { delta := int(state.decodeUint()) if delta < 0 { @@ -512,19 +450,25 @@ func (dec *Decoder) decodeStruct(engine *decEngine, ut *userTypeInfo, p unsafe.P break } instr := &engine.instr[fieldnum] - p := unsafe.Pointer(uintptr(basep) + instr.offset) - if instr.indir > 1 { - p = decIndirect(p, instr.indir) + var field reflect.Value + if instr.index != nil { + // Otherwise the field is unknown to us and instr.op is an ignore op. + field = value.FieldByIndex(instr.index) + if instr.indir > 1 { + field = decIndirect(field, instr.indir) + } } - instr.op(instr, state, p) + instr.op(instr, state, field) state.fieldnum = fieldnum } - dec.freeDecoderState(state) } +var zeroValue reflect.Value + // ignoreStruct discards the data for a struct with no destination. func (dec *Decoder) ignoreStruct(engine *decEngine) { state := dec.newDecoderState(&dec.buf) + defer dec.freeDecoderState(state) state.fieldnum = -1 for state.b.Len() > 0 { delta := int(state.decodeUint()) @@ -539,97 +483,88 @@ func (dec *Decoder) ignoreStruct(engine *decEngine) { error_(errRange) } instr := &engine.instr[fieldnum] - instr.op(instr, state, unsafe.Pointer(nil)) + instr.op(instr, state, zeroValue) state.fieldnum = fieldnum } - dec.freeDecoderState(state) } // ignoreSingle discards the data for a top-level non-struct value with no // destination. It's used when calling Decode with a nil value. func (dec *Decoder) ignoreSingle(engine *decEngine) { state := dec.newDecoderState(&dec.buf) + defer dec.freeDecoderState(state) state.fieldnum = singletonField delta := int(state.decodeUint()) if delta != 0 { errorf("decode: corrupted data: non-zero delta for singleton") } instr := &engine.instr[singletonField] - instr.op(instr, state, unsafe.Pointer(nil)) - dec.freeDecoderState(state) + instr.op(instr, state, zeroValue) } // decodeArrayHelper does the work for decoding arrays and slices. -func (dec *Decoder) decodeArrayHelper(state *decoderState, p unsafe.Pointer, elemOp decOp, elemWid uintptr, length, elemIndir int, ovfl error) { - instr := &decInstr{elemOp, 0, elemIndir, 0, ovfl} +func (dec *Decoder) decodeArrayHelper(state *decoderState, value reflect.Value, elemOp decOp, length, elemIndir int, ovfl error) { + instr := &decInstr{elemOp, 0, nil, elemIndir, ovfl} for i := 0; i < length; i++ { if state.b.Len() == 0 { errorf("decoding array or slice: length exceeds input size (%d elements)", length) } - up := p + elem := value.Index(i) if elemIndir > 1 { - up = decIndirect(up, elemIndir) + elem = decIndirect(elem, elemIndir) } - elemOp(instr, state, up) - p = unsafe.Pointer(uintptr(p) + elemWid) + elemOp(instr, state, elem) } } -// decodeArray decodes an array and stores it through p, that is, p points to the zeroth element. +// decodeArray decodes an array and stores it in value. // The length is an unsigned integer preceding the elements. Even though the length is redundant // (it's part of the type), it's a useful check and is included in the encoding. -func (dec *Decoder) decodeArray(atyp reflect.Type, state *decoderState, p unsafe.Pointer, elemOp decOp, elemWid uintptr, length, indir, elemIndir int, ovfl error) { +func (dec *Decoder) decodeArray(atyp reflect.Type, state *decoderState, value reflect.Value, elemOp decOp, length, indir, elemIndir int, ovfl error) { if indir > 0 { - p = allocate(atyp, p, 1) // All but the last level has been allocated by dec.Indirect + value = allocate(atyp, value, 1) // All but the last level has been allocated by dec.Indirect } if n := state.decodeUint(); n != uint64(length) { errorf("length mismatch in decodeArray") } - dec.decodeArrayHelper(state, p, elemOp, elemWid, length, elemIndir, ovfl) + dec.decodeArrayHelper(state, value, elemOp, length, elemIndir, ovfl) } -// decodeIntoValue is a helper for map decoding. Since maps are decoded using reflection, -// unlike the other items we can't use a pointer directly. -func decodeIntoValue(state *decoderState, op decOp, indir int, v reflect.Value, ovfl error) reflect.Value { - instr := &decInstr{op, 0, indir, 0, ovfl} - up := unsafeAddr(v) +// decodeIntoValue is a helper for map decoding. +func decodeIntoValue(state *decoderState, op decOp, indir int, value reflect.Value, ovfl error) reflect.Value { + instr := &decInstr{op, 0, nil, indir, ovfl} if indir > 1 { - up = decIndirect(up, indir) + value = decIndirect(value, indir) } - op(instr, state, up) - return v + op(instr, state, value) + return value } -// decodeMap decodes a map and stores its header through p. +// decodeMap decodes a map and stores it in value. // Maps are encoded as a length followed by key:value pairs. // Because the internals of maps are not visible to us, we must // use reflection rather than pointer magic. -func (dec *Decoder) decodeMap(mtyp reflect.Type, state *decoderState, p unsafe.Pointer, keyOp, elemOp decOp, indir, keyIndir, elemIndir int, ovfl error) { +func (dec *Decoder) decodeMap(mtyp reflect.Type, state *decoderState, value reflect.Value, keyOp, elemOp decOp, indir, keyIndir, elemIndir int, ovfl error) { if indir > 0 { - p = allocate(mtyp, p, 1) // All but the last level has been allocated by dec.Indirect + value = allocate(mtyp, value, 1) // All but the last level has been allocated by dec.Indirect } - up := unsafe.Pointer(p) - if *(*unsafe.Pointer)(up) == nil { // maps are represented as a pointer in the runtime + if value.IsNil() { // Allocate map. - *(*unsafe.Pointer)(up) = unsafe.Pointer(reflect.MakeMap(mtyp).Pointer()) + value.Set(reflect.MakeMap(mtyp)) } - // Maps cannot be accessed by moving addresses around the way - // that slices etc. can. We must recover a full reflection value for - // the iteration. - v := reflect.NewAt(mtyp, unsafe.Pointer(p)).Elem() n := int(state.decodeUint()) for i := 0; i < n; i++ { key := decodeIntoValue(state, keyOp, keyIndir, allocValue(mtyp.Key()), ovfl) elem := decodeIntoValue(state, elemOp, elemIndir, allocValue(mtyp.Elem()), ovfl) - v.SetMapIndex(key, elem) + value.SetMapIndex(key, elem) } } // ignoreArrayHelper does the work for discarding arrays and slices. func (dec *Decoder) ignoreArrayHelper(state *decoderState, elemOp decOp, length int) { - instr := &decInstr{elemOp, 0, 0, 0, errors.New("no error")} + instr := &decInstr{elemOp, 0, nil, 0, errors.New("no error")} for i := 0; i < length; i++ { - elemOp(instr, state, nil) + elemOp(instr, state, zeroValue) } } @@ -644,36 +579,52 @@ func (dec *Decoder) ignoreArray(state *decoderState, elemOp decOp, length int) { // ignoreMap discards the data for a map value with no destination. func (dec *Decoder) ignoreMap(state *decoderState, keyOp, elemOp decOp) { n := int(state.decodeUint()) - keyInstr := &decInstr{keyOp, 0, 0, 0, errors.New("no error")} - elemInstr := &decInstr{elemOp, 0, 0, 0, errors.New("no error")} + keyInstr := &decInstr{keyOp, 0, nil, 0, errors.New("no error")} + elemInstr := &decInstr{elemOp, 0, nil, 0, errors.New("no error")} for i := 0; i < n; i++ { - keyOp(keyInstr, state, nil) - elemOp(elemInstr, state, nil) + keyOp(keyInstr, state, zeroValue) + elemOp(elemInstr, state, zeroValue) } } -// decodeSlice decodes a slice and stores the slice header through p. +// decodeSlice decodes a slice and stores it in value. // Slices are encoded as an unsigned length followed by the elements. -func (dec *Decoder) decodeSlice(atyp reflect.Type, state *decoderState, p unsafe.Pointer, elemOp decOp, elemWid uintptr, indir, elemIndir int, ovfl error) { - nr := state.decodeUint() - n := int(nr) - if indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - // Allocate the slice header. - *(*unsafe.Pointer)(p) = unsafe.Pointer(new([]unsafe.Pointer)) +func (dec *Decoder) decodeSlice(state *decoderState, value reflect.Value, elemOp decOp, indir, elemIndir int, ovfl error) { + u := state.decodeUint() + n := int(u) + if n < 0 { + errorf("negative slice length: %d", n) + } + // Indirect if necessary until we have a settable slice header with enough storage. + typ := value.Type() + switch typ.Kind() { + default: + panic("should be slice " + typ.String()) + case reflect.Slice: + if value.Cap() < n { + value.Set(reflect.MakeSlice(typ, n, n)) } - p = *(*unsafe.Pointer)(p) - } - // Allocate storage for the slice elements, that is, the underlying array, - // if the existing slice does not have the capacity. - // Always write a header at p. - hdrp := (*reflect.SliceHeader)(p) - if hdrp.Cap < n { - hdrp.Data = reflect.MakeSlice(atyp, n, n).Pointer() - hdrp.Cap = n + case reflect.Ptr: + for typ.Elem().Kind() == reflect.Ptr { + if value.IsNil() { + value.Set(reflect.New(typ.Elem())) + } + value = value.Elem() + typ = typ.Elem() + } + // Value is now a pointer to a slice header. + // It might be nil. If so, allocate the header. + if value.IsNil() { + value.Set(reflect.New(typ.Elem())) + } + if value.Elem().IsNil() || value.Elem().Cap() < n { + value.Elem().Set(reflect.MakeSlice(typ.Elem(), n, n)) + } else { + value.Elem().Set(value.Elem().Slice(0, n)) + } + value = value.Elem() } - hdrp.Len = n - dec.decodeArrayHelper(state, unsafe.Pointer(hdrp.Data), elemOp, elemWid, n, elemIndir, ovfl) + dec.decodeArrayHelper(state, value, elemOp, n, elemIndir, ovfl) } // ignoreSlice skips over the data for a slice value with no destination. @@ -690,10 +641,10 @@ func setInterfaceValue(ivalue reflect.Value, value reflect.Value) { ivalue.Set(value) } -// decodeInterface decodes an interface value and stores it through p. +// decodeInterface decodes an interface value and stores it in value. // Interfaces are encoded as the name of a concrete type followed by a value. // If the name is empty, the value is nil and no value is sent. -func (dec *Decoder) decodeInterface(ityp reflect.Type, state *decoderState, p unsafe.Pointer, indir int) { +func (dec *Decoder) decodeInterface(ityp reflect.Type, state *decoderState, v reflect.Value, indir int) { // Create a writable interface reflect.Value. We need one even for the nil case. ivalue := allocValue(ityp) // Read the name of the concrete type. @@ -708,12 +659,11 @@ func (dec *Decoder) decodeInterface(ityp reflect.Type, state *decoderState, p un state.b.Read(b) name := string(b) if name == "" { - // Copy the representation of the nil interface value to the target. - // This is horribly unsafe and special. + // Copy the nil interface value to the target. if indir > 0 { - p = allocate(ityp, p, 1) // All but the last level has been allocated by dec.Indirect + v = allocate(ityp, v, 1) // All but the last level has been allocated by dec.Indirect } - *(*[2]uintptr)(unsafe.Pointer(p)) = ivalue.InterfaceData() + v.Set(ivalue) return } if len(name) > 1024 { @@ -742,14 +692,13 @@ func (dec *Decoder) decodeInterface(ityp reflect.Type, state *decoderState, p un } // Allocate the destination interface value. if indir > 0 { - p = allocate(ityp, p, 1) // All but the last level has been allocated by dec.Indirect + v = allocate(ityp, v, 1) // All but the last level has been allocated by dec.Indirect } // Assign the concrete value to the interface. // Tread carefully; it might not satisfy the interface. setInterfaceValue(ivalue, value) - // Copy the representation of the interface value to the target. - // This is horribly unsafe and special. - *(*[2]uintptr)(unsafe.Pointer(p)) = ivalue.InterfaceData() + // Copy the interface value to the target. + v.Set(value) } // ignoreInterface discards the data for an interface value with no destination. @@ -860,8 +809,8 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg elemId := dec.wireType[wireId].ArrayT.Elem elemOp, elemIndir := dec.decOpFor(elemId, t.Elem(), name, inProgress) ovfl := overflow(name) - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { - state.dec.decodeArray(t, state, p, *elemOp, t.Elem().Size(), t.Len(), i.indir, elemIndir, ovfl) + op = func(i *decInstr, state *decoderState, value reflect.Value) { + state.dec.decodeArray(t, state, value, *elemOp, t.Len(), i.indir, elemIndir, ovfl) } case reflect.Map: @@ -870,8 +819,8 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg keyOp, keyIndir := dec.decOpFor(keyId, t.Key(), "key of "+name, inProgress) elemOp, elemIndir := dec.decOpFor(elemId, t.Elem(), "element of "+name, inProgress) ovfl := overflow(name) - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { - state.dec.decodeMap(t, state, p, *keyOp, *elemOp, i.indir, keyIndir, elemIndir, ovfl) + op = func(i *decInstr, state *decoderState, value reflect.Value) { + state.dec.decodeMap(t, state, value, *keyOp, *elemOp, i.indir, keyIndir, elemIndir, ovfl) } case reflect.Slice: @@ -888,8 +837,8 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg } elemOp, elemIndir := dec.decOpFor(elemId, t.Elem(), name, inProgress) ovfl := overflow(name) - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { - state.dec.decodeSlice(t, state, p, *elemOp, t.Elem().Size(), i.indir, elemIndir, ovfl) + op = func(i *decInstr, state *decoderState, value reflect.Value) { + state.dec.decodeSlice(state, value, *elemOp, i.indir, elemIndir, ovfl) } case reflect.Struct: @@ -898,13 +847,13 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg if err != nil { error_(err) } - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { // indirect through enginePtr to delay evaluation for recursive structs. - dec.decodeStruct(*enginePtr, userType(typ), p, i.indir) + dec.decodeStruct(*enginePtr, userType(typ), value, i.indir) } case reflect.Interface: - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { - state.dec.decodeInterface(t, state, p, i.indir) + op = func(i *decInstr, state *decoderState, value reflect.Value) { + state.dec.decodeInterface(t, state, value, i.indir) } } } @@ -921,7 +870,7 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp { if wireId == tInterface { // Special case because it's a method: the ignored item might // define types and we need to record their state in the decoder. - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { state.dec.ignoreInterface(state) } return op @@ -934,7 +883,7 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp { case wire.ArrayT != nil: elemId := wire.ArrayT.Elem elemOp := dec.decIgnoreOpFor(elemId) - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { state.dec.ignoreArray(state, elemOp, wire.ArrayT.Len) } @@ -943,14 +892,14 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp { elemId := dec.wireType[wireId].MapT.Elem keyOp := dec.decIgnoreOpFor(keyId) elemOp := dec.decIgnoreOpFor(elemId) - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { state.dec.ignoreMap(state, keyOp, elemOp) } case wire.SliceT != nil: elemId := wire.SliceT.Elem elemOp := dec.decIgnoreOpFor(elemId) - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { state.dec.ignoreSlice(state, elemOp) } @@ -960,13 +909,13 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp { if err != nil { error_(err) } - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { // indirect through enginePtr to delay evaluation for recursive structs state.dec.ignoreStruct(*enginePtr) } case wire.GobEncoderT != nil, wire.BinaryMarshalerT != nil, wire.TextMarshalerT != nil: - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { state.dec.ignoreGobDecoder(state) } } @@ -989,25 +938,21 @@ func (dec *Decoder) gobDecodeOpFor(ut *userTypeInfo) (*decOp, int) { } } var op decOp - op = func(i *decInstr, state *decoderState, p unsafe.Pointer) { + op = func(i *decInstr, state *decoderState, value reflect.Value) { // Caller has gotten us to within one indirection of our value. if i.indir > 0 { - if *(*unsafe.Pointer)(p) == nil { - *(*unsafe.Pointer)(p) = unsafe.Pointer(reflect.New(ut.base).Pointer()) + if value.IsNil() { + value.Set(reflect.New(ut.base)) } } - // Now p is a pointer to the base type. Do we need to climb out to + // Now value is a pointer to the base type. Do we need to climb out to // get to the receiver type? - var v reflect.Value if ut.decIndir == -1 { - v = reflect.NewAt(rcvrType, unsafe.Pointer(&p)).Elem() - } else { - v = reflect.NewAt(rcvrType, p).Elem() + value = value.Addr() } - state.dec.decodeGobDecoder(ut, state, v) + state.dec.decodeGobDecoder(ut, state, value) } return &op, int(ut.indir) - } // compatibleType asks: Are these two gob Types compatible? @@ -1110,7 +1055,7 @@ func (dec *Decoder) compileSingle(remoteId typeId, ut *userTypeInfo) (engine *de } op, indir := dec.decOpFor(remoteId, rt, name, make(map[reflect.Type]*decOp)) ovfl := errors.New(`value for "` + name + `" out of range`) - engine.instr[singletonField] = decInstr{*op, singletonField, indir, 0, ovfl} + engine.instr[singletonField] = decInstr{*op, singletonField, nil, indir, ovfl} engine.numInstr = 1 return } @@ -1121,7 +1066,7 @@ func (dec *Decoder) compileIgnoreSingle(remoteId typeId) (engine *decEngine, err engine.instr = make([]decInstr, 1) // one item op := dec.decIgnoreOpFor(remoteId) ovfl := overflow(dec.typeString(remoteId)) - engine.instr[0] = decInstr{op, 0, 0, 0, ovfl} + engine.instr[0] = decInstr{op, 0, nil, 0, ovfl} engine.numInstr = 1 return } @@ -1164,14 +1109,14 @@ func (dec *Decoder) compileDec(remoteId typeId, ut *userTypeInfo) (engine *decEn // TODO(r): anonymous names if !present || !isExported(wireField.Name) { op := dec.decIgnoreOpFor(wireField.Id) - engine.instr[fieldnum] = decInstr{op, fieldnum, 0, 0, ovfl} + engine.instr[fieldnum] = decInstr{op, fieldnum, nil, 0, ovfl} continue } if !dec.compatibleType(localField.Type, wireField.Id, make(map[reflect.Type]typeId)) { errorf("wrong type (%s) for received field %s.%s", localField.Type, wireStruct.Name, wireField.Name) } op, indir := dec.decOpFor(wireField.Id, localField.Type, localField.Name, seen) - engine.instr[fieldnum] = decInstr{*op, fieldnum, indir, uintptr(localField.Offset), ovfl} + engine.instr[fieldnum] = decInstr{*op, fieldnum, localField.Index, indir, ovfl} engine.numInstr++ } return @@ -1222,16 +1167,16 @@ func (dec *Decoder) getIgnoreEnginePtr(wireId typeId) (enginePtr **decEngine, er return } -// decodeValue decodes the data stream representing a value and stores it in val. -func (dec *Decoder) decodeValue(wireId typeId, val reflect.Value) { +// decodeValue decodes the data stream representing a value and stores it in value. +func (dec *Decoder) decodeValue(wireId typeId, value reflect.Value) { defer catchError(&dec.err) // If the value is nil, it means we should just ignore this item. - if !val.IsValid() { + if !value.IsValid() { dec.decodeIgnoredValue(wireId) return } // Dereference down to the underlying type. - ut := userType(val.Type()) + ut := userType(value.Type()) base := ut.base var enginePtr **decEngine enginePtr, dec.err = dec.getDecEnginePtr(wireId, ut) @@ -1245,9 +1190,9 @@ func (dec *Decoder) decodeValue(wireId typeId, val reflect.Value) { name := base.Name() errorf("type mismatch: no fields matched compiling decoder for %s", name) } - dec.decodeStruct(engine, ut, unsafeAddr(val), ut.indir) + dec.decodeStruct(engine, ut, value, ut.indir) } else { - dec.decodeSingle(engine, ut, unsafeAddr(val)) + dec.decodeSingle(engine, ut, value) } } @@ -1293,21 +1238,6 @@ func init() { decOpTable[reflect.Uintptr] = uop } -// Gob assumes it can call UnsafeAddr on any Value -// in order to get a pointer it can copy data from. -// Values that have just been created and do not point -// into existing structs or slices cannot be addressed, -// so simulate it by returning a pointer to a copy. -// Each call allocates once. -func unsafeAddr(v reflect.Value) unsafe.Pointer { - if v.CanAddr() { - return unsafe.Pointer(v.UnsafeAddr()) - } - x := reflect.New(v.Type()).Elem() - x.Set(v) - return unsafe.Pointer(x.UnsafeAddr()) -} - // Gob depends on being able to take the address // of zeroed Values it creates, so use this wrapper instead // of the standard reflect.Zero. diff --git a/src/pkg/encoding/gob/encode.go b/src/pkg/encoding/gob/encode.go index 7831c02d13..54e1751f96 100644 --- a/src/pkg/encoding/gob/encode.go +++ b/src/pkg/encoding/gob/encode.go @@ -9,10 +9,9 @@ import ( "encoding" "math" "reflect" - "unsafe" ) -const uint64Size = int(unsafe.Sizeof(uint64(0))) +const uint64Size = 8 // encoderState is the global execution state of an instance of the encoder. // Field numbers are delta encoded and always increase. The field @@ -87,14 +86,14 @@ func (state *encoderState) encodeInt(i int64) { } // encOp is the signature of an encoding operator for a given type. -type encOp func(i *encInstr, state *encoderState, p unsafe.Pointer) +type encOp func(i *encInstr, state *encoderState, v reflect.Value) // The 'instructions' of the encoding machine type encInstr struct { - op encOp - field int // field number - indir int // how many pointer indirections to reach the value in the struct - offset uintptr // offset in the structure of the field to encode + op encOp + field int // field number in input + index []int // struct index + indir int // how many pointer indirections to reach the value in the struct } // update emits a field number and updates the state to record its value for delta encoding. @@ -115,20 +114,20 @@ func (state *encoderState) update(instr *encInstr) { // encoded integer, followed by the field data in its appropriate // format. -// encIndirect dereferences p indir times and returns the result. -func encIndirect(p unsafe.Pointer, indir int) unsafe.Pointer { +// encIndirect dereferences pv indir times and returns the result. +func encIndirect(pv reflect.Value, indir int) reflect.Value { for ; indir > 0; indir-- { - p = *(*unsafe.Pointer)(p) - if p == nil { - return unsafe.Pointer(nil) + if pv.IsNil() { + break } + pv = pv.Elem() } - return p + return pv } -// encBool encodes the bool with address p as an unsigned 0 or 1. -func encBool(i *encInstr, state *encoderState, p unsafe.Pointer) { - b := *(*bool)(p) +// encBool encodes the bool referenced by v as an unsigned 0 or 1. +func encBool(i *encInstr, state *encoderState, v reflect.Value) { + b := v.Bool() if b || state.sendZero { state.update(i) if b { @@ -139,102 +138,21 @@ func encBool(i *encInstr, state *encoderState, p unsafe.Pointer) { } } -// encInt encodes the int with address p. -func encInt(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := int64(*(*int)(p)) - if v != 0 || state.sendZero { +// encInt encodes the signed integer (int int8 int16 int32 int64) referenced by v. +func encInt(i *encInstr, state *encoderState, v reflect.Value) { + value := v.Int() + if value != 0 || state.sendZero { state.update(i) - state.encodeInt(v) + state.encodeInt(value) } } -// encUint encodes the uint with address p. -func encUint(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := uint64(*(*uint)(p)) - if v != 0 || state.sendZero { +// encUint encodes the unsigned integer (uint uint8 uint16 uint32 uint64 uintptr) referenced by v. +func encUint(i *encInstr, state *encoderState, v reflect.Value) { + value := v.Uint() + if value != 0 || state.sendZero { state.update(i) - state.encodeUint(v) - } -} - -// encInt8 encodes the int8 with address p. -func encInt8(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := int64(*(*int8)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeInt(v) - } -} - -// encUint8 encodes the uint8 with address p. -func encUint8(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := uint64(*(*uint8)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeUint(v) - } -} - -// encInt16 encodes the int16 with address p. -func encInt16(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := int64(*(*int16)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeInt(v) - } -} - -// encUint16 encodes the uint16 with address p. -func encUint16(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := uint64(*(*uint16)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeUint(v) - } -} - -// encInt32 encodes the int32 with address p. -func encInt32(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := int64(*(*int32)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeInt(v) - } -} - -// encUint encodes the uint32 with address p. -func encUint32(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := uint64(*(*uint32)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeUint(v) - } -} - -// encInt64 encodes the int64 with address p. -func encInt64(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := *(*int64)(p) - if v != 0 || state.sendZero { - state.update(i) - state.encodeInt(v) - } -} - -// encInt64 encodes the uint64 with address p. -func encUint64(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := *(*uint64)(p) - if v != 0 || state.sendZero { - state.update(i) - state.encodeUint(v) - } -} - -// encUintptr encodes the uintptr with address p. -func encUintptr(i *encInstr, state *encoderState, p unsafe.Pointer) { - v := uint64(*(*uintptr)(p)) - if v != 0 || state.sendZero { - state.update(i) - state.encodeUint(v) + state.encodeUint(value) } } @@ -255,42 +173,20 @@ func floatBits(f float64) uint64 { return v } -// encFloat32 encodes the float32 with address p. -func encFloat32(i *encInstr, state *encoderState, p unsafe.Pointer) { - f := *(*float32)(p) - if f != 0 || state.sendZero { - v := floatBits(float64(f)) - state.update(i) - state.encodeUint(v) - } -} - -// encFloat64 encodes the float64 with address p. -func encFloat64(i *encInstr, state *encoderState, p unsafe.Pointer) { - f := *(*float64)(p) +// encFloat encodes the floating point value (float32 float64) referenced by v. +func encFloat(i *encInstr, state *encoderState, v reflect.Value) { + f := v.Float() if f != 0 || state.sendZero { + bits := floatBits(f) state.update(i) - v := floatBits(f) - state.encodeUint(v) + state.encodeUint(bits) } } -// encComplex64 encodes the complex64 with address p. +// encComplex encodes the complex value (complex64 complex128) referenced by v. // Complex numbers are just a pair of floating-point numbers, real part first. -func encComplex64(i *encInstr, state *encoderState, p unsafe.Pointer) { - c := *(*complex64)(p) - if c != 0+0i || state.sendZero { - rpart := floatBits(float64(real(c))) - ipart := floatBits(float64(imag(c))) - state.update(i) - state.encodeUint(rpart) - state.encodeUint(ipart) - } -} - -// encComplex128 encodes the complex128 with address p. -func encComplex128(i *encInstr, state *encoderState, p unsafe.Pointer) { - c := *(*complex128)(p) +func encComplex(i *encInstr, state *encoderState, v reflect.Value) { + c := v.Complex() if c != 0+0i || state.sendZero { rpart := floatBits(real(c)) ipart := floatBits(imag(c)) @@ -300,10 +196,10 @@ func encComplex128(i *encInstr, state *encoderState, p unsafe.Pointer) { } } -// encUint8Array encodes the byte slice whose header has address p. +// encUint8Array encodes the byte array referenced by v. // Byte arrays are encoded as an unsigned count followed by the raw bytes. -func encUint8Array(i *encInstr, state *encoderState, p unsafe.Pointer) { - b := *(*[]byte)(p) +func encUint8Array(i *encInstr, state *encoderState, v reflect.Value) { + b := v.Bytes() if len(b) > 0 || state.sendZero { state.update(i) state.encodeUint(uint64(len(b))) @@ -311,10 +207,10 @@ func encUint8Array(i *encInstr, state *encoderState, p unsafe.Pointer) { } } -// encString encodes the string whose header has address p. +// encString encodes the string referenced by v. // Strings are encoded as an unsigned count followed by the raw bytes. -func encString(i *encInstr, state *encoderState, p unsafe.Pointer) { - s := *(*string)(p) +func encString(i *encInstr, state *encoderState, v reflect.Value) { + s := v.String() if len(s) > 0 || state.sendZero { state.update(i) state.encodeUint(uint64(len(s))) @@ -324,7 +220,7 @@ func encString(i *encInstr, state *encoderState, p unsafe.Pointer) { // encStructTerminator encodes the end of an encoded struct // as delta field number of 0. -func encStructTerminator(i *encInstr, state *encoderState, p unsafe.Pointer) { +func encStructTerminator(i *encInstr, state *encoderState, v reflect.Value) { state.encodeUint(0) } @@ -338,60 +234,78 @@ type encEngine struct { const singletonField = 0 +// valid reports whether the value is valid and a non-nil pointer. +// (Slices, maps, and chans take care of themselves.) +func valid(v reflect.Value) bool { + switch v.Kind() { + case reflect.Invalid: + return false + case reflect.Ptr: + return !v.IsNil() + } + return true +} + // encodeSingle encodes a single top-level non-struct value. -func (enc *Encoder) encodeSingle(b *bytes.Buffer, engine *encEngine, basep unsafe.Pointer) { +func (enc *Encoder) encodeSingle(b *bytes.Buffer, engine *encEngine, value reflect.Value) { state := enc.newEncoderState(b) + defer enc.freeEncoderState(state) state.fieldnum = singletonField // There is no surrounding struct to frame the transmission, so we must // generate data even if the item is zero. To do this, set sendZero. state.sendZero = true instr := &engine.instr[singletonField] - p := basep // offset will be zero if instr.indir > 0 { - if p = encIndirect(p, instr.indir); p == nil { - return - } + value = encIndirect(value, instr.indir) + } + if valid(value) { + instr.op(instr, state, value) } - instr.op(instr, state, p) - enc.freeEncoderState(state) } // encodeStruct encodes a single struct value. -func (enc *Encoder) encodeStruct(b *bytes.Buffer, engine *encEngine, basep unsafe.Pointer) { +func (enc *Encoder) encodeStruct(b *bytes.Buffer, engine *encEngine, value reflect.Value) { + if !valid(value) { + return + } state := enc.newEncoderState(b) + defer enc.freeEncoderState(state) state.fieldnum = -1 for i := 0; i < len(engine.instr); i++ { instr := &engine.instr[i] - p := unsafe.Pointer(uintptr(basep) + instr.offset) + if i >= value.NumField() { + // encStructTerminator + instr.op(instr, state, reflect.Value{}) + break + } + field := value.FieldByIndex(instr.index) if instr.indir > 0 { - if p = encIndirect(p, instr.indir); p == nil { - continue - } + field = encIndirect(field, instr.indir) + } + if !valid(field) { + continue } - instr.op(instr, state, p) + instr.op(instr, state, field) } - enc.freeEncoderState(state) } // encodeArray encodes the array whose 0th element is at p. -func (enc *Encoder) encodeArray(b *bytes.Buffer, p unsafe.Pointer, op encOp, elemWid uintptr, elemIndir int, length int) { +func (enc *Encoder) encodeArray(b *bytes.Buffer, value reflect.Value, op encOp, elemIndir int, length int) { state := enc.newEncoderState(b) + defer enc.freeEncoderState(state) state.fieldnum = -1 state.sendZero = true state.encodeUint(uint64(length)) for i := 0; i < length; i++ { - elemp := p + elem := value.Index(i) if elemIndir > 0 { - up := encIndirect(elemp, elemIndir) - if up == nil { + elem = encIndirect(elem, elemIndir) + if !valid(elem) { errorf("encodeArray: nil element") } - elemp = up } - op(nil, state, elemp) - p = unsafe.Pointer(uintptr(p) + elemWid) + op(nil, state, elem) } - enc.freeEncoderState(state) } // encodeReflectValue is a helper for maps. It encodes the value v. @@ -402,12 +316,10 @@ func encodeReflectValue(state *encoderState, v reflect.Value, op encOp, indir in if !v.IsValid() { errorf("encodeReflectValue: nil element") } - op(nil, state, unsafeAddr(v)) + op(nil, state, v) } // encodeMap encodes a map as unsigned count followed by key:value pairs. -// Because map internals are not exposed, we must use reflection rather than -// addresses. func (enc *Encoder) encodeMap(b *bytes.Buffer, mv reflect.Value, keyOp, elemOp encOp, keyIndir, elemIndir int) { state := enc.newEncoderState(b) state.fieldnum = -1 @@ -539,20 +451,20 @@ func (enc *Encoder) encodeGobEncoder(b *bytes.Buffer, ut *userTypeInfo, v reflec var encOpTable = [...]encOp{ reflect.Bool: encBool, reflect.Int: encInt, - reflect.Int8: encInt8, - reflect.Int16: encInt16, - reflect.Int32: encInt32, - reflect.Int64: encInt64, + reflect.Int8: encInt, + reflect.Int16: encInt, + reflect.Int32: encInt, + reflect.Int64: encInt, reflect.Uint: encUint, - reflect.Uint8: encUint8, - reflect.Uint16: encUint16, - reflect.Uint32: encUint32, - reflect.Uint64: encUint64, - reflect.Uintptr: encUintptr, - reflect.Float32: encFloat32, - reflect.Float64: encFloat64, - reflect.Complex64: encComplex64, - reflect.Complex128: encComplex128, + reflect.Uint8: encUint, + reflect.Uint16: encUint, + reflect.Uint32: encUint, + reflect.Uint64: encUint, + reflect.Uintptr: encUint, + reflect.Float32: encFloat, + reflect.Float64: encFloat, + reflect.Complex64: encComplex, + reflect.Complex128: encComplex, reflect.String: encString, } @@ -587,30 +499,24 @@ func (enc *Encoder) encOpFor(rt reflect.Type, inProgress map[reflect.Type]*encOp } // Slices have a header; we decode it to find the underlying array. elemOp, elemIndir := enc.encOpFor(t.Elem(), inProgress) - op = func(i *encInstr, state *encoderState, p unsafe.Pointer) { - slice := (*reflect.SliceHeader)(p) - if !state.sendZero && slice.Len == 0 { + op = func(i *encInstr, state *encoderState, slice reflect.Value) { + if !state.sendZero && slice.Len() == 0 { return } state.update(i) - state.enc.encodeArray(state.b, unsafe.Pointer(slice.Data), *elemOp, t.Elem().Size(), elemIndir, int(slice.Len)) + state.enc.encodeArray(state.b, slice, *elemOp, elemIndir, slice.Len()) } case reflect.Array: // True arrays have size in the type. elemOp, elemIndir := enc.encOpFor(t.Elem(), inProgress) - op = func(i *encInstr, state *encoderState, p unsafe.Pointer) { + op = func(i *encInstr, state *encoderState, array reflect.Value) { state.update(i) - state.enc.encodeArray(state.b, p, *elemOp, t.Elem().Size(), elemIndir, t.Len()) + state.enc.encodeArray(state.b, array, *elemOp, elemIndir, array.Len()) } case reflect.Map: keyOp, keyIndir := enc.encOpFor(t.Key(), inProgress) elemOp, elemIndir := enc.encOpFor(t.Elem(), inProgress) - op = func(i *encInstr, state *encoderState, p unsafe.Pointer) { - // Maps cannot be accessed by moving addresses around the way - // that slices etc. can. We must recover a full reflection value for - // the iteration. - v := reflect.NewAt(t, unsafe.Pointer(p)).Elem() - mv := reflect.Indirect(v) + op = func(i *encInstr, state *encoderState, mv reflect.Value) { // We send zero-length (but non-nil) maps because the // receiver might want to use the map. (Maps don't use append.) if !state.sendZero && mv.IsNil() { @@ -623,17 +529,13 @@ func (enc *Encoder) encOpFor(rt reflect.Type, inProgress map[reflect.Type]*encOp // Generate a closure that calls out to the engine for the nested type. enc.getEncEngine(userType(typ)) info := mustGetTypeInfo(typ) - op = func(i *encInstr, state *encoderState, p unsafe.Pointer) { + op = func(i *encInstr, state *encoderState, sv reflect.Value) { state.update(i) // indirect through info to delay evaluation for recursive structs - state.enc.encodeStruct(state.b, info.encoder, p) + state.enc.encodeStruct(state.b, info.encoder, sv) } case reflect.Interface: - op = func(i *encInstr, state *encoderState, p unsafe.Pointer) { - // Interfaces transmit the name and contents of the concrete - // value they contain. - v := reflect.NewAt(t, unsafe.Pointer(p)).Elem() - iv := reflect.Indirect(v) + op = func(i *encInstr, state *encoderState, iv reflect.Value) { if !state.sendZero && (!iv.IsValid() || iv.IsNil()) { return } @@ -660,13 +562,13 @@ func (enc *Encoder) gobEncodeOpFor(ut *userTypeInfo) (*encOp, int) { } } var op encOp - op = func(i *encInstr, state *encoderState, p unsafe.Pointer) { - var v reflect.Value + op = func(i *encInstr, state *encoderState, v reflect.Value) { if ut.encIndir == -1 { // Need to climb up one level to turn value into pointer. - v = reflect.NewAt(rt, unsafe.Pointer(&p)).Elem() - } else { - v = reflect.NewAt(rt, p).Elem() + if !v.CanAddr() { + errorf("unaddressable value of type %s", rt) + } + v = v.Addr() } if !state.sendZero && isZero(v) { return @@ -693,17 +595,17 @@ func (enc *Encoder) compileEnc(ut *userTypeInfo) *encEngine { continue } op, indir := enc.encOpFor(f.Type, seen) - engine.instr = append(engine.instr, encInstr{*op, wireFieldNum, indir, uintptr(f.Offset)}) + engine.instr = append(engine.instr, encInstr{*op, wireFieldNum, f.Index, indir}) wireFieldNum++ } if srt.NumField() > 0 && len(engine.instr) == 0 { errorf("type %s has no exported fields", rt) } - engine.instr = append(engine.instr, encInstr{encStructTerminator, 0, 0, 0}) + engine.instr = append(engine.instr, encInstr{encStructTerminator, 0, nil, 0}) } else { engine.instr = make([]encInstr, 1) op, indir := enc.encOpFor(rt, seen) - engine.instr[0] = encInstr{*op, singletonField, indir, 0} // offset is zero + engine.instr[0] = encInstr{*op, singletonField, nil, indir} } return engine } @@ -753,8 +655,8 @@ func (enc *Encoder) encode(b *bytes.Buffer, value reflect.Value, ut *userTypeInf value = reflect.Indirect(value) } if ut.externalEnc == 0 && value.Type().Kind() == reflect.Struct { - enc.encodeStruct(b, engine, unsafeAddr(value)) + enc.encodeStruct(b, engine, value) } else { - enc.encodeSingle(b, engine, unsafeAddr(value)) + enc.encodeSingle(b, engine, value) } } diff --git a/src/pkg/encoding/gob/encoder_test.go b/src/pkg/encoding/gob/encoder_test.go index 6445ce1002..376df82f15 100644 --- a/src/pkg/encoding/gob/encoder_test.go +++ b/src/pkg/encoding/gob/encoder_test.go @@ -13,6 +13,52 @@ import ( "testing" ) +// Test basic operations in a safe manner. +func TestBasicEncoderDecoder(t *testing.T) { + var values = []interface{}{ + true, + int(123), + int8(123), + int16(-12345), + int32(123456), + int64(-1234567), + uint(123), + uint8(123), + uint16(12345), + uint32(123456), + uint64(1234567), + uintptr(12345678), + float32(1.2345), + float64(1.2345678), + complex64(1.2345 + 2.3456i), + complex128(1.2345678 + 2.3456789i), + []byte("hello"), + string("hello"), + } + for _, value := range values { + b := new(bytes.Buffer) + enc := NewEncoder(b) + err := enc.Encode(value) + if err != nil { + t.Error("encoder fail:", err) + } + dec := NewDecoder(b) + result := reflect.New(reflect.TypeOf(value)) + err = dec.Decode(result.Interface()) + if err != nil { + t.Fatalf("error decoding %T: %v:", reflect.TypeOf(value), err) + } + if !reflect.DeepEqual(value, result.Elem().Interface()) { + t.Fatalf("%T: expected %v got %v", value, value, result.Elem().Interface()) + } + } +} + +type ET0 struct { + A int + B string +} + type ET2 struct { X string } @@ -40,14 +86,40 @@ type ET4 struct { func TestEncoderDecoder(t *testing.T) { b := new(bytes.Buffer) enc := NewEncoder(b) + et0 := new(ET0) + et0.A = 7 + et0.B = "gobs of fun" + err := enc.Encode(et0) + if err != nil { + t.Error("encoder fail:", err) + } + //fmt.Printf("% x %q\n", b, b) + //Debug(b) + dec := NewDecoder(b) + newEt0 := new(ET0) + err = dec.Decode(newEt0) + if err != nil { + t.Fatal("error decoding ET0:", err) + } + + if !reflect.DeepEqual(et0, newEt0) { + t.Fatalf("invalid data for et0: expected %+v; got %+v", *et0, *newEt0) + } + if b.Len() != 0 { + t.Error("not at eof;", b.Len(), "bytes left") + } + // t.FailNow() + + b = new(bytes.Buffer) + enc = NewEncoder(b) et1 := new(ET1) et1.A = 7 et1.Et2 = new(ET2) - err := enc.Encode(et1) + err = enc.Encode(et1) if err != nil { t.Error("encoder fail:", err) } - dec := NewDecoder(b) + dec = NewDecoder(b) newEt1 := new(ET1) err = dec.Decode(newEt1) if err != nil { diff --git a/src/pkg/encoding/gob/gobencdec_test.go b/src/pkg/encoding/gob/gobencdec_test.go index 157b7723a7..eb76b481d1 100644 --- a/src/pkg/encoding/gob/gobencdec_test.go +++ b/src/pkg/encoding/gob/gobencdec_test.go @@ -279,7 +279,7 @@ func TestGobEncoderValueField(t *testing.T) { b := new(bytes.Buffer) // First a field that's a structure. enc := NewEncoder(b) - err := enc.Encode(GobTestValueEncDec{17, StringStruct{"HIJKL"}}) + err := enc.Encode(&GobTestValueEncDec{17, StringStruct{"HIJKL"}}) if err != nil { t.Fatal("encode error:", err) } @@ -326,7 +326,7 @@ func TestGobEncoderArrayField(t *testing.T) { for i := range a.A.a { a.A.a[i] = byte(i) } - err := enc.Encode(a) + err := enc.Encode(&a) if err != nil { t.Fatal("encode error:", err) } @@ -589,7 +589,8 @@ func TestGobEncoderStructSingleton(t *testing.T) { func TestGobEncoderNonStructSingleton(t *testing.T) { b := new(bytes.Buffer) enc := NewEncoder(b) - err := enc.Encode(Gobber(1234)) + var g Gobber = 1234 + err := enc.Encode(&g) if err != nil { t.Fatal("encode error:", err) } diff --git a/src/pkg/encoding/gob/timing_test.go b/src/pkg/encoding/gob/timing_test.go index 9fbb0ac6d5..acfb065b12 100644 --- a/src/pkg/encoding/gob/timing_test.go +++ b/src/pkg/encoding/gob/timing_test.go @@ -23,7 +23,7 @@ func benchmarkEndToEnd(r io.Reader, w io.Writer, b *testing.B) { b.StopTimer() enc := NewEncoder(w) dec := NewDecoder(r) - bench := &Bench{7, 3.2, "now is the time", []byte("for all good men")} + bench := &Bench{7, 3.2, "now is the time", bytes.Repeat([]byte("for all good men"), 100)} b.StartTimer() for i := 0; i < b.N; i++ { if enc.Encode(bench) != nil { @@ -103,7 +103,7 @@ func TestCountDecodeMallocs(t *testing.T) { t.Fatal("decode:", err) } }) - if allocs != 3 { - t.Fatalf("mallocs per decode of type Bench: %v; wanted 3\n", allocs) + if allocs != 4 { + t.Fatalf("mallocs per decode of type Bench: %v; wanted 4\n", allocs) } } -- 2.48.1