223 Commits

Author SHA1 Message Date
Damien Neil
4d918167a9 internal/impl: catch varint overflow in validator
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20477

Change-Id: I6afe82e3818f8b4e9cf5eded2125317eae8be49d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217309
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-03 18:21:31 +00:00
Joe Tsai
74b1460c5b encoding: add Format helper function and method
The Format function and MarshalOptions.Format method are helper
functions for directly obtaining the formatted string for a message
without having to deal with errors or convert a []byte to string.
It is only intended for human consumption (e.g., debugging or logging).

We also add a MarshalOptions.Multiline option to specify that the output
should use some default indentation in a multiline output.

This assists in the v1 to v2 migration where:
	protoV1.CompactTextString(m) => prototext.MarshalOptions{}.Format(m)
	protoV1.MarshalTextString(m) => prototext.Format(m)

At Google, there are approximately 10x more usages of MarshalTextString than
CompactTextString, so it makes sense that the top-level Format function
does multiline expansion by default.

Fixes #850

Change-Id: I149c9e190a6d99b985d3884df675499a3313e9b3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213460
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Herbie Ong <herbie@google.com>
2020-01-30 07:50:58 +00:00
Damien Neil
1887ff702c internal/impl: better fast-path init checks for extensions
Unknown extensions are initialized.

Valid extensions with no isInit func are initialized.

Change-Id: I2975c7ef85d2b777eca467d3b1861d20de8e24fc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216960
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-30 05:43:23 +00:00
Damien Neil
6f2977906d internal/impl: fix validator bytes field length decoding
Missing a bounds check on the first byte.

Change-Id: I089fa8dcc1a14d11faca1acba758b6b811b16ac4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216957
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-30 00:26:49 +00:00
Damien Neil
c70f5d59d1 internal/impl: avoid redundant lazy extension inits
After taking the lock on a lazy extension's state, check to see if it
was initialized while we were waiting for the lock.

Change-Id: I1cbd52e9d655eec6c9142c97689ae36f219a28f2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216898
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-29 23:04:37 +00:00
Damien Neil
0ae1c9789a internal/impl: lazy extension decoding
Historically, extensions have been placed in the unknown fields section
of the unmarshaled message and decoded lazily on demand. The current
unmarshal implementation decodes extensions eagerly at unmarshal time,
permitting errors to be immediately reported and correctly detecting
unset required fields in extension values.

Add support for validated lazy extension decoding, where the extension
value is fully validated at initial unmarshal time but the fully
unmarshaled message is only created lazily.

Make this behavior conditional on the protolegacy flag for now.

Change-Id: I9d742496a4bd4dafea83fca8619cd6e8d7e65bc3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216764
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-29 21:35:31 +00:00
Damien Neil
a522d5fa0c internal/impl: fix tag decoding when field num doesn't fit in int32
Discoverd by OSS-Fuzz.

Change-Id: Ie2feefacee4ae632802fa920ac9694b525690eb2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216619
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-29 20:57:21 +00:00
Damien Neil
524c60670a runtime/protoiface: use more efficient options representation
Change the representation of option flags in protoiface from bools to a
bitfield. This brings the representation of options in protoiface in
sync with that in internal/impl.

This change has several benefits:

1. We will probably find that we need to add more option flags over time.
Converting to the more efficient representation of these flags as high
in the call stack as possible minimizes the performance implication of
the struct growing.

2. On a similar note, this avoids the need to convert from the compact
representation to the larger one when passing from internal/impl to
proto, since the {Marshal,Unmarshal}State methods take the compact form.

3. This removes unused options from protoiface. Instead of documenting
that AllowPartial is always set, we can just not include an AllowPartial
flag in the protoiface options.

4. Conversely, this provides a way to add option flags to protoiface
that we don't want to expose in the proto package.

name                             old time/op    new time/op    delta
EmptyMessage/Wire/Marshal-12       11.1ns ± 7%    10.1ns ± 1%   -9.35%  (p=0.000 n=8+8)
EmptyMessage/Wire/Unmarshal-12     7.07ns ± 0%    6.74ns ± 1%   -4.58%  (p=0.000 n=8+8)
EmptyMessage/Wire/Validate-12      4.30ns ± 1%    3.80ns ± 8%  -11.45%  (p=0.000 n=7+8)
RepeatedInt32/Wire/Marshal-12      1.17µs ± 1%    1.21µs ± 7%   +4.09%  (p=0.000 n=8+8)
RepeatedInt32/Wire/Unmarshal-12     938ns ± 0%     942ns ± 3%     ~     (p=0.178 n=7+8)
RepeatedInt32/Wire/Validate-12      521ns ± 4%     543ns ± 7%     ~     (p=0.157 n=7+8)
Required/Wire/Marshal-12           97.2ns ± 1%    95.3ns ± 1%   -1.98%  (p=0.001 n=7+7)
Required/Wire/Unmarshal-12         41.0ns ± 9%    38.6ns ± 3%   -5.73%  (p=0.048 n=8+8)
Required/Wire/Validate-12          25.4ns ±11%    21.4ns ± 3%  -15.62%  (p=0.000 n=8+7)

Change-Id: I3ac1b00ab36cfdf61316ec087a5dd20d9248e4f6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216760
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:33:31 +00:00
Damien Neil
212b05b808 internal/testprotos: make TestAllExtensions recursive
Tweak the test message to allow creating messages with extensions that
contain extensions that contain extensions, etc.

Change-Id: I41844ae699c88ab96bf0d30db3a3fbaf09616161
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216761
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:28:36 +00:00
Damien Neil
a60e709ac8 proto: fix DiscardUnknown
UnmarshalOptions.DiscardUnknown was simply not working. Oops. Fix it.
Add a test.

Change-Id: I76888eae1221d99a007f0e9cdb711d292e6856b1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216762
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:27:58 +00:00
Damien Neil
cb0bfd0f40 internal/impl: reduce redundant MessageInfo initializations in validator
name                            old time/op    new time/op    delta
EmptyMessage/Wire/Validate-12     4.58ns ± 0%    4.29ns ± 1%   -6.22%  (p=0.000 n=7+8)
RepeatedInt32/Wire/Validate-12     702ns ± 1%     518ns ± 0%  -26.12%  (p=0.001 n=7+7)
Required/Wire/Validate-12         30.6ns ± 6%    22.1ns ± 0%  -27.81%  (p=0.000 n=8+7)

Change-Id: I0d1db8583aa0bf4468bc385c213eb6adff001297
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216627
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 22:46:45 +00:00
Damien Neil
8fa11b1122 internal/impl: inline most field decoding in the validator
name                            old time/op    new time/op    delta
EmptyMessage/Wire/Validate-12     4.51ns ± 1%    4.57ns ± 0%   +1.19%  (p=0.045 n=8+8)
RepeatedInt32/Wire/Validate-12     910ns ± 0%     726ns ± 3%  -20.13%  (p=0.000 n=8+8)
Required/Wire/Validate-12         34.5ns ± 0%    29.6ns ± 5%  -13.99%  (p=0.000 n=7+8)

Change-Id: I8ac90ed3fc79dfef7f2500f13b33fd2593fc0fc1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216625
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 22:46:30 +00:00
Damien Neil
5d82883c5a internal/impl: inline small tag decoding in the validator
name                            old time/op    new time/op    delta
EmptyMessage/Wire/Validate-12     4.59ns ± 0%    4.51ns ± 1%   -1.74%  (p=0.001 n=8+8)
RepeatedInt32/Wire/Validate-12    1.28µs ± 0%    0.91µs ± 0%  -28.71%  (p=0.000 n=7+8)
Required/Wire/Validate-12         48.3ns ± 2%    34.5ns ± 0%  -28.69%  (p=0.001 n=7+7)

Change-Id: If7c431ee23d930d44af0fc26b7bd2149d3aded64
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216624
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 22:46:12 +00:00
Damien Neil
adbbc8ec47 internal/impl: inline some small varint decoding
Inline decoding of 1- and 2-byte varints in generated unmarshal
functions.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.2ns ± 2%  40.1ns ± 1%     ~     (p=0.355 n=37+37)
EmptyMessage/Wire/Unmarshal-12   7.12ns ± 1%  6.87ns ± 1%   -3.49%  (p=0.000 n=37+39)
RepeatedInt32/Wire/Unmarshal     6.46µs ± 1%  5.78µs ± 1%  -10.65%  (p=0.000 n=35+33)
RepeatedInt32/Wire/Unmarshal-12  1.05µs ± 2%  0.98µs ± 2%   -6.79%  (p=0.000 n=33+40)
Required/Wire/Unmarshal           251ns ± 1%   216ns ± 1%  -13.69%  (p=0.000 n=38+36)
Required/Wire/Unmarshal-12       42.4ns ± 1%  37.7ns ± 2%  -11.02%  (p=0.000 n=37+39)

Change-Id: Iecfc38fcae00979b89a093368821cca7f2357578
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216421
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-27 19:09:55 +00:00
Damien Neil
170b2bfca6 internal/impl: precompute required bit in validator
Required field validation populates a bitmask of observed required
fields. Store a uint64 containing the bit to set in the validationInfo
rather than the index of the bit. Provides a noticeable speed increase
in validation.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.2ns ± 1%  40.2ns ± 2%    ~     (p=0.860 n=35+37)
EmptyMessage/Wire/Unmarshal-12   7.13ns ± 5%  7.12ns ± 1%    ~     (p=0.112 n=37+37)
RepeatedInt32/Wire/Unmarshal     6.57µs ± 1%  6.46µs ± 1%  -1.56%  (p=0.000 n=39+35)
RepeatedInt32/Wire/Unmarshal-12  1.05µs ± 2%  1.05µs ± 2%    ~     (p=0.659 n=37+33)
Required/Wire/Unmarshal           258ns ± 1%   251ns ± 1%  -2.87%  (p=0.000 n=32+38)
Required/Wire/Unmarshal-12       44.3ns ± 2%  42.4ns ± 1%  -4.36%  (p=0.000 n=36+37)

Change-Id: Ib1cb74d3e348355a6a2f66aecf8fdc4b58cd84d4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216420
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-26 22:23:18 +00:00
Damien Neil
ce8f7f6353 internal/impl: inline small tag decoding
Inline varint decoding of small (1- and 2-byte) field tags in the
fast-path unmarshaler.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.6ns ± 1%  40.2ns ± 1%   -1.02%  (p=0.000 n=37+35)
EmptyMessage/Wire/Unmarshal-12   6.77ns ± 2%  7.13ns ± 5%   +5.32%  (p=0.000 n=37+37)
RepeatedInt32/Wire/Unmarshal     9.46µs ± 1%  6.57µs ± 1%  -30.56%  (p=0.000 n=38+39)
RepeatedInt32/Wire/Unmarshal-12  1.50µs ± 2%  1.05µs ± 2%  -30.00%  (p=0.000 n=39+37)
Required/Wire/Unmarshal           371ns ± 1%   258ns ± 1%  -30.44%  (p=0.000 n=38+32)
Required/Wire/Unmarshal-12       60.3ns ± 1%  44.3ns ± 2%  -26.45%  (p=0.000 n=38+36)

Change-Id: Ie80415dea8cb6b840eafa52f0572046a1910a9b1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216419
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-26 22:23:05 +00:00
Damien Neil
0bf97b7e36 internal/impl: messageset validation and isinit fixes
Recognize messagesets in the validator. Currently, this just gives
up and reports an unknown validity rather than trying to descend
into the messageset.

Plumb fast-path initialization checks through messageset decoding.

Change-Id: Ice55f28e8555764e4ce2720251830e8cf475c133
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216245
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-24 20:27:57 +00:00
Damien Neil
c600d6c086 all: do best-effort initialization check on fast path unmarshal
Add a fast check for required fields to the fast path unmarshal.
This is best-effort and will fail to detect some initialized
messages: Messages with more than 64 required fields, messages
split across multiple tags, possibly other cases.

In the cases where it works (which is most of them in practice),
this permits us to skip the IsInitialized check.

Change-Id: I6b70953a333033a5e64fb7ca37a59786cb0f75a0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215878
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 20:57:14 +00:00
Damien Neil
d30e561d9e proto: add MarshalState, UnmarshalState
Add functions to the proto package which plumb through the fast-path state.

As a sample use case: A followup CL adds an Initialized field to
protoiface.UnmarshalOutput, permitting the unmarshaller to report back
when it can confirm that a message is fully initialized. We want to
preserve that information when an unmarshal operation threads through
the proto package (such as when unmarshaling extensions).

To allow these functions to be added as methods of MarshalOptions and
UnmarshalOptions rather than top-level functions, separate the options
from the input structs.

Also update options passed to fast-path methods to set AllowPartial and
Merge to reflect the expected behavior of those methods. (Always allow
partial, never merge.)

Change-Id: I482477b0c9340793be533e75a86d0bb88708716a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215877
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 20:52:17 +00:00
Damien Neil
f0831e87e2 internal/impl: change unmarshal func return to unmarshalOptions
The fast-path unmarshal funcs return the number of bytes consumed.

Change these functions to return an unmarshalOutput struct instead, to
make it easier to add to the results. This is groundwork for allowing
the fast-path unmarshaler to indicate when the unmarshaled message is
known to be initialized.

Change-Id: Ia8c44731a88f5be969a55cd98ea26282f412c7ae
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215720
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 00:22:58 +00:00
Damien Neil
61781dd92f all: abstract fast-path marshal and unmarshal inputs and outputs
We may want to make changes to the inputs and outputs of the fast-path
functions in the future. For example, we likely want to add the ability
for the fast-path unmarshal to report back whether the unmarshaled
message is known to be initialized.

Change the signatures of these functions to take in and return struct
types which can be extended with whatever fields we want in the future.

Change-Id: Idead360785df730283a4630ea405265b72482e62
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215719
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 00:22:46 +00:00
Damien Neil
f12fb45fd6 all: add ProtoMethods method to protoreflect.Message
Promote the fast-path magic ProtoMethods method to first-class citizen
of the protoreflect.Message interface.

To avoid polluting the protoreflect package with the various types
required by this method, make the necessary protoiface types unnamed and
duplicate them in protoreflect.

Updates golang/protobuf#1022.

Change-Id: I9595bae40b3bc7536d727fb6f99b3bce8f73da87
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215718
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-21 21:05:54 +00:00
Damien Neil
2f643a9741 internal/impl: avoid type conversion on UnmarshalOptions.Resolver
Remove a trivial difference in the definition of the resolver
unmarshaler option to avoid a relatively expensive interface->interface
type conversion.

Change-Id: Iecf9a686af5d17fe3e2d9b80f886c644bf8a25df
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215657
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-21 19:16:14 +00:00
Damien Neil
6635e7d00a internal/impl: recognized required bytes fields in validation
Add a missed case in validation so we correctly validate bytes fields.
Fixes a case where we would report required bytes fields as potentially
missing.

Change-Id: I3dc4196d6995942d32a795a64214b3679d60ab6c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215000
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-15 23:51:51 +00:00
Damien Neil
2ae60936c2 internal/impl: fix unmarshal of group containing their own field number
The fast-path unmarshal was getting confused when parsing a group
containing a field with a number the same as the group's own field
number. Separate the handling of EndGroup tags.

Change-Id: I637702b42c94a26102e693ee29a55e80b37d7f28
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214737
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-14 19:43:24 +00:00
Joe Tsai
1c8015fff5 all: minor tweaks
Fix a typo in legacy_enum.go.
Rename package in ancient legacy proto so that it doesn't confuse
tooling that assume that the package and directory names match.

Change-Id: I0b896045e74b0a7f998d3e5693b853eb3aa3839c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214182
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-12 09:18:34 +00:00
Joe Tsai
55f18259ef internal/testprotos/legacy: rename and regenerate
Avoid dots and dashes in the directory to avoid issues on
build systems that cannot support them well.

Change-Id: I7ea5e6ce0b16c7158c7e53bcf5c3c1a334fe4718
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214342
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-12 08:13:18 +00:00
Damien Neil
7abc2def69 internal/impl: omit isInit func when not needed
When generating the fast-path functions for a message, group, repeated
message, or repeated group field, check to see if the field message type
requires initialization checks. If not, leave the isInit func unset.

This permits the fast-path isInitialized to skip over these fields
entirely.

Change-Id: Icb5c380077d2216c4215bb0ebc16408e905aaece
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214179
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 23:00:50 +00:00
Damien Neil
54a0a0476a internal/impl: check for required fields in missing map value
If a map value is a message with required fields, the validator should
note that it is uninitialized if a map item contains no value. In this
case, the value is an empty message which obviously does not have the
required field set.

Change-Id: I7698e60765e3c95478f293e121bba3ad7fc88e27
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213900
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 05:38:08 +00:00
Damien Neil
b0c26f1868 internal/impl: add message validator
This adds a experimental function to the internal/impl package which
validates a wire-format message against a message type. The validator
reports whether the message can be successfully unmarshaled, and whether
the result is initialized (all required fields are set). In some cases,
the validator returns ambiguous results when full validation would be
expensive.

The validator is unused outside of tests. In the future, it may be used
to permit lazy unmarshaling of some data. It is being added now for
testing; in particular, the wire fuzzer now checks the validator output
for consistency with the unmarshaler.

The validator adds a small amount of unused per-MessageType state. If
this becomes a concern, we could conditionalize it with a build tag.

Change-Id: I4216ef81d6a9ed975302eed189b02d08608858b4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212302
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-07 21:36:47 +00:00
Damien Neil
b0d217f664 proto, internal/impl: don't create fast path Size for legacy Marshalers
Implementations of the legacy Marshaler type have no way to efficiently
compute the size of the message. Rather than generating an inefficient
fast-path Size method which marshals the message and examines the
length of the result, don't generate a fast-path at all.

Drop the requirement that a fast-path MarshalAppend requires a
corresponding Size.

Avoids O(N^2) behavior when marshaling a legacy Marshaler that
recursively calls proto.Marshal.

Change-Id: I4793cf32275d08f29c8e1a1a44a193d9a5724058
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213443
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-06 22:47:37 +00:00
Joe Tsai
14ac241a96 internal/impl: return nil for nil enum or message
Ensure that EnumOf, EnumDescriptorOf, EnumTypeOf, ProtoMessageV1Of,
ProtoMessageV2Of, MessageOf, MessageDescriptorOf, and MessageTypeOf
all return nil if passed a nil interface.

This parallels the behavior of reflect.TypeOf or reflect.ValueOf,
which return nil or an invalid value rather than panicking.

Change-Id: I461f15542f16cb0922d627bca6fcad5fc27d87e2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213239
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 19:10:59 +00:00
Damien Neil
f2427c09d6 proto, internal/impl: reject invalid field numbers in map items
Change-Id: I44a44a36538f6f8b94078b43711d865edb6244f5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212257
Reviewed-by: Herbie Ong <herbie@google.com>
2019-12-21 00:16:12 +00:00
Damien Neil
2c0824b512 internal/impl: fix size for zero-length packed extensions
The size calculation for packed repeated extension fields was
considering a zero-length list as encoding to a zero-length
wire.BytesType field, rather than being omitted entirely.

Change-Id: I7d4424a21ca8afd4fa81391caede49cadb4e2505
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212297
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-20 22:08:18 +00:00
Damien Neil
7e690b5b4c internal/impl: fix map decode when value is before key
Fix a bug in handling the case where the encoding for a map item places
the value field (2) before the key field (1).

Change-Id: I2e6ad9af729a199e960e566ed7ef96bba3726990
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211804
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-18 17:42:10 +00:00
Damien Neil
3e42b667d2 internal/impl: faster map fast path
Avoid using protobuf reflection on map values in the fast path. Range
operations in particular are expensive in protoreflect, because the
closure passed to Map.Range escapes.

Iterate maps using a reflect.MapIter when available.

When operating on maps of messages where we have a *MessageInfo for the
message type, directly jump to the fast-path *MessageInfo methods rather
than passing through the proto package.

Benchmarks deltas for a google.protobuf.Struct with JSON represention:
  {"parameters":{"a":{"b":{"c":{"d":{"e":{"f":{"g":{}}}}}}}}}

Compared to previous revision:

  name                      old time/op  new time/op  delta
  NestedStruct/Size         7.22µs ± 2%  4.84µs ± 2%  -32.96%  (p=0.000 n=8+8)
  NestedStruct/Size-8       9.30µs ± 2%  5.89µs ± 2%  -36.60%  (p=0.000 n=8+8)
  NestedStruct/Marshal      77.6µs ±12%   9.8µs ± 4%  -87.33%  (p=0.000 n=8+8)
  NestedStruct/Marshal-8    91.6µs ± 2%  11.9µs ± 2%  -86.99%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal    11.5µs ± 4%   8.7µs ± 2%  -24.76%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal-8  15.4µs ± 4%  11.9µs ± 2%  -22.41%  (p=0.000 n=8+8)

Compared to github.com/golang/protobuf:

  name                      old time/op  new time/op  delta
  NestedStruct/Size         5.42µs ± 1%  4.84µs ± 2%  -10.61%  (p=0.000 n=8+8)
  NestedStruct/Size-8       6.34µs ± 2%  5.89µs ± 2%   -7.10%  (p=0.000 n=8+8)
  NestedStruct/Marshal      12.5µs ± 2%   9.8µs ± 4%  -21.41%  (p=0.000 n=7+8)
  NestedStruct/Marshal-8    14.1µs ± 3%  11.9µs ± 2%  -15.52%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal    9.66µs ± 1%  8.65µs ± 2%  -10.40%  (p=0.000 n=7+8)
  NestedStruct/Unmarshal-8  11.7µs ± 3%  11.9µs ± 2%   +1.95%  (p=0.038 n=8+8)

Change-Id: I0effe6491f30d41f31904777f74eca3ac3694db3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211737
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-17 22:14:17 +00:00
Damien Neil
b120a23bc8 internal/impl: fix reversed IsValid test
Change-Id: Iaf5291a6bf31ad3dd130fca06840cec66b896f59
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211558
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-16 21:49:33 +00:00
Joe Tsai
d65001875f internal/impl: fix aberrant message detection logic
In very old messages predating the existence of the size cache or the
proto3 unknown fields, it is possible that the generated struct lacks both
XXX_ fields and ones tagged with "protobuf". This can happen with a message
that only contains oneofs. As such, check for the "protobuf_oneof" tag as well.

Change-Id: I1981cd7dde68aece1a013356b6bc91cc5529f951
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210747
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-11 04:25:07 +00:00
Damien Neil
4151cae27a internal/impl: more checks for aberrant messages
When loading a *MessageInfo for a legacy message type, check to see if
the Go type contains at least one field which looks like a message
field. Specifically, look for at least one field with a `protobuf:` tag,
or an XXX_unrecognized field.

If a message has no recognizable fields, assume that it's something we
don't know how to interpret and treat it as an aberrant message.

Change-Id: If5c09087f1a0187271c98539d761395a2ee70a9e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210617
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-10 23:02:58 +00:00
Joe Tsai
62d204ccc3 internal/impl: fix legacy UnmarshalJSONEnum
The conditional was accidentally inverted.
This function provides dubious support for encoding/json.

Change-Id: Ib4131a229afa14d9aef1ad31fec51f4dac417a3b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210638
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-10 16:55:03 +00:00
Joe Tsai
4663ebc852 internal/genname: centralize the definitions for generated names
Both the generator and the runtime need to agree upon the names of
specialized Go struct fields. Centralize that information in an
internal genname package.

In the mean time, also change the XXX_weak field name to match
the name used internally at Google.

Change-Id: I026bf354418c363482e5902f21aa5e0cacae24b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/207080
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-09 22:57:38 +00:00
Damien Neil
ce413af0b3 internal/impl: faster oneof marshaling
Change size, marshal, and isinit operations on oneofs to look up the
currently-set oneof type in a map rather than testing for each possible
oneof field in turn.

Significantly improves oneof encoding speed for oneofs with a
substantial number of fields:

  go test ./proto -bench=./oneof.*string.*test.TestAll -benchmem -count=8 -cpu=1

  name                                        old time/op    new time/op    delta
  Encode/oneof_(string)_(*test.TestAllTypes)     911ns ± 1%     397ns ± 3%  -56.45%  (p=0.000 n=8+7)
  Decode/oneof_(string)_(*test.TestAllTypes)     899ns ± 1%     922ns ± 1%   +2.49%  (p=0.001 n=7+7)

Fixes golang/protobuf#950

Change-Id: I9393a87975ce09011d885a8af4a63a639ea8452f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210281
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 22:00:58 +00:00
Damien Neil
79571e90e2 internal/impl: improve extension fast path performance
Stash fast-path information for extensions on the ExtensionInfo. In
the usual case where an ExtensionType's underlying implementation is
an *ExtensionInfo, fetching the fast-path information becomes a type
assertion rather than a mutex-guarded map access.

Maintain a global sync.Map for the case where an ExtensionType isn't an
*ExtensionInfo.

Substantially improves performance for fast-path operations on
extensions:

Encode/MessageSet_type_id_before_message_content-12      267ns ± 1%   185ns ± 1%  -30.44%  (p=0.001 n=7+7)
Encode/basic_scalar_types_(*test.TestAllExtensions)-12  1.94µs ± 1%  0.40µs ± 1%  -79.32%  (p=0.000 n=8+7)

Change-Id: If048b521deb3665a090ea3d0a178c61691d4201e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210540
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-09 18:59:42 +00:00
Damien Neil
fe15dd4cdd all: don't allow invalid field numbers when legacy support is on
The deprecated messageset format permits extension fields with numbers
greater than the usual maximum (1<<29-1). To support this, the
internal/encoding/wire package has disabled field number validation when
legacy support is enabled.

We shouldn't skip validating all field numbers for validity just because
we support larger ones in messagesets.

This change drops range validation from the wire package (other than
checking that numbers fit in an int32) and adds it to the wire
unmarshalers instead. This gives us validation where we care
about it (when unmarshaling a wire-format message) and allows for
best-effort handling of out-of-range numbers everywhere else.

Fixes golang/protobuf#996

Change-Id: I4e11b8a8aa177dd60e89723570af074a317c2451
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210290
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 18:35:13 +00:00
Damien Neil
5366f825ad proto: consistently use non-nil, zero-length []bytes for empty bytes strings
The fast-path decoder decodes zero-length repeated bytes values as
non-nil, zero-length []bytes. Do the same in the reflection decoder.

This isn't really a correctness issue, since there's no ambiguity about what a
nil entry in a [][]byte means. Still a good idea for consistency, and
retains v1 behavior.

Change-Id: Icd2cb726d14ff1f2b9f142e65756777a359971f3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210257
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 17:33:50 +00:00
Damien Neil
9f1165c3bf all: fix reflection behavior for empty lists and maps
The protoreflect documentation states that Get on an empty repeated or
map field returns an invalid (empty, read-only) List or Map. We weren't
doing this; fix it.

The documentation also states that an invalid List or Map may not be
assigned via Set. We were permitting Set with an invalid map; fix this.

Add tests for this behavior in prototest.

Change-Id: I4678af532e192210af0bde7c96a1439a4cd26efa
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/209019
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-26 22:58:51 +00:00
Damien Neil
82886da2b9 reflect/protoreflect: add {Message,List,Map}.IsValid
Various protoreflect methods can return an "empty, read-only" message,
list, or map value. Provide a method to test if a value is one of these.

Fixes golang/protobuf#966

Change-Id: I793d8426d6e2201755983c06f024412a7e09bc4c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/209018
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-26 22:56:18 +00:00
Joe Tsai
613285cf7a internal/impl: refactor makeStructInfo
Simplify the implementation of makeStructInfo by checking the
names of the internal fields in a switch statement.
Also, add "weakFields" as the unexported name for weak fields
in preparation for actually unexporting it.

Change-Id: Ide970a39e9caa5a24bc288ba3e3a0d223a6bfcb6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/207057
Reviewed-by: Damien Neil <dneil@google.com>
2019-11-13 17:53:39 +00:00
Damien Neil
01c0e8d680 proto, internal/impl: make wire output more consistent with v1
The v1 wire marshaler sorts fields as follows:
  - All extensions, sorted by field number.
  - All non-oneof fields, sorted by field number.
  - All oneof fields, in indeterminate order.

We already make some steps toward supporting this ordering: The
fast path encoder places extensions in sorted order at the start
of the message.

This commit moves oneof fields to the end of the message, makes the
reflection-based encoder use this ordering when deterministic marshaling
is enabled, and adds a test to catch unintentional changes to the
ordering.

Users SHOULD NOT depend on stability of the marshal output. It is
subject to change over time. Without deterministic marshaling enabled,
it is subject to change over calls to Marshal.

Change-Id: I6cfd89090d790a3bb50785f32b94d2781d7d08db
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206800
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-12 20:59:03 +00:00
Damien Neil
1605775be0 internal/impl: handle some dynamic legacy messages
When creating a MessageDescriptor for a legacy message with a
Descriptor method, we call that method on the type's zero value to
get the message's DescriptorProto. Some existing dynamic message
types have a Descriptor method which panics in this case.

Catch the panic and continue as if the Descriptor method wasn't present.

Change-Id: I98d4625d6917cc1ec25737e5670a443f5d02a404
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206637
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-12 17:55:36 +00:00