From 0297427ef56e1b6b8aa672f0bb71f50e2155ce63 Mon Sep 17 00:00:00 2001 From: Masaaki Goshima Date: Mon, 1 Feb 2021 18:43:28 +0900 Subject: [PATCH 1/5] Fix encoding of MarshalJSON type --- encode_vm_escaped.go | 71 +++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/encode_vm_escaped.go b/encode_vm_escaped.go index 7bc194f..47f6486 100644 --- a/encode_vm_escaped.go +++ b/encode_vm_escaped.go @@ -7801,6 +7801,15 @@ func encodeRunEscaped(ctx *encodeRuntimeContext, b []byte, codeSet *opcodeSet, o ptr := load(ctxptr, code.headIdx) b = append(b, code.escapedKey...) p := ptr + code.offset + if code.typ.Kind() == reflect.Ptr { + p = ptrToPtr(p) + } + if p == 0 { + b = encodeNull(b) + b = encodeComma(b) + code = code.next + break + } v := ptrToInterface(code, p) bb, err := v.(Marshaler).MarshalJSON() if err != nil { @@ -7816,23 +7825,25 @@ func encodeRunEscaped(ctx *encodeRuntimeContext, b []byte, codeSet *opcodeSet, o case opStructFieldOmitEmptyMarshalJSON: ptr := load(ctxptr, code.headIdx) p := ptr + code.offset - if code.typ.Kind() == reflect.Ptr && code.typ.Elem().Implements(marshalJSONType) { + if code.typ.Kind() == reflect.Ptr { p = ptrToPtr(p) } - v := ptrToInterface(code, p) - if v != nil && p != 0 { - bb, err := v.(Marshaler).MarshalJSON() - if err != nil { - return nil, errMarshaler(code, err) - } - b = append(b, code.escapedKey...) - buf := bytes.NewBuffer(b) - if err := compact(buf, bb, true); err != nil { - return nil, err - } - b = buf.Bytes() - b = encodeComma(b) + if p == 0 { + code = code.next + break } + v := ptrToInterface(code, p) + bb, err := v.(Marshaler).MarshalJSON() + if err != nil { + return nil, errMarshaler(code, err) + } + b = append(b, code.escapedKey...) + buf := bytes.NewBuffer(b) + if err := compact(buf, bb, true); err != nil { + return nil, err + } + b = buf.Bytes() + b = encodeComma(b) code = code.next case opStructFieldStringTagMarshalJSON: ptr := load(ctxptr, code.headIdx) @@ -9256,20 +9267,10 @@ func encodeRunEscaped(ctx *encodeRuntimeContext, b []byte, codeSet *opcodeSet, o case opStructEndOmitEmptyMarshalJSON: ptr := load(ctxptr, code.headIdx) p := ptr + code.offset - v := ptrToInterface(code, p) - if v != nil && (code.typ.Kind() != reflect.Ptr || ptrToPtr(p) != 0) { - bb, err := v.(Marshaler).MarshalJSON() - if err != nil { - return nil, errMarshaler(code, err) - } - b = append(b, code.escapedKey...) - buf := bytes.NewBuffer(b) - if err := compact(buf, bb, true); err != nil { - return nil, err - } - b = buf.Bytes() - b = appendStructEnd(b) - } else { + if code.typ.Kind() == reflect.Ptr { + p = ptrToPtr(p) + } + if p == 0 { last := len(b) - 1 if b[last] == ',' { b[last] = '}' @@ -9277,7 +9278,21 @@ func encodeRunEscaped(ctx *encodeRuntimeContext, b []byte, codeSet *opcodeSet, o } else { b = appendStructEnd(b) } + code = code.next + break } + v := ptrToInterface(code, p) + bb, err := v.(Marshaler).MarshalJSON() + if err != nil { + return nil, errMarshaler(code, err) + } + b = append(b, code.escapedKey...) + buf := bytes.NewBuffer(b) + if err := compact(buf, bb, true); err != nil { + return nil, err + } + b = buf.Bytes() + b = appendStructEnd(b) code = code.next case opStructEndStringTagMarshalJSON: ptr := load(ctxptr, code.headIdx) From b431a095d6ce12c9b119a327cbb919614464b923 Mon Sep 17 00:00:00 2001 From: Masaaki Goshima Date: Mon, 1 Feb 2021 20:02:43 +0900 Subject: [PATCH 2/5] Fix error by race detector --- encode_compile.go | 41 +++++++------------------------------ encode_compile_norace.go | 34 +++++++++++++++++++++++++++++++ encode_compile_race.go | 44 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 encode_compile_norace.go create mode 100644 encode_compile_race.go diff --git a/encode_compile.go b/encode_compile.go index 04e256c..06f21a0 100644 --- a/encode_compile.go +++ b/encode_compile.go @@ -22,11 +22,12 @@ type opcodeSet struct { } var ( - marshalJSONType = reflect.TypeOf((*Marshaler)(nil)).Elem() - marshalTextType = reflect.TypeOf((*encoding.TextMarshaler)(nil)).Elem() - cachedOpcode unsafe.Pointer // map[uintptr]*opcodeSet - baseTypeAddr uintptr - cachedOpcodeSets []*opcodeSet + marshalJSONType = reflect.TypeOf((*Marshaler)(nil)).Elem() + marshalTextType = reflect.TypeOf((*encoding.TextMarshaler)(nil)).Elem() + cachedOpcode unsafe.Pointer // map[uintptr]*opcodeSet + baseTypeAddr uintptr + cachedOpcodeSets []*opcodeSet + existsCachedOpcodeSets bool ) const ( @@ -80,6 +81,7 @@ func setupOpcodeSets() error { return fmt.Errorf("too big address range %d", addrRange) } cachedOpcodeSets = make([]*opcodeSet, addrRange) + existsCachedOpcodeSets = true baseTypeAddr = min return nil } @@ -90,35 +92,6 @@ func init() { } } -func encodeCompileToGetCodeSet(typeptr uintptr) (*opcodeSet, error) { - if cachedOpcodeSets == nil { - return encodeCompileToGetCodeSetSlowPath(typeptr) - } - if codeSet := cachedOpcodeSets[typeptr-baseTypeAddr]; codeSet != nil { - return codeSet, nil - } - - // noescape trick for header.typ ( reflect.*rtype ) - copiedType := *(**rtype)(unsafe.Pointer(&typeptr)) - - code, err := encodeCompileHead(&encodeCompileContext{ - typ: copiedType, - root: true, - structTypeToCompiledCode: map[uintptr]*compiledCode{}, - }) - if err != nil { - return nil, err - } - code = copyOpcode(code) - codeLength := code.totalLength() - codeSet := &opcodeSet{ - code: code, - codeLength: codeLength, - } - cachedOpcodeSets[int(typeptr-baseTypeAddr)] = codeSet - return codeSet, nil -} - func encodeCompileToGetCodeSetSlowPath(typeptr uintptr) (*opcodeSet, error) { opcodeMap := loadOpcodeMap() if codeSet, exists := opcodeMap[typeptr]; exists { diff --git a/encode_compile_norace.go b/encode_compile_norace.go new file mode 100644 index 0000000..209f60c --- /dev/null +++ b/encode_compile_norace.go @@ -0,0 +1,34 @@ +// +build !race + +package json + +import "unsafe" + +func encodeCompileToGetCodeSet(typeptr uintptr) (*opcodeSet, error) { + if !existsCachedOpcodeSets { + return encodeCompileToGetCodeSetSlowPath(typeptr) + } + if codeSet := cachedOpcodeSets[typeptr-baseTypeAddr]; codeSet != nil { + return codeSet, nil + } + + // noescape trick for header.typ ( reflect.*rtype ) + copiedType := *(**rtype)(unsafe.Pointer(&typeptr)) + + code, err := encodeCompileHead(&encodeCompileContext{ + typ: copiedType, + root: true, + structTypeToCompiledCode: map[uintptr]*compiledCode{}, + }) + if err != nil { + return nil, err + } + code = copyOpcode(code) + codeLength := code.totalLength() + codeSet := &opcodeSet{ + code: code, + codeLength: codeLength, + } + cachedOpcodeSets[int(typeptr-baseTypeAddr)] = codeSet + return codeSet, nil +} diff --git a/encode_compile_race.go b/encode_compile_race.go new file mode 100644 index 0000000..a2fcb73 --- /dev/null +++ b/encode_compile_race.go @@ -0,0 +1,44 @@ +// +build race + +package json + +import ( + "sync" + "unsafe" +) + +var setsMu sync.RWMutex + +func encodeCompileToGetCodeSet(typeptr uintptr) (*opcodeSet, error) { + if !existsCachedOpcodeSets { + return encodeCompileToGetCodeSetSlowPath(typeptr) + } + setsMu.RLock() + if codeSet := cachedOpcodeSets[typeptr-baseTypeAddr]; codeSet != nil { + setsMu.RUnlock() + return codeSet, nil + } + setsMu.RUnlock() + + // noescape trick for header.typ ( reflect.*rtype ) + copiedType := *(**rtype)(unsafe.Pointer(&typeptr)) + + code, err := encodeCompileHead(&encodeCompileContext{ + typ: copiedType, + root: true, + structTypeToCompiledCode: map[uintptr]*compiledCode{}, + }) + if err != nil { + return nil, err + } + code = copyOpcode(code) + codeLength := code.totalLength() + codeSet := &opcodeSet{ + code: code, + codeLength: codeLength, + } + setsMu.Lock() + cachedOpcodeSets[int(typeptr-baseTypeAddr)] = codeSet + setsMu.Unlock() + return codeSet, nil +} From 2a0d4603ea96242bf6944a55296ec3be065bf11d Mon Sep 17 00:00:00 2001 From: Masaaki Goshima Date: Mon, 1 Feb 2021 22:31:39 +0900 Subject: [PATCH 3/5] Fix error output by golangci-lint --- decode.go | 8 +- decode_bytes.go | 1 - decode_compile.go | 6 +- decode_context.go | 1 - decode_int.go | 1 - decode_interface.go | 2 - decode_map.go | 2 - decode_ptr.go | 1 + decode_stream.go | 2 - decode_string.go | 2 +- decode_struct.go | 1 - decode_uint.go | 4 +- encode.go | 10 +- encode_compile.go | 14 +- encode_context.go | 1 - encode_opcode.go | 1 - encode_string.go | 371 ------------------------------------------- encode_vm.go | 3 +- encode_vm_escaped.go | 2 +- indent.go | 2 +- struct_field.go | 5 +- 21 files changed, 25 insertions(+), 415 deletions(-) diff --git a/decode.go b/decode.go index 255a7b5..66dcc17 100644 --- a/decode.go +++ b/decode.go @@ -21,9 +21,8 @@ type decoder interface { } type Decoder struct { - s *stream - disallowUnknownFields bool - structTypeToDecoder map[uintptr]decoder + s *stream + structTypeToDecoder map[uintptr]decoder } type decoderMap struct { @@ -240,11 +239,12 @@ func (d *Decoder) Token() (Token, error) { if s.read() { continue } - return nil, io.EOF + goto END default: return nil, errInvalidCharacter(s.char(), "token", s.totalOffset()) } } +END: return nil, io.EOF } diff --git a/decode_bytes.go b/decode_bytes.go index e563c4a..a82fe06 100644 --- a/decode_bytes.go +++ b/decode_bytes.go @@ -147,7 +147,6 @@ func (d *bytesDecoder) decodeBinary(buf []byte, cursor int64, p unsafe.Pointer) } cursor++ } - return nil, 0, errUnexpectedEndOfJSON("[]byte", cursor) case '[': if d.sliceDecoder == nil { return nil, 0, &UnmarshalTypeError{ diff --git a/decode_compile.go b/decode_compile.go index 3e3d82f..cd07394 100644 --- a/decode_compile.go +++ b/decode_compile.go @@ -309,7 +309,7 @@ func (d *Decoder) compileStruct(typ *rtype, structName, fieldName string) (decod // recursive definition continue } - d.removeConflictFields(fieldMap, conflictedMap, stDec, uintptr(field.Offset)) + d.removeConflictFields(fieldMap, conflictedMap, stDec, field.Offset) } else if pdec, ok := dec.(*ptrDecoder); ok { contentDec := pdec.contentDecoder() if pdec.typ == typ { @@ -326,7 +326,7 @@ func (d *Decoder) compileStruct(typ *rtype, structName, fieldName string) (decod if !exists { fieldSet := &structFieldSet{ dec: newAnonymousFieldDecoder(pdec.typ, v.offset, v.dec), - offset: uintptr(field.Offset), + offset: field.Offset, isTaggedKey: v.isTaggedKey, } fieldMap[k] = fieldSet @@ -347,7 +347,7 @@ func (d *Decoder) compileStruct(typ *rtype, structName, fieldName string) (decod if v.isTaggedKey { fieldSet := &structFieldSet{ dec: newAnonymousFieldDecoder(pdec.typ, v.offset, v.dec), - offset: uintptr(field.Offset), + offset: field.Offset, isTaggedKey: v.isTaggedKey, } fieldMap[k] = fieldSet diff --git a/decode_context.go b/decode_context.go index 13ec668..23e0d3f 100644 --- a/decode_context.go +++ b/decode_context.go @@ -142,5 +142,4 @@ func skipValue(buf []byte, cursor int64) (int64, error) { } cursor++ } - return cursor, errUnexpectedEndOfJSON("value of object", cursor) } diff --git a/decode_int.go b/decode_int.go index b752101..fb0d3c7 100644 --- a/decode_int.go +++ b/decode_int.go @@ -149,7 +149,6 @@ func (d *intDecoder) decodeByte(buf []byte, cursor int64) ([]byte, int64, error) return nil, 0, d.typeError([]byte{buf[cursor]}, cursor) } } - return nil, 0, errUnexpectedEndOfJSON("number(integer)", cursor) } func (d *intDecoder) decodeStream(s *stream, p unsafe.Pointer) error { diff --git a/decode_interface.go b/decode_interface.go index 840718e..3590f0c 100644 --- a/decode_interface.go +++ b/decode_interface.go @@ -143,7 +143,6 @@ func (d *interfaceDecoder) decodeStream(s *stream, p unsafe.Pointer) error { } s.cursor++ } - return errUnexpectedEndOfJSON("string", s.totalOffset()) case 't': if err := trueBytes(s); err != nil { return err @@ -229,7 +228,6 @@ func (d *interfaceDecoder) decode(buf []byte, cursor int64, p unsafe.Pointer) (i } cursor++ } - return 0, errUnexpectedEndOfJSON("string", cursor) case 't': if cursor+3 >= int64(len(buf)) { return 0, errUnexpectedEndOfJSON("bool(true)", cursor) diff --git a/decode_map.go b/decode_map.go index e59f0f7..8a26dd6 100644 --- a/decode_map.go +++ b/decode_map.go @@ -94,7 +94,6 @@ func (d *mapDecoder) decodeStream(s *stream, p unsafe.Pointer) error { return errExpected("comma after object value", s.totalOffset()) } } - return nil } func (d *mapDecoder) decode(buf []byte, cursor int64, p unsafe.Pointer) (int64, error) { @@ -151,7 +150,6 @@ func (d *mapDecoder) decode(buf []byte, cursor int64, p unsafe.Pointer) (int64, if err != nil { return 0, err } - cursor = valueCursor mapassign(d.mapType, mapValue, unsafe.Pointer(&key), unsafe.Pointer(&value)) cursor = skipWhiteSpace(buf, valueCursor) if buf[cursor] == '}' { diff --git a/decode_ptr.go b/decode_ptr.go index 2b507bf..b7d0fd3 100644 --- a/decode_ptr.go +++ b/decode_ptr.go @@ -28,6 +28,7 @@ func (d *ptrDecoder) contentDecoder() decoder { return dec.contentDecoder() } +//nolint:golint //go:linkname unsafe_New reflect.unsafe_New func unsafe_New(*rtype) unsafe.Pointer diff --git a/decode_stream.go b/decode_stream.go index ca9d7d4..9907505 100644 --- a/decode_stream.go +++ b/decode_stream.go @@ -16,7 +16,6 @@ type stream struct { r io.Reader offset int64 cursor int64 - readPos int64 allRead bool useNumber bool disallowUnknownFields bool @@ -200,5 +199,4 @@ func (s *stream) skipValue() error { } s.cursor++ } - return errUnexpectedEndOfJSON("value of object", s.offset) } diff --git a/decode_string.go b/decode_string.go index f26a9b4..a90f911 100644 --- a/decode_string.go +++ b/decode_string.go @@ -150,6 +150,7 @@ RETRY: return nil } +//nolint:deadcode,unused func appendCoerceInvalidUTF8(b []byte, s []byte) []byte { c := [4]byte{} @@ -298,7 +299,6 @@ func (d *stringDecoder) decodeByte(buf []byte, cursor int64) ([]byte, int64, err } cursor++ } - return nil, 0, errUnexpectedEndOfJSON("string", cursor) case 'n': buflen := int64(len(buf)) if cursor+3 >= buflen { diff --git a/decode_struct.go b/decode_struct.go index 4ed88d8..0110f97 100644 --- a/decode_struct.go +++ b/decode_struct.go @@ -93,7 +93,6 @@ func (d *structDecoder) decodeStream(s *stream, p unsafe.Pointer) error { } s.cursor++ } - return nil } func (d *structDecoder) decode(buf []byte, cursor int64, p unsafe.Pointer) (int64, error) { diff --git a/decode_uint.go b/decode_uint.go index be242ee..cc67922 100644 --- a/decode_uint.go +++ b/decode_uint.go @@ -70,12 +70,12 @@ func (d *uintDecoder) decodeStreamByte(s *stream) ([]byte, error) { } num := s.buf[start:s.cursor] return num, nil - default: - return nil, d.typeError([]byte{s.char()}, s.totalOffset()) case nul: if s.read() { continue } + default: + return nil, d.typeError([]byte{s.char()}, s.totalOffset()) } break } diff --git a/encode.go b/encode.go index ad6aa78..1d13270 100644 --- a/encode.go +++ b/encode.go @@ -128,7 +128,7 @@ func (e *Encoder) SetIndent(prefix, indent string) { func marshal(v interface{}, opt EncodeOption) ([]byte, error) { ctx := takeEncodeRuntimeContext() - buf, err := encode(ctx, v, EncodeOptionHTMLEscape) + buf, err := encode(ctx, v, opt|EncodeOptionHTMLEscape) if err != nil { releaseEncodeRuntimeContext(ctx) return nil, err @@ -149,7 +149,7 @@ func marshal(v interface{}, opt EncodeOption) ([]byte, error) { func marshalNoEscape(v interface{}, opt EncodeOption) ([]byte, error) { ctx := takeEncodeRuntimeContext() - buf, err := encodeNoEscape(ctx, v, EncodeOptionHTMLEscape) + buf, err := encodeNoEscape(ctx, v, opt|EncodeOptionHTMLEscape) if err != nil { releaseEncodeRuntimeContext(ctx) return nil, err @@ -170,7 +170,7 @@ func marshalNoEscape(v interface{}, opt EncodeOption) ([]byte, error) { func marshalIndent(v interface{}, prefix, indent string, opt EncodeOption) ([]byte, error) { ctx := takeEncodeRuntimeContext() - buf, err := encodeIndent(ctx, v, prefix, indent, EncodeOptionHTMLEscape) + buf, err := encodeIndent(ctx, v, prefix, indent, opt|EncodeOptionHTMLEscape) if err != nil { releaseEncodeRuntimeContext(ctx) return nil, err @@ -320,10 +320,6 @@ func encodeBool(b []byte, v bool) []byte { return append(b, "false"...) } -func encodeBytes(dst []byte, src []byte) []byte { - return append(dst, src...) -} - func encodeNull(b []byte) []byte { return append(b, "null"...) } diff --git a/encode_compile.go b/encode_compile.go index 04e256c..a7eee7b 100644 --- a/encode_compile.go +++ b/encode_compile.go @@ -72,7 +72,7 @@ func setupOpcodeSets() error { } } } - addrRange := uintptr(max) - uintptr(min) + addrRange := max - min if addrRange == 0 { return fmt.Errorf("failed to get address range of types") } @@ -85,9 +85,7 @@ func setupOpcodeSets() error { } func init() { - if err := setupOpcodeSets(); err != nil { - // fallback to slow path - } + _ = setupOpcodeSets() } func encodeCompileToGetCodeSet(typeptr uintptr) (*opcodeSet, error) { @@ -799,6 +797,7 @@ func encodeTypeToHeaderType(ctx *encodeCompileContext, code *opcode) opType { ptrNum++ code = code.next ctx.decIndex() + continue } break } @@ -923,6 +922,7 @@ func encodeTypeToFieldType(ctx *encodeCompileContext, code *opcode) opType { ptrNum++ code = code.next ctx.decIndex() + continue } break } @@ -1182,7 +1182,7 @@ type structFieldPair struct { linked bool } -func encodeAnonymousStructFieldPairMap(typ *rtype, tags structTags, named string, valueCode *opcode) map[string][]structFieldPair { +func encodeAnonymousStructFieldPairMap(tags structTags, named string, valueCode *opcode) map[string][]structFieldPair { anonymousFields := map[string][]structFieldPair{} f := valueCode var prevAnonymousField *opcode @@ -1224,7 +1224,7 @@ func encodeAnonymousStructFieldPairMap(typ *rtype, tags structTags, named string isTaggedKey: f.isTaggedKey, }) if f.next != nil && f.nextField != f.next && f.next.op.codeType() == codeStructField { - for k, v := range encodeAnonymousStructFieldPairMap(typ, tags, named, f.next) { + for k, v := range encodeAnonymousStructFieldPairMap(tags, named, f.next) { anonymousFields[k] = append(anonymousFields[k], v...) } } @@ -1350,7 +1350,7 @@ func encodeCompileStruct(ctx *encodeCompileContext, isPtr bool) (*opcode, error) if tag.isTaggedKey { tagKey = tag.key } - for k, v := range encodeAnonymousStructFieldPairMap(typ, tags, tagKey, valueCode) { + for k, v := range encodeAnonymousStructFieldPairMap(tags, tagKey, valueCode) { anonymousFields[k] = append(anonymousFields[k], v...) } } diff --git a/encode_context.go b/encode_context.go index 692f9d7..83ceb12 100644 --- a/encode_context.go +++ b/encode_context.go @@ -28,7 +28,6 @@ func (m *mapslice) Swap(i, j int) { } type encodeMapContext struct { - iter unsafe.Pointer pos []int slice *mapslice buf []byte diff --git a/encode_opcode.go b/encode_opcode.go index 96492e2..44f38c5 100644 --- a/encode_opcode.go +++ b/encode_opcode.go @@ -124,7 +124,6 @@ func (c *opcode) beforeLastCode() *opcode { } code = nextCode } - return nil } func (c *opcode) totalLength() int { diff --git a/encode_string.go b/encode_string.go index 9dca7a5..a0c1280 100644 --- a/encode_string.go +++ b/encode_string.go @@ -347,345 +347,6 @@ var needEscape = [256]bool{ 0xff: true, } -// htmlSafeSet holds the value true if the ASCII character with the given -// array position can be safely represented inside a JSON string, embedded -// inside of HTML