Skip to content

Commit 792d359

Browse files
gh2ostapelberg
authored andcommitted
internal/encoding/tag: use proto3 defaults if proto3
proto3 legacy struct field tags are improperly parsed as proto2, resulting in fd.L1.EditionFeatures.IsFieldPresence set to true when it should have been false. This causes coderFieldInfo.isPointer to be true for said fields. When the encode process (particularly MessageInfo.sizePointerSlow) sees isPointer=true for the field, it attempts to interpret the field as a pointer, which results in a "fatal error: checkptr: misaligned pointer conversion" runtime error if the field address happens to not be pointer-aligned. This only happens if building with -race/-asan/-msan as they enable checkptr. To avoid this issue, parse from proto3 edition defaults if the proto3 key is found in the struct field tag. Change-Id: Id81ab38fe7efd437d0748a485653d27b1acc3afb Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/716360 TryBot-Bypass: Michael Stapelberg <stapelberg@google.com> Auto-Submit: Nicolas Hillegeer <aktau@google.com> Reviewed-by: Nicolas Hillegeer <aktau@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Chressie Himpel <chressie@google.com> Reviewed-by: Michael Stapelberg <stapelberg@google.com>
1 parent 4a45645 commit 792d359

2 files changed

Lines changed: 13 additions & 2 deletions

File tree

internal/encoding/tag/tag.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ var byteType = reflect.TypeOf(byte(0))
3232
func Unmarshal(tag string, goType reflect.Type, evs protoreflect.EnumValueDescriptors) protoreflect.FieldDescriptor {
3333
f := new(filedesc.Field)
3434
f.L0.ParentFile = filedesc.SurrogateProto2
35-
f.L1.EditionFeatures = f.L0.ParentFile.L1.EditionFeatures
35+
packed := false
3636
for len(tag) > 0 {
3737
i := strings.IndexByte(tag, ',')
3838
if i < 0 {
@@ -108,7 +108,7 @@ func Unmarshal(tag string, goType reflect.Type, evs protoreflect.EnumValueDescri
108108
f.L1.StringName.InitJSON(jsonName)
109109
}
110110
case s == "packed":
111-
f.L1.EditionFeatures.IsPacked = true
111+
packed = true
112112
case strings.HasPrefix(s, "def="):
113113
// The default tag is special in that everything afterwards is the
114114
// default regardless of the presence of commas.
@@ -121,6 +121,13 @@ func Unmarshal(tag string, goType reflect.Type, evs protoreflect.EnumValueDescri
121121
tag = strings.TrimPrefix(tag[i:], ",")
122122
}
123123

124+
// Update EditionFeatures after the loop and after we know whether this is
125+
// a proto2 or proto3 field.
126+
f.L1.EditionFeatures = f.L0.ParentFile.L1.EditionFeatures
127+
if packed {
128+
f.L1.EditionFeatures.IsPacked = true
129+
}
130+
124131
// The generator uses the group message name instead of the field name.
125132
// We obtain the real field name by lowercasing the group name.
126133
if f.L1.Kind == protoreflect.GroupKind {

internal/encoding/tag/tag_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ func Test(t *testing.T) {
2323
fd.L1.Cardinality = protoreflect.Repeated
2424
fd.L1.Kind = protoreflect.BytesKind
2525
fd.L1.Default = filedesc.DefaultValue(protoreflect.ValueOf([]byte("hello, \xde\xad\xbe\xef\n")), nil)
26+
fd.L1.EditionFeatures = fd.L0.ParentFile.L1.EditionFeatures
2627

2728
// Marshal test.
2829
gotTag := tag.Marshal(fd, "")
@@ -37,4 +38,7 @@ func Test(t *testing.T) {
3738
if !proto.Equal(protodesc.ToFieldDescriptorProto(gotFD), protodesc.ToFieldDescriptorProto(wantFD)) {
3839
t.Errorf("Umarshal() mismatch:\ngot %v\nwant %v", gotFD, wantFD)
3940
}
41+
if gotFD.(*filedesc.Field).L1.EditionFeatures != wantFD.L1.EditionFeatures {
42+
t.Errorf("Unmarshal() did not set proto3 edition features")
43+
}
4044
}

0 commit comments

Comments
 (0)