From 048cb4ceee652e358d84fbca260fc93d7a0dfbe3 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 10 May 2021 09:59:07 -0700 Subject: [PATCH 01/40] crypto/x509: remove duplicate import Change-Id: I86742ae7aa4ff49a38f8e3bc1d64fb223feae73e Reviewed-on: https://go-review.googlesource.com/c/go/+/318409 Trust: Roland Shoemaker Trust: Katie Hockman Run-TryBot: Roland Shoemaker TryBot-Result: Go Bot Reviewed-by: Katie Hockman --- src/crypto/x509/parser.go | 115 +++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 58 deletions(-) diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index 578227ab8e..3d51ddd7f5 100644 --- a/src/crypto/x509/parser.go +++ b/src/crypto/x509/parser.go @@ -24,7 +24,6 @@ import ( "unicode/utf8" "golang.org/x/crypto/cryptobyte" - cbasn1 "golang.org/x/crypto/cryptobyte/asn1" cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" ) @@ -55,23 +54,23 @@ func isPrintable(b byte) bool { // UTF8String, BMPString, and IA5String. This is mostly copied from the // respective encoding/asn1.parse... methods, rather than just increasing // the API surface of that package. -func parseASN1String(tag cbasn1.Tag, value []byte) (string, error) { +func parseASN1String(tag cryptobyte_asn1.Tag, value []byte) (string, error) { switch tag { - case cbasn1.T61String: + case cryptobyte_asn1.T61String: return string(value), nil - case cbasn1.PrintableString: + case cryptobyte_asn1.PrintableString: for _, b := range value { if !isPrintable(b) { return "", errors.New("invalid PrintableString") } } return string(value), nil - case cbasn1.UTF8String: + case cryptobyte_asn1.UTF8String: if !utf8.Valid(value) { return "", errors.New("invalid UTF-8 string") } return string(value), nil - case cbasn1.Tag(asn1.TagBMPString): + case cryptobyte_asn1.Tag(asn1.TagBMPString): if len(value)%2 != 0 { return "", errors.New("invalid BMPString") } @@ -88,7 +87,7 @@ func parseASN1String(tag cbasn1.Tag, value []byte) (string, error) { } return string(utf16.Decode(s)), nil - case cbasn1.IA5String: + case cryptobyte_asn1.IA5String: s := string(value) if isIA5String(s) != nil { return "", errors.New("invalid IA5String") @@ -101,7 +100,7 @@ func parseASN1String(tag cbasn1.Tag, value []byte) (string, error) { // parseName parses a DER encoded Name as defined in RFC 5280. We may // want to export this function in the future for use in crypto/tls. func parseName(raw cryptobyte.String) (*pkix.RDNSequence, error) { - if !raw.ReadASN1(&raw, cbasn1.SEQUENCE) { + if !raw.ReadASN1(&raw, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: invalid RDNSequence") } @@ -109,12 +108,12 @@ func parseName(raw cryptobyte.String) (*pkix.RDNSequence, error) { for !raw.Empty() { var rdnSet pkix.RelativeDistinguishedNameSET var set cryptobyte.String - if !raw.ReadASN1(&set, cbasn1.SET) { + if !raw.ReadASN1(&set, cryptobyte_asn1.SET) { return nil, errors.New("x509: invalid RDNSequence") } for !set.Empty() { var atav cryptobyte.String - if !set.ReadASN1(&atav, cbasn1.SEQUENCE) { + if !set.ReadASN1(&atav, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: invalid RDNSequence: invalid attribute") } var attr pkix.AttributeTypeAndValue @@ -122,7 +121,7 @@ func parseName(raw cryptobyte.String) (*pkix.RDNSequence, error) { return nil, errors.New("x509: invalid RDNSequence: invalid attribute type") } var rawValue cryptobyte.String - var valueTag cbasn1.Tag + var valueTag cryptobyte_asn1.Tag if !atav.ReadAnyASN1(&rawValue, &valueTag) { return nil, errors.New("x509: invalid RDNSequence: invalid attribute value") } @@ -149,7 +148,7 @@ func parseAI(der cryptobyte.String) (pkix.AlgorithmIdentifier, error) { return ai, nil } var params cryptobyte.String - var tag cbasn1.Tag + var tag cryptobyte_asn1.Tag if !der.ReadAnyASN1Element(¶ms, &tag) { return ai, errors.New("x509: malformed parameters") } @@ -162,11 +161,11 @@ func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) { extract := func() (time.Time, error) { var t time.Time switch { - case der.PeekASN1Tag(cbasn1.UTCTime): + case der.PeekASN1Tag(cryptobyte_asn1.UTCTime): // TODO(rolandshoemaker): once #45411 is fixed, the following code // should be replaced with a call to der.ReadASN1UTCTime. var utc cryptobyte.String - if !der.ReadASN1(&utc, cbasn1.UTCTime) { + if !der.ReadASN1(&utc, cryptobyte_asn1.UTCTime) { return t, errors.New("x509: malformed UTCTime") } s := string(utc) @@ -190,7 +189,7 @@ func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) { // UTCTime only encodes times prior to 2050. See https://tools.ietf.org/html/rfc5280#section-4.1.2.5.1 t = t.AddDate(-100, 0, 0) } - case der.PeekASN1Tag(cbasn1.GeneralizedTime): + case der.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime): if !der.ReadASN1GeneralizedTime(&t) { return t, errors.New("x509: malformed GeneralizedTime") } @@ -217,13 +216,13 @@ func parseExtension(der cryptobyte.String) (pkix.Extension, error) { if !der.ReadASN1ObjectIdentifier(&ext.Id) { return ext, errors.New("x509: malformed extention OID field") } - if der.PeekASN1Tag(cbasn1.BOOLEAN) { + if der.PeekASN1Tag(cryptobyte_asn1.BOOLEAN) { if !der.ReadASN1Boolean(&ext.Critical) { return ext, errors.New("x509: malformed extention critical field") } } var val cryptobyte.String - if !der.ReadASN1(&val, cbasn1.OCTET_STRING) { + if !der.ReadASN1(&val, cryptobyte_asn1.OCTET_STRING) { return ext, errors.New("x509: malformed extention value field") } ext.Value = val @@ -241,7 +240,7 @@ func parsePublicKey(algo PublicKeyAlgorithm, keyData *publicKeyInfo) (interface{ } p := &pkcs1PublicKey{N: new(big.Int)} - if !der.ReadASN1(&der, cbasn1.SEQUENCE) { + if !der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: invalid RSA public key") } if !der.ReadASN1Integer(p.N) { @@ -307,7 +306,7 @@ func parsePublicKey(algo PublicKeyAlgorithm, keyData *publicKeyInfo) (interface{ }, } paramsDer := cryptobyte.String(keyData.Algorithm.Parameters.FullBytes) - if !paramsDer.ReadASN1(¶msDer, cbasn1.SEQUENCE) || + if !paramsDer.ReadASN1(¶msDer, cryptobyte_asn1.SEQUENCE) || !paramsDer.ReadASN1Integer(pub.Parameters.P) || !paramsDer.ReadASN1Integer(pub.Parameters.Q) || !paramsDer.ReadASN1Integer(pub.Parameters.G) { @@ -340,16 +339,16 @@ func parseKeyUsageExtension(der cryptobyte.String) (KeyUsage, error) { func parseBasicConstraintsExtension(der cryptobyte.String) (bool, int, error) { var isCA bool - if !der.ReadASN1(&der, cbasn1.SEQUENCE) { + if !der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) { return false, 0, errors.New("x509: invalid basic constraints a") } - if der.PeekASN1Tag(cbasn1.BOOLEAN) { + if der.PeekASN1Tag(cryptobyte_asn1.BOOLEAN) { if !der.ReadASN1Boolean(&isCA) { return false, 0, errors.New("x509: invalid basic constraints b") } } maxPathLen := -1 - if !der.Empty() && der.PeekASN1Tag(cbasn1.INTEGER) { + if !der.Empty() && der.PeekASN1Tag(cryptobyte_asn1.INTEGER) { if !der.ReadASN1Integer(&maxPathLen) { return false, 0, errors.New("x509: invalid basic constraints c") } @@ -360,12 +359,12 @@ func parseBasicConstraintsExtension(der cryptobyte.String) (bool, int, error) { } func forEachSAN(der cryptobyte.String, callback func(tag int, data []byte) error) error { - if !der.ReadASN1(&der, cbasn1.SEQUENCE) { + if !der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid subject alternative names") } for !der.Empty() { var san cryptobyte.String - var tag cbasn1.Tag + var tag cryptobyte_asn1.Tag if !der.ReadAnyASN1(&san, &tag) { return errors.New("x509: invalid subject alternative name") } @@ -425,7 +424,7 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string func parseExtKeyUsageExtension(der cryptobyte.String) ([]ExtKeyUsage, []asn1.ObjectIdentifier, error) { var extKeyUsages []ExtKeyUsage var unknownUsages []asn1.ObjectIdentifier - if !der.ReadASN1(&der, cbasn1.SEQUENCE) { + if !der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) { return nil, nil, errors.New("x509: invalid extended key usages") } for !der.Empty() { @@ -444,12 +443,12 @@ func parseExtKeyUsageExtension(der cryptobyte.String) ([]ExtKeyUsage, []asn1.Obj func parseCertificatePoliciesExtension(der cryptobyte.String) ([]asn1.ObjectIdentifier, error) { var oids []asn1.ObjectIdentifier - if !der.ReadASN1(&der, cbasn1.SEQUENCE) { + if !der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: invalid certificate policies") } for !der.Empty() { var cp cryptobyte.String - if !der.ReadASN1(&cp, cbasn1.SEQUENCE) { + if !der.ReadASN1(&cp, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: invalid certificate policies") } var oid asn1.ObjectIdentifier @@ -697,31 +696,31 @@ func processExtensions(out *Certificate) error { // fullName [0] GeneralNames, // nameRelativeToCRLIssuer [1] RelativeDistinguishedName } val := cryptobyte.String(e.Value) - if !val.ReadASN1(&val, cbasn1.SEQUENCE) { + if !val.ReadASN1(&val, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid CRL distribution points") } for !val.Empty() { var dpDER cryptobyte.String - if !val.ReadASN1(&dpDER, cbasn1.SEQUENCE) { + if !val.ReadASN1(&dpDER, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid CRL distribution point") } var dpNameDER cryptobyte.String var dpNamePresent bool - if !dpDER.ReadOptionalASN1(&dpNameDER, &dpNamePresent, cbasn1.Tag(0).Constructed().ContextSpecific()) { + if !dpDER.ReadOptionalASN1(&dpNameDER, &dpNamePresent, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) { return errors.New("x509: invalid CRL distribution point") } if !dpNamePresent { continue } - if !dpNameDER.ReadASN1(&dpNameDER, cbasn1.Tag(0).Constructed().ContextSpecific()) { + if !dpNameDER.ReadASN1(&dpNameDER, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) { return errors.New("x509: invalid CRL distribution point") } for !dpNameDER.Empty() { - if !dpNameDER.PeekASN1Tag(cbasn1.Tag(6).ContextSpecific()) { + if !dpNameDER.PeekASN1Tag(cryptobyte_asn1.Tag(6).ContextSpecific()) { break } var uri cryptobyte.String - if !dpNameDER.ReadASN1(&uri, cbasn1.Tag(6).ContextSpecific()) { + if !dpNameDER.ReadASN1(&uri, cryptobyte_asn1.Tag(6).ContextSpecific()) { return errors.New("x509: invalid CRL distribution point") } out.CRLDistributionPoints = append(out.CRLDistributionPoints, string(uri)) @@ -732,10 +731,10 @@ func processExtensions(out *Certificate) error { // RFC 5280, 4.2.1.1 val := cryptobyte.String(e.Value) var akid cryptobyte.String - if !val.ReadASN1(&akid, cbasn1.SEQUENCE) { + if !val.ReadASN1(&akid, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid authority key identifier") } - if !akid.ReadASN1(&akid, cbasn1.Tag(0).ContextSpecific()) { + if !akid.ReadASN1(&akid, cryptobyte_asn1.Tag(0).ContextSpecific()) { return errors.New("x509: invalid authority key identifier") } out.AuthorityKeyId = akid @@ -748,7 +747,7 @@ func processExtensions(out *Certificate) error { // RFC 5280, 4.2.1.2 val := cryptobyte.String(e.Value) var skid cryptobyte.String - if !val.ReadASN1(&skid, cbasn1.OCTET_STRING) { + if !val.ReadASN1(&skid, cryptobyte_asn1.OCTET_STRING) { return errors.New("x509: invalid subject key identifier") } out.SubjectKeyId = skid @@ -764,22 +763,22 @@ func processExtensions(out *Certificate) error { } else if e.Id.Equal(oidExtensionAuthorityInfoAccess) { // RFC 5280 4.2.2.1: Authority Information Access val := cryptobyte.String(e.Value) - if !val.ReadASN1(&val, cbasn1.SEQUENCE) { + if !val.ReadASN1(&val, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid authority info access") } for !val.Empty() { var aiaDER cryptobyte.String - if !val.ReadASN1(&aiaDER, cbasn1.SEQUENCE) { + if !val.ReadASN1(&aiaDER, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid authority info access") } var method asn1.ObjectIdentifier if !aiaDER.ReadASN1ObjectIdentifier(&method) { return errors.New("x509: invalid authority info access") } - if !aiaDER.PeekASN1Tag(cbasn1.Tag(6).ContextSpecific()) { + if !aiaDER.PeekASN1Tag(cryptobyte_asn1.Tag(6).ContextSpecific()) { continue } - if !aiaDER.ReadASN1(&aiaDER, cbasn1.Tag(6).ContextSpecific()) { + if !aiaDER.ReadASN1(&aiaDER, cryptobyte_asn1.Tag(6).ContextSpecific()) { return errors.New("x509: invalid authority info access") } switch { @@ -809,26 +808,26 @@ func parseCertificate(der []byte) (*Certificate, error) { // we read the SEQUENCE including length and tag bytes so that // we can populate Certificate.Raw, before unwrapping the // SEQUENCE so it can be operated on - if !input.ReadASN1Element(&input, cbasn1.SEQUENCE) { + if !input.ReadASN1Element(&input, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed certificate") } cert.Raw = input - if !input.ReadASN1(&input, cbasn1.SEQUENCE) { + if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed certificate") } var tbs cryptobyte.String // do the same trick again as above to extract the raw // bytes for Certificate.RawTBSCertificate - if !input.ReadASN1Element(&tbs, cbasn1.SEQUENCE) { + if !input.ReadASN1Element(&tbs, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed tbs certificate") } cert.RawTBSCertificate = tbs - if !tbs.ReadASN1(&tbs, cbasn1.SEQUENCE) { + if !tbs.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed tbs certificate") } - if !tbs.ReadOptionalASN1Integer(&cert.Version, cbasn1.Tag(0).Constructed().ContextSpecific(), 0) { + if !tbs.ReadOptionalASN1Integer(&cert.Version, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific(), 0) { return nil, errors.New("x509: malformed version") } if cert.Version < 0 { @@ -853,14 +852,14 @@ func parseCertificate(der []byte) (*Certificate, error) { cert.SerialNumber = serial var sigAISeq cryptobyte.String - if !tbs.ReadASN1(&sigAISeq, cbasn1.SEQUENCE) { + if !tbs.ReadASN1(&sigAISeq, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed signature algorithm identifier") } // Before parsing the inner algorithm identifier, extract // the outer algorithm identifier and make sure that they // match. var outerSigAISeq cryptobyte.String - if !input.ReadASN1(&outerSigAISeq, cbasn1.SEQUENCE) { + if !input.ReadASN1(&outerSigAISeq, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed algorithm identifier") } if !bytes.Equal(outerSigAISeq, sigAISeq) { @@ -873,7 +872,7 @@ func parseCertificate(der []byte) (*Certificate, error) { cert.SignatureAlgorithm = getSignatureAlgorithmFromAI(sigAI) var issuerSeq cryptobyte.String - if !tbs.ReadASN1Element(&issuerSeq, cbasn1.SEQUENCE) { + if !tbs.ReadASN1Element(&issuerSeq, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed issuer") } cert.RawIssuer = issuerSeq @@ -884,7 +883,7 @@ func parseCertificate(der []byte) (*Certificate, error) { cert.Issuer.FillFromRDNSequence(issuerRDNs) var validity cryptobyte.String - if !tbs.ReadASN1(&validity, cbasn1.SEQUENCE) { + if !tbs.ReadASN1(&validity, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed validity") } cert.NotBefore, cert.NotAfter, err = parseValidity(validity) @@ -893,7 +892,7 @@ func parseCertificate(der []byte) (*Certificate, error) { } var subjectSeq cryptobyte.String - if !tbs.ReadASN1Element(&subjectSeq, cbasn1.SEQUENCE) { + if !tbs.ReadASN1Element(&subjectSeq, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed issuer") } cert.RawSubject = subjectSeq @@ -904,15 +903,15 @@ func parseCertificate(der []byte) (*Certificate, error) { cert.Subject.FillFromRDNSequence(subjectRDNs) var spki cryptobyte.String - if !tbs.ReadASN1Element(&spki, cbasn1.SEQUENCE) { + if !tbs.ReadASN1Element(&spki, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed spki") } cert.RawSubjectPublicKeyInfo = spki - if !spki.ReadASN1(&spki, cbasn1.SEQUENCE) { + if !spki.ReadASN1(&spki, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed spki") } var pkAISeq cryptobyte.String - if !spki.ReadASN1(&pkAISeq, cbasn1.SEQUENCE) { + if !spki.ReadASN1(&pkAISeq, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed public key algorithm identifier") } pkAI, err := parseAI(pkAISeq) @@ -933,25 +932,25 @@ func parseCertificate(der []byte) (*Certificate, error) { } if cert.Version > 1 { - if !tbs.SkipOptionalASN1(cbasn1.Tag(1).Constructed().ContextSpecific()) { + if !tbs.SkipOptionalASN1(cryptobyte_asn1.Tag(1).Constructed().ContextSpecific()) { return nil, errors.New("x509: malformed issuerUniqueID") } - if !tbs.SkipOptionalASN1(cbasn1.Tag(2).Constructed().ContextSpecific()) { + if !tbs.SkipOptionalASN1(cryptobyte_asn1.Tag(2).Constructed().ContextSpecific()) { return nil, errors.New("x509: malformed subjectUniqueID") } if cert.Version == 3 { var extensions cryptobyte.String var present bool - if !tbs.ReadOptionalASN1(&extensions, &present, cbasn1.Tag(3).Constructed().ContextSpecific()) { + if !tbs.ReadOptionalASN1(&extensions, &present, cryptobyte_asn1.Tag(3).Constructed().ContextSpecific()) { return nil, errors.New("x509: malformed extensions") } if present { - if !extensions.ReadASN1(&extensions, cbasn1.SEQUENCE) { + if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed extensions") } for !extensions.Empty() { var extension cryptobyte.String - if !extensions.ReadASN1(&extension, cbasn1.SEQUENCE) { + if !extensions.ReadASN1(&extension, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed extension") } ext, err := parseExtension(extension) From 6d2ef2ef2a3ed375b5c782e6c8b0f8a59c3d3c8c Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 18 May 2021 12:58:02 -0400 Subject: [PATCH 02/40] cmd/compile: don't emit inltree for closure within body of inlined func When inlining functions with closures, ensure that we don't mark the body of the closure with a src.Pos marker that reflects the inline, since this will result in the generation of an inltree table for the closure itself (as opposed to the routine that the func-with-closure was inlined into). Fixes #46234. Change-Id: I348296de6504fc4745d99adab436640f50be299a Reviewed-on: https://go-review.googlesource.com/c/go/+/320913 Reviewed-by: Cherry Mui Reviewed-by: Matthew Dempsky Run-TryBot: Cherry Mui TryBot-Result: Go Bot Trust: Than McIntosh --- src/cmd/compile/internal/inline/inl.go | 16 +++- test/closure3.dir/main.go | 8 +- test/fixedbugs/issue46234.go | 103 +++++++++++++++++++++++++ test/inline.go | 4 +- 4 files changed, 124 insertions(+), 7 deletions(-) create mode 100644 test/fixedbugs/issue46234.go diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index a6829e9835..d6b4ced4e1 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -1124,6 +1124,10 @@ type inlsubst struct { newclofn *ir.Func fn *ir.Func // For debug -- the func that is being inlined + + // If true, then don't update source positions during substitution + // (retain old source positions). + noPosUpdate bool } // list inlines a list of nodes. @@ -1219,7 +1223,14 @@ func (subst *inlsubst) clovar(n *ir.Name) *ir.Name { // closure node. func (subst *inlsubst) closure(n *ir.ClosureExpr) ir.Node { m := ir.Copy(n) - m.SetPos(subst.updatedPos(m.Pos())) + + // Prior to the subst edit, set a flag in the inlsubst to + // indicated that we don't want to update the source positions in + // the new closure. If we do this, it will appear that the closure + // itself has things inlined into it, which is not the case. See + // issue #46234 for more details. + defer func(prev bool) { subst.noPosUpdate = prev }(subst.noPosUpdate) + subst.noPosUpdate = true ir.EditChildren(m, subst.edit) //fmt.Printf("Inlining func %v with closure into %v\n", subst.fn, ir.FuncName(ir.CurFunc)) @@ -1445,6 +1456,9 @@ func (subst *inlsubst) node(n ir.Node) ir.Node { } func (subst *inlsubst) updatedPos(xpos src.XPos) src.XPos { + if subst.noPosUpdate { + return xpos + } pos := base.Ctxt.PosTable.Pos(xpos) oldbase := pos.Base() // can be nil newbase := subst.bases[oldbase] diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go index 2fc33753ed..662a2e967b 100644 --- a/test/closure3.dir/main.go +++ b/test/closure3.dir/main.go @@ -94,10 +94,10 @@ func main() { return x + 2 } y, sink = func() (func(int) int, int) { // ERROR "can inline main.func12" - return func(x int) int { // ERROR "can inline main.func12" + return func(x int) int { // ERROR "func literal does not escape" "can inline main.func12" return x + 1 }, 42 - }() // ERROR "func literal does not escape" "inlining call to main.func12" + }() // ERROR "inlining call to main.func12" if y(40) != 41 { ppanic("y(40) != 41") } @@ -109,10 +109,10 @@ func main() { return x + 2 } y, sink = func() (func(int) int, int) { // ERROR "can inline main.func13.2" - return func(x int) int { // ERROR "can inline main.func13.2" + return func(x int) int { // ERROR "func literal does not escape" "can inline main.func13.2" return x + 1 }, 42 - }() // ERROR "inlining call to main.func13.2" "func literal does not escape" + }() // ERROR "inlining call to main.func13.2" if y(40) != 41 { ppanic("y(40) != 41") } diff --git a/test/fixedbugs/issue46234.go b/test/fixedbugs/issue46234.go new file mode 100644 index 0000000000..c669cc01a6 --- /dev/null +++ b/test/fixedbugs/issue46234.go @@ -0,0 +1,103 @@ +// buildrun -t 30 + +// +build !js + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Ensure that runtime traceback does not infinite loop for +// the testcase below. + +package main + +import ( + "bytes" + "io/ioutil" + "log" + "os" + "os/exec" + "path/filepath" +) + +const prog = ` + +package main + +import "context" + +var gpi *int + +type nAO struct { + eE bool +} + +type NAO func(*nAO) + +func WEA() NAO { + return func(o *nAO) { o.eE = true } +} + +type R struct { + cM *CM +} + +type CM int + +type A string + +func (m *CM) NewA(ctx context.Context, cN string, nn *nAO, opts ...NAO) (*A, error) { + for _, o := range opts { + o(nn) + } + s := A("foo") + return &s, nil +} + +func (r *R) CA(ctx context.Context, cN string, nn *nAO) (*int, error) { + cA, err := r.cM.NewA(ctx, cN, nn, WEA(), WEA()) + if err == nil { + return nil, err + } + println(cA) + x := int(42) + return &x, nil +} + +func main() { + c := CM(1) + r := R{cM: &c} + var ctx context.Context + nnr := nAO{} + pi, err := r.CA(ctx, "foo", nil) + if err != nil { + panic("bad") + } + println(nnr.eE) + gpi = pi +} +` + +func main() { + dir, err := ioutil.TempDir("", "46234") + if err != nil { + log.Fatal(err) + } + defer os.RemoveAll(dir) + + file := filepath.Join(dir, "main.go") + if err := ioutil.WriteFile(file, []byte(prog), 0655); err != nil { + log.Fatalf("Write error %v", err) + } + + cmd := exec.Command("go", "run", file) + output, err := cmd.CombinedOutput() + if err == nil { + log.Fatalf("Passed, expected an error") + } + + want := []byte("segmentation violation") + if !bytes.Contains(output, want) { + log.Fatalf("Unmatched error message %q:\nin\n%s\nError: %v", want, output, err) + } +} diff --git a/test/inline.go b/test/inline.go index bc23768d01..472a941dca 100644 --- a/test/inline.go +++ b/test/inline.go @@ -92,9 +92,9 @@ func o() int { foo := func() int { return 1 } // ERROR "can inline o.func1" "func literal does not escape" func(x int) { // ERROR "can inline o.func2" if x > 10 { - foo = func() int { return 2 } // ERROR "can inline o.func2" + foo = func() int { return 2 } // ERROR "func literal does not escape" "can inline o.func2" } - }(11) // ERROR "func literal does not escape" "inlining call to o.func2" + }(11) // ERROR "inlining call to o.func2" return foo() } From eeadce2d871358306f2a95b0cfbe809ea017932a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 18 May 2021 11:16:38 -0400 Subject: [PATCH 03/40] go/build/constraint: fix parsing of "// +build" (with no args) "// +build" by itself was like "// +build !" - unsatisfiable. Make it so again (right now it panics). Fixes #44487. Change-Id: Iacbc1398af6f988ef011f9f438e792eb62f8f434 Reviewed-on: https://go-review.googlesource.com/c/go/+/320829 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- src/go/build/constraint/expr.go | 3 +++ src/go/build/constraint/expr_test.go | 32 ++++++++++++++++------------ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/go/build/constraint/expr.go b/src/go/build/constraint/expr.go index 1ef707ceac..957eb9b527 100644 --- a/src/go/build/constraint/expr.go +++ b/src/go/build/constraint/expr.go @@ -426,6 +426,9 @@ func parsePlusBuildExpr(text string) Expr { x = or(x, y) } } + if x == nil { + x = tag("ignore") + } return x } diff --git a/src/go/build/constraint/expr_test.go b/src/go/build/constraint/expr_test.go index 4979f8b5f2..15d189012e 100644 --- a/src/go/build/constraint/expr_test.go +++ b/src/go/build/constraint/expr_test.go @@ -216,6 +216,7 @@ var parsePlusBuildExprTests = []struct { {"!!x", tag("ignore")}, {"!x", not(tag("x"))}, {"!", tag("ignore")}, + {"", tag("ignore")}, } func TestParsePlusBuildExpr(t *testing.T) { @@ -232,19 +233,22 @@ func TestParsePlusBuildExpr(t *testing.T) { var constraintTests = []struct { in string x Expr - err error + err string }{ - {"//+build x y", or(tag("x"), tag("y")), nil}, - {"// +build x y \n", or(tag("x"), tag("y")), nil}, - {"// +build x y \n ", nil, errNotConstraint}, - {"// +build x y \nmore", nil, errNotConstraint}, - {" //+build x y", nil, errNotConstraint}, + {"//+build !", tag("ignore"), ""}, + {"//+build", tag("ignore"), ""}, + {"//+build x y", or(tag("x"), tag("y")), ""}, + {"// +build x y \n", or(tag("x"), tag("y")), ""}, + {"// +build x y \n ", nil, "not a build constraint"}, + {"// +build x y \nmore", nil, "not a build constraint"}, + {" //+build x y", nil, "not a build constraint"}, - {"//go:build x && y", and(tag("x"), tag("y")), nil}, - {"//go:build x && y\n", and(tag("x"), tag("y")), nil}, - {"//go:build x && y\n ", nil, errNotConstraint}, - {"//go:build x && y\nmore", nil, errNotConstraint}, - {" //go:build x && y", nil, errNotConstraint}, + {"//go:build x && y", and(tag("x"), tag("y")), ""}, + {"//go:build x && y\n", and(tag("x"), tag("y")), ""}, + {"//go:build x && y\n ", nil, "not a build constraint"}, + {"//go:build x && y\nmore", nil, "not a build constraint"}, + {" //go:build x && y", nil, "not a build constraint"}, + {"//go:build\n", nil, "unexpected end of expression"}, } func TestParse(t *testing.T) { @@ -252,14 +256,14 @@ func TestParse(t *testing.T) { t.Run(fmt.Sprint(i), func(t *testing.T) { x, err := Parse(tt.in) if err != nil { - if tt.err == nil { + if tt.err == "" { t.Errorf("Constraint(%q): unexpected error: %v", tt.in, err) - } else if tt.err != err { + } else if !strings.Contains(err.Error(), tt.err) { t.Errorf("Constraint(%q): error %v, want %v", tt.in, err, tt.err) } return } - if tt.err != nil { + if tt.err != "" { t.Errorf("Constraint(%q) = %v, want error %v", tt.in, x, tt.err) return } From 15a374d5c1336e9cc2f8b615477d5917e9477440 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Tue, 18 May 2021 18:25:44 -0400 Subject: [PATCH 04/40] test: check portable error message on issue46234.go issue46234.go expects an error output "segmentation violation", which is UNIX-specific. Check for "nil pointer dereference" instead, which is emitted by the Go runtime and should work on all platforms. Should fix Windows builders. Change-Id: I3f5a66a687d43cae5eaf6a9e942b877e5a248900 Reviewed-on: https://go-review.googlesource.com/c/go/+/321072 Trust: Cherry Mui Run-TryBot: Cherry Mui TryBot-Result: Go Bot Reviewed-by: Than McIntosh --- test/fixedbugs/issue46234.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixedbugs/issue46234.go b/test/fixedbugs/issue46234.go index c669cc01a6..8e7eb8bf8d 100644 --- a/test/fixedbugs/issue46234.go +++ b/test/fixedbugs/issue46234.go @@ -96,7 +96,7 @@ func main() { log.Fatalf("Passed, expected an error") } - want := []byte("segmentation violation") + want := []byte("nil pointer dereference") if !bytes.Contains(output, want) { log.Fatalf("Unmatched error message %q:\nin\n%s\nError: %v", want, output, err) } From 658b5e66ecbc41a49e6fb5aa63c5d9c804cf305f Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Tue, 18 May 2021 12:23:56 -0400 Subject: [PATCH 05/40] net: return nil UDPAddr from ReadFromUDP In cases where the socket operation has no underlying address, golang.org/cl/291509 unintentionally changed ReadFromUDP from return a nil *UDPAddr to a non-nil (but zero value) *UDPAddr. This may break callers that assume "no address" is always addr == nil, so change it back to remain nil. Fixes #46238 Change-Id: I8531e8fa16b853ed7560088eabda0b9e3e53f5be Reviewed-on: https://go-review.googlesource.com/c/go/+/320909 Trust: Michael Pratt Trust: Josh Bleecher Snyder Reviewed-by: Filippo Valsorda Reviewed-by: Josh Bleecher Snyder --- src/net/udpsock_posix.go | 3 +++ src/net/udpsock_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/net/udpsock_posix.go b/src/net/udpsock_posix.go index fcfb9c004c..96fb373ce7 100644 --- a/src/net/udpsock_posix.go +++ b/src/net/udpsock_posix.go @@ -50,6 +50,9 @@ func (c *UDPConn) readFrom(b []byte, addr *UDPAddr) (int, *UDPAddr, error) { *addr = UDPAddr{IP: sa.Addr[0:], Port: sa.Port} case *syscall.SockaddrInet6: *addr = UDPAddr{IP: sa.Addr[0:], Port: sa.Port, Zone: zoneCache.name(int(sa.ZoneId))} + default: + // No sockaddr, so don't return UDPAddr. + addr = nil } return n, addr, err } diff --git a/src/net/udpsock_test.go b/src/net/udpsock_test.go index b4000b5664..0e8c3511c3 100644 --- a/src/net/udpsock_test.go +++ b/src/net/udpsock_test.go @@ -8,7 +8,9 @@ package net import ( + "errors" "internal/testenv" + "os" "reflect" "runtime" "testing" @@ -446,6 +448,33 @@ func TestUDPReadSizeError(t *testing.T) { } } +// TestUDPReadTimeout verifies that ReadFromUDP with timeout returns an error +// without data or an address. +func TestUDPReadTimeout(t *testing.T) { + la, err := ResolveUDPAddr("udp4", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + c, err := ListenUDP("udp4", la) + if err != nil { + t.Fatal(err) + } + defer c.Close() + + c.SetDeadline(time.Now()) + b := make([]byte, 1) + n, addr, err := c.ReadFromUDP(b) + if !errors.Is(err, os.ErrDeadlineExceeded) { + t.Errorf("ReadFromUDP got err %v want os.ErrDeadlineExceeded", err) + } + if n != 0 { + t.Errorf("ReadFromUDP got n %d want 0", n) + } + if addr != nil { + t.Errorf("ReadFromUDP got addr %+#v want nil", addr) + } +} + func BenchmarkWriteToReadFromUDP(b *testing.B) { conn, err := ListenUDP("udp4", &UDPAddr{IP: IPv4(127, 0, 0, 1)}) if err != nil { From 6c1c055d1ea417d050503efe92c1eead0da68cef Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Tue, 18 May 2021 23:29:14 -0400 Subject: [PATCH 06/40] cmd/internal/moddeps: use filepath.SkipDir only on directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a filepath.WalkFunc returns filepath.SkipDir when invoked on a non-directory file, it skips the remaining files in the containing directory.¹ CL 276272 accidentally added a code path that triggers this behavior whenever filepath.Walk reaches a non-directory file that begins with a dot, such as .gitattributes or .DS_Store, causing findGorootModules to return early without finding any modules in GOROOT. Tests that use it ceased to provide test coverage that the tree is tidy. Add an explicit check for info.IsDir in the 5 places that intend to use filepath.SkipDir to skip traversing that directory. Even paths like GOROOT/bin and GOROOT/pkg which are unlikely to be anything but a directory are worth checking, since the goal of moddeps is to take a possibly problematic GOROOT tree as input and detect problems. While the goal of findGorootModules is to find all modules in GOROOT programmatically (in case new modules are added or modified), there are 4 modules now that are quite likely to exist, so check for their presence to avoid similar regressions. (It's not hard to update this test if a well-known GOROOT module is removed or otherwise modified; but if it becomes hard we can simplify it to check for a reasonable number of modules instead.) Also fix the minor skew that has crept in since the test got disabled. ¹ This wasn't necessarily an intentional design decision, but it was found only when Go 1.4 was already out. See CL 11690 for details. Fixes #46254. Change-Id: Id55ed926f8c0094b1af923070de72bacca05996f Reviewed-on: https://go-review.googlesource.com/c/go/+/320991 Trust: Dmitri Shuralyov Run-TryBot: Dmitri Shuralyov TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills Reviewed-by: Filippo Valsorda --- src/cmd/internal/moddeps/moddeps_test.go | 36 ++++-- src/go.mod | 2 +- src/go.sum | 4 +- src/net/http/h2_bundle.go | 133 ++++++++++++++++----- src/net/http/socks_bundle.go | 2 +- src/vendor/golang.org/x/sys/cpu/cpu.go | 5 +- src/vendor/golang.org/x/sys/cpu/cpu_aix.go | 1 + src/vendor/modules.txt | 2 +- 8 files changed, 137 insertions(+), 48 deletions(-) diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go index ba574f4004..7723250468 100644 --- a/src/cmd/internal/moddeps/moddeps_test.go +++ b/src/cmd/internal/moddeps/moddeps_test.go @@ -227,7 +227,7 @@ func makeGOROOTCopy(t *testing.T) string { if err != nil { return err } - if src == filepath.Join(runtime.GOROOT(), ".git") { + if info.IsDir() && src == filepath.Join(runtime.GOROOT(), ".git") { return filepath.SkipDir } @@ -237,9 +237,8 @@ func makeGOROOTCopy(t *testing.T) string { } dst := filepath.Join(gorootCopyDir, rel) - switch src { - case filepath.Join(runtime.GOROOT(), "bin"), - filepath.Join(runtime.GOROOT(), "pkg"): + if info.IsDir() && (src == filepath.Join(runtime.GOROOT(), "bin") || + src == filepath.Join(runtime.GOROOT(), "pkg")) { // If the OS supports symlinks, use them instead // of copying the bin and pkg directories. if err := os.Symlink(src, dst); err == nil { @@ -414,7 +413,7 @@ func findGorootModules(t *testing.T) []gorootModule { if info.IsDir() && (info.Name() == "vendor" || info.Name() == "testdata") { return filepath.SkipDir } - if path == filepath.Join(runtime.GOROOT(), "pkg") { + if info.IsDir() && path == filepath.Join(runtime.GOROOT(), "pkg") { // GOROOT/pkg contains generated artifacts, not source code. // // In https://golang.org/issue/37929 it was observed to somehow contain @@ -422,7 +421,7 @@ func findGorootModules(t *testing.T) []gorootModule { // running time of this test anyway.) return filepath.SkipDir } - if strings.HasPrefix(info.Name(), "_") || strings.HasPrefix(info.Name(), ".") { + if info.IsDir() && (strings.HasPrefix(info.Name(), "_") || strings.HasPrefix(info.Name(), ".")) { // _ and . prefixed directories can be used for internal modules // without a vendor directory that don't contribute to the build // but might be used for example as code generators. @@ -457,8 +456,31 @@ func findGorootModules(t *testing.T) []gorootModule { goroot.modules = append(goroot.modules, m) return nil }) - }) + if goroot.err != nil { + return + } + // knownGOROOTModules is a hard-coded list of modules that are known to exist in GOROOT. + // If findGorootModules doesn't find a module, it won't be covered by tests at all, + // so make sure at least these modules are found. See issue 46254. If this list + // becomes a nuisance to update, can be replaced with len(goroot.modules) check. + knownGOROOTModules := [...]string{ + "std", + "cmd", + "misc", + "test/bench/go1", + } + var seen = make(map[string]bool) // Key is module path. + for _, m := range goroot.modules { + seen[m.Path] = true + } + for _, m := range knownGOROOTModules { + if !seen[m] { + goroot.err = fmt.Errorf("findGorootModules didn't find the well-known module %q", m) + break + } + } + }) if goroot.err != nil { t.Fatal(goroot.err) } diff --git a/src/go.mod b/src/go.mod index 93ff2d8d3c..379dcf504e 100644 --- a/src/go.mod +++ b/src/go.mod @@ -5,6 +5,6 @@ go 1.17 require ( golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e golang.org/x/net v0.0.0-20210510120150-4163338589ed - golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6 // indirect + golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 // indirect golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f // indirect ) diff --git a/src/go.sum b/src/go.sum index 1390ce6d45..b3de6c526c 100644 --- a/src/go.sum +++ b/src/go.sum @@ -2,7 +2,7 @@ golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e h1:8foAy0aoO5GkqCvAEJ4VC4 golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= golang.org/x/net v0.0.0-20210510120150-4163338589ed h1:p9UgmWI9wKpfYmgaV/IZKGdXc5qEK45tDwwwDyjS26I= golang.org/x/net v0.0.0-20210510120150-4163338589ed/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6 h1:cdsMqa2nXzqlgs183pHxtvoVwU7CyzaCTAUOg94af4c= -golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 h1:yhBbb4IRs2HS9PPlAg6DMC6mUOKexJBNsLf4Z+6En1Q= +golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f h1:yQJrRE0hDxDFmZLlRaw+3vusO4fwNHgHIjUOMO7bHYI= golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index fd540ff255..a948ff3eed 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -53,6 +53,48 @@ import ( "golang.org/x/net/idna" ) +// asciiEqualFold is strings.EqualFold, ASCII only. It reports whether s and t +// are equal, ASCII-case-insensitively. +func http2asciiEqualFold(s, t string) bool { + if len(s) != len(t) { + return false + } + for i := 0; i < len(s); i++ { + if http2lower(s[i]) != http2lower(t[i]) { + return false + } + } + return true +} + +// lower returns the ASCII lowercase version of b. +func http2lower(b byte) byte { + if 'A' <= b && b <= 'Z' { + return b + ('a' - 'A') + } + return b +} + +// isASCIIPrint returns whether s is ASCII and printable according to +// https://tools.ietf.org/html/rfc20#section-4.2. +func http2isASCIIPrint(s string) bool { + for i := 0; i < len(s); i++ { + if s[i] < ' ' || s[i] > '~' { + return false + } + } + return true +} + +// asciiToLower returns the lowercase version of s if s is ASCII and printable, +// and whether or not it was. +func http2asciiToLower(s string) (lower string, ok bool) { + if !http2isASCIIPrint(s) { + return "", false + } + return strings.ToLower(s), true +} + // A list of the possible cipher suite ids. Taken from // https://www.iana.org/assignments/tls-parameters/tls-parameters.txt @@ -2907,6 +2949,20 @@ func http2traceGot1xxResponseFunc(trace *httptrace.ClientTrace) func(int, textpr return nil } +// dialTLSWithContext uses tls.Dialer, added in Go 1.15, to open a TLS +// connection. +func (t *http2Transport) dialTLSWithContext(ctx context.Context, network, addr string, cfg *tls.Config) (*tls.Conn, error) { + dialer := &tls.Dialer{ + Config: cfg, + } + cn, err := dialer.DialContext(ctx, network, addr) + if err != nil { + return nil, err + } + tlsCn := cn.(*tls.Conn) // DialContext comment promises this will always succeed + return tlsCn, nil +} + var http2DebugGoroutines = os.Getenv("DEBUG_HTTP2_GOROUTINES") == "1" type http2goroutineLock uint64 @@ -3128,12 +3184,12 @@ func http2buildCommonHeaderMaps() { } } -func http2lowerHeader(v string) string { +func http2lowerHeader(v string) (lower string, ascii bool) { http2buildCommonHeaderMapsOnce() if s, ok := http2commonLowerHeader[v]; ok { - return s + return s, true } - return strings.ToLower(v) + return http2asciiToLower(v) } var ( @@ -3831,13 +3887,12 @@ func http2ConfigureServer(s *Server, conf *http2Server) error { if s.TLSConfig == nil { s.TLSConfig = new(tls.Config) - } else if s.TLSConfig.CipherSuites != nil { - // If they already provided a CipherSuite list, return - // an error if it has a bad order or is missing - // ECDHE_RSA_WITH_AES_128_GCM_SHA256 or ECDHE_ECDSA_WITH_AES_128_GCM_SHA256. + } else if s.TLSConfig.CipherSuites != nil && s.TLSConfig.MinVersion < tls.VersionTLS13 { + // If they already provided a TLS 1.0–1.2 CipherSuite list, return an + // error if it is missing ECDHE_RSA_WITH_AES_128_GCM_SHA256 or + // ECDHE_ECDSA_WITH_AES_128_GCM_SHA256. haveRequired := false - sawBad := false - for i, cs := range s.TLSConfig.CipherSuites { + for _, cs := range s.TLSConfig.CipherSuites { switch cs { case tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, // Alternative MTI cipher to not discourage ECDSA-only servers. @@ -3845,14 +3900,9 @@ func http2ConfigureServer(s *Server, conf *http2Server) error { tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: haveRequired = true } - if http2isBadCipher(cs) { - sawBad = true - } else if sawBad { - return fmt.Errorf("http2: TLSConfig.CipherSuites index %d contains an HTTP/2-approved cipher suite (%#04x), but it comes after unapproved cipher suites. With this configuration, clients that don't support previous, approved cipher suites may be given an unapproved one and reject the connection.", i, cs) - } } if !haveRequired { - return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256).") + return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)") } } @@ -6394,8 +6444,12 @@ func (w *http2responseWriter) Push(target string, opts *PushOptions) error { // but PUSH_PROMISE requests cannot have a body. // http://tools.ietf.org/html/rfc7540#section-8.2 // Also disallow Host, since the promised URL must be absolute. - switch strings.ToLower(k) { - case "content-length", "content-encoding", "trailer", "te", "expect", "host": + if http2asciiEqualFold(k, "content-length") || + http2asciiEqualFold(k, "content-encoding") || + http2asciiEqualFold(k, "trailer") || + http2asciiEqualFold(k, "te") || + http2asciiEqualFold(k, "expect") || + http2asciiEqualFold(k, "host") { return fmt.Errorf("promised request headers cannot include %q", k) } } @@ -7148,14 +7202,10 @@ func (t *http2Transport) dialTLS(ctx context.Context) func(string, string, *tls. return t.DialTLS } return func(network, addr string, cfg *tls.Config) (net.Conn, error) { - dialer := &tls.Dialer{ - Config: cfg, - } - cn, err := dialer.DialContext(ctx, network, addr) + tlsCn, err := t.dialTLSWithContext(ctx, network, addr, cfg) if err != nil { return nil, err } - tlsCn := cn.(*tls.Conn) // DialContext comment promises this will always succeed state := tlsCn.ConnectionState() if p := state.NegotiatedProtocol; p != http2NextProtoTLS { return nil, fmt.Errorf("http2: unexpected ALPN protocol %q; want %q", p, http2NextProtoTLS) @@ -7163,7 +7213,7 @@ func (t *http2Transport) dialTLS(ctx context.Context) func(string, string, *tls. if !state.NegotiatedProtocolIsMutual { return nil, errors.New("http2: could not negotiate protocol mutually") } - return cn, nil + return tlsCn, nil } } @@ -7552,7 +7602,7 @@ func http2checkConnHeaders(req *Request) error { if vv := req.Header["Transfer-Encoding"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && vv[0] != "chunked") { return fmt.Errorf("http2: invalid Transfer-Encoding request header: %q", vv) } - if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && !strings.EqualFold(vv[0], "close") && !strings.EqualFold(vv[0], "keep-alive")) { + if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && !http2asciiEqualFold(vv[0], "close") && !http2asciiEqualFold(vv[0], "keep-alive")) { return fmt.Errorf("http2: invalid Connection request header: %q", vv) } return nil @@ -8078,19 +8128,21 @@ func (cc *http2ClientConn) encodeHeaders(req *Request, addGzipHeader bool, trail var didUA bool for k, vv := range req.Header { - if strings.EqualFold(k, "host") || strings.EqualFold(k, "content-length") { + if http2asciiEqualFold(k, "host") || http2asciiEqualFold(k, "content-length") { // Host is :authority, already sent. // Content-Length is automatic, set below. continue - } else if strings.EqualFold(k, "connection") || strings.EqualFold(k, "proxy-connection") || - strings.EqualFold(k, "transfer-encoding") || strings.EqualFold(k, "upgrade") || - strings.EqualFold(k, "keep-alive") { + } else if http2asciiEqualFold(k, "connection") || + http2asciiEqualFold(k, "proxy-connection") || + http2asciiEqualFold(k, "transfer-encoding") || + http2asciiEqualFold(k, "upgrade") || + http2asciiEqualFold(k, "keep-alive") { // Per 8.1.2.2 Connection-Specific Header // Fields, don't send connection-specific // fields. We have already checked if any // are error-worthy so just ignore the rest. continue - } else if strings.EqualFold(k, "user-agent") { + } else if http2asciiEqualFold(k, "user-agent") { // Match Go's http1 behavior: at most one // User-Agent. If set to nil or empty string, // then omit it. Otherwise if not mentioned, @@ -8103,7 +8155,7 @@ func (cc *http2ClientConn) encodeHeaders(req *Request, addGzipHeader bool, trail if vv[0] == "" { continue } - } else if strings.EqualFold(k, "cookie") { + } else if http2asciiEqualFold(k, "cookie") { // Per 8.1.2.5 To allow for better compression efficiency, the // Cookie header field MAY be split into separate header fields, // each with one or more cookie-pairs. @@ -8162,7 +8214,12 @@ func (cc *http2ClientConn) encodeHeaders(req *Request, addGzipHeader bool, trail // Header list size is ok. Write the headers. enumerateHeaders(func(name, value string) { - name = strings.ToLower(name) + name, ascii := http2asciiToLower(name) + if !ascii { + // Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header + // field names have to be ASCII characters (just as in HTTP/1.x). + return + } cc.writeHeader(name, value) if traceHeaders { http2traceWroteHeaderField(trace, name, value) @@ -8210,9 +8267,14 @@ func (cc *http2ClientConn) encodeTrailers(req *Request) ([]byte, error) { } for k, vv := range req.Trailer { + lowKey, ascii := http2asciiToLower(k) + if !ascii { + // Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header + // field names have to be ASCII characters (just as in HTTP/1.x). + continue + } // Transfer-Encoding, etc.. have already been filtered at the // start of RoundTrip - lowKey := strings.ToLower(k) for _, v := range vv { cc.writeHeader(lowKey, v) } @@ -9635,7 +9697,12 @@ func http2encodeHeaders(enc *hpack.Encoder, h Header, keys []string) { } for _, k := range keys { vv := h[k] - k = http2lowerHeader(k) + k, ascii := http2lowerHeader(k) + if !ascii { + // Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header + // field names have to be ASCII characters (just as in HTTP/1.x). + continue + } if !http2validWireHeaderFieldName(k) { // Skip it as backup paranoia. Per // golang.org/issue/14048, these should diff --git a/src/net/http/socks_bundle.go b/src/net/http/socks_bundle.go index e6db1c7640..e446669589 100644 --- a/src/net/http/socks_bundle.go +++ b/src/net/http/socks_bundle.go @@ -453,7 +453,7 @@ func (up *socksUsernamePassword) Authenticate(ctx context.Context, rw io.ReadWri b = append(b, up.Username...) b = append(b, byte(len(up.Password))) b = append(b, up.Password...) - // TODO(mikio): handle IO deadlines and cancellation if + // TODO(mikio): handle IO deadlines and cancelation if // necessary if _, err := rw.Write(b); err != nil { return err diff --git a/src/vendor/golang.org/x/sys/cpu/cpu.go b/src/vendor/golang.org/x/sys/cpu/cpu.go index f77701fe86..abbec2d44b 100644 --- a/src/vendor/golang.org/x/sys/cpu/cpu.go +++ b/src/vendor/golang.org/x/sys/cpu/cpu.go @@ -154,14 +154,13 @@ var MIPS64X struct { // For ppc64/ppc64le, it is safe to check only for ISA level starting on ISA v3.00, // since there are no optional categories. There are some exceptions that also // require kernel support to work (DARN, SCV), so there are feature bits for -// those as well. The minimum processor requirement is POWER8 (ISA 2.07). -// The struct is padded to avoid false sharing. +// those as well. The struct is padded to avoid false sharing. var PPC64 struct { _ CacheLinePad HasDARN bool // Hardware random number generator (requires kernel enablement) HasSCV bool // Syscall vectored (requires kernel enablement) IsPOWER8 bool // ISA v2.07 (POWER8) - IsPOWER9 bool // ISA v3.00 (POWER9) + IsPOWER9 bool // ISA v3.00 (POWER9), implies IsPOWER8 _ CacheLinePad } diff --git a/src/vendor/golang.org/x/sys/cpu/cpu_aix.go b/src/vendor/golang.org/x/sys/cpu/cpu_aix.go index 28b521643b..8aaeef545a 100644 --- a/src/vendor/golang.org/x/sys/cpu/cpu_aix.go +++ b/src/vendor/golang.org/x/sys/cpu/cpu_aix.go @@ -20,6 +20,7 @@ func archInit() { PPC64.IsPOWER8 = true } if impl&_IMPL_POWER9 != 0 { + PPC64.IsPOWER8 = true PPC64.IsPOWER9 = true } diff --git a/src/vendor/modules.txt b/src/vendor/modules.txt index 1b850342a5..ff01db5cdc 100644 --- a/src/vendor/modules.txt +++ b/src/vendor/modules.txt @@ -18,7 +18,7 @@ golang.org/x/net/idna golang.org/x/net/lif golang.org/x/net/nettest golang.org/x/net/route -# golang.org/x/sys v0.0.0-20210503173754-0981d6026fa6 +# golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 ## explicit; go 1.17 golang.org/x/sys/cpu # golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f From a8d85918b63d481a414ec5ca3978d07b2b047363 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Wed, 19 May 2021 22:51:11 +0000 Subject: [PATCH 07/40] misc/cgo/testplugin: skip TestIssue25756pie on darwin/arm64 builder This test is known to be broken on the darwin/arm64 builder. Skip it while it's being investigated so it doesn't mask other failures. For #46239. Updates #43228. Change-Id: I8fe57a0636bba84c3100337146dcb96cc264e524 Reviewed-on: https://go-review.googlesource.com/c/go/+/321349 Trust: Dmitri Shuralyov Run-TryBot: Dmitri Shuralyov TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- misc/cgo/testplugin/plugin_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/misc/cgo/testplugin/plugin_test.go b/misc/cgo/testplugin/plugin_test.go index a6accc1dfb..9697dbf7a7 100644 --- a/misc/cgo/testplugin/plugin_test.go +++ b/misc/cgo/testplugin/plugin_test.go @@ -265,6 +265,10 @@ func TestIssue25756(t *testing.T) { // Test with main using -buildmode=pie with plugin for issue #43228 func TestIssue25756pie(t *testing.T) { + if os.Getenv("GO_BUILDER_NAME") == "darwin-arm64-11_0-toothrot" { + t.Skip("broken on darwin/arm64 builder in sharded mode; see issue 46239") + } + goCmd(t, "build", "-buildmode=plugin", "-o", "life.so", "./issue25756/plugin") goCmd(t, "build", "-buildmode=pie", "-o", "issue25756pie.exe", "./issue25756/main.go") run(t, "./issue25756pie.exe") From f07e4dae3c5cb608b4f0b9db57d1562d2125243b Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Wed, 19 May 2021 15:46:44 +0000 Subject: [PATCH 08/40] syscall: document NewCallback and NewCallbackCDecl limitations Currently NewCallback and NewCallbackCDecl may only be called a limited number of times in a single Go process, but this property of the API is not documented. This change fixes that, but does not document the precise limit to avoid making that limit part of the API, leaving us open to increasing or decreasing the limit in the future as needed. Although the API avoids documenting a limit, it does guarantee a minimum callback count so users can rely on at least some amount of callbacks working. Updates #46184. Change-Id: I5129bf5fe301efff73ac112ba1f207ab32058833 Reviewed-on: https://go-review.googlesource.com/c/go/+/321133 Trust: Michael Knyszek Reviewed-by: Ian Lance Taylor --- src/syscall/syscall_windows.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go index fa0b5d959a..fc734effbb 100644 --- a/src/syscall/syscall_windows.go +++ b/src/syscall/syscall_windows.go @@ -174,6 +174,9 @@ func compileCallback(fn interface{}, cleanstack bool) uintptr // NewCallback converts a Go function to a function pointer conforming to the stdcall calling convention. // This is useful when interoperating with Windows code requiring callbacks. // The argument is expected to be a function with one uintptr-sized result. The function must not have arguments with size larger than the size of uintptr. +// Only a limited number of callbacks may be created in a single Go process, and any memory allocated +// for these callbacks is never released. +// Between NewCallback and NewCallbackCDecl, at least 1024 callbacks can always be created. func NewCallback(fn interface{}) uintptr { return compileCallback(fn, true) } @@ -181,6 +184,9 @@ func NewCallback(fn interface{}) uintptr { // NewCallbackCDecl converts a Go function to a function pointer conforming to the cdecl calling convention. // This is useful when interoperating with Windows code requiring callbacks. // The argument is expected to be a function with one uintptr-sized result. The function must not have arguments with size larger than the size of uintptr. +// Only a limited number of callbacks may be created in a single Go process, and any memory allocated +// for these callbacks is never released. +// Between NewCallback and NewCallbackCDecl, at least 1024 callbacks can always be created. func NewCallbackCDecl(fn interface{}) uintptr { return compileCallback(fn, false) } From bb7495a46d6024da9d77722f04b438e573bcb26f Mon Sep 17 00:00:00 2001 From: Vishal Dalwadi Date: Fri, 7 May 2021 10:25:27 +0530 Subject: [PATCH 09/40] doc/go1.17: document new math constants Documents the newly introduced: * MaxInt * MinInt * MaxUint Updates #28538. For #44513. Fixes #46012. Change-Id: Iab6bbcf8f76ebe105b973d5fd39b86b8cd078348 Reviewed-on: https://go-review.googlesource.com/c/go/+/317911 Trust: Heschi Kreinick Reviewed-by: Emmanuel Odeke --- doc/go1.17.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 4b2f4bce79..4c7348a36d 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -334,7 +334,9 @@ Do not send CLs removing the interior tags from such phrases.
math

- TODO: https://golang.org/cl/247058: add MaxUint, MinInt, MaxInt + The math package now defines three more constants: MaxUint, MaxInt and MinInt. + For 32-bit systems their values are 2^32 - 1, 2^31 - 1 and -2^31, respectively. + For 64-bit systems their values are 2^64 - 1, 2^63 - 1 and -2^63, respectively.

From ef1f52cc38cc8773a4ae2e4e71219140a08ce98f Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Wed, 19 May 2021 16:07:52 -0400 Subject: [PATCH 10/40] doc/go1.17: add release note for windows/arm64 port Updates #44513, #42604. Change-Id: I8200e8087c219a0042ab2a6770a7275c3b17942a Reviewed-on: https://go-review.googlesource.com/c/go/+/321309 Trust: Cherry Mui Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/go1.17.html b/doc/go1.17.html index 4c7348a36d..ea8bc3ccd6 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -43,6 +43,13 @@ Do not send CLs removing the interior tags from such phrases. for previous versions has been discontinued.

+

Windows

+ +

+ Go 1.17 adds support of 64-bit ARM architecture on Windows (the + windows/arm64 port). This port supports cgo. +

+

TODO: complete the Ports section

From def53605418c8134880ffa3700c6a4ba6c689234 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Wed, 19 May 2021 16:22:34 -0400 Subject: [PATCH 11/40] doc/go1.17: add release notes for OpenBSD ports Updates #44513. Change-Id: I8758768f6231fd7fcbaa7109eb49ee56cd60d93d Reviewed-on: https://go-review.googlesource.com/c/go/+/321310 Trust: Cherry Mui Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/go1.17.html b/doc/go1.17.html index ea8bc3ccd6..97307bc508 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -50,6 +50,24 @@ Do not send CLs removing the interior tags from such phrases. windows/arm64 port). This port supports cgo.

+

OpenBSD

+ +

+ The 64-bit MIPS architecture on OpenBSD (the openbsd/mips64 + port) now supports cgo. +

+ +

+ In Go 1.16, on the 64-bit x86 and 64-bit ARM architectures on + OpenBSD (the openbsd/amd64 and openbsd/arm64 + ports) system calls are made through libc, instead + of directly using the machine instructions. In Go 1.17, this is + also done on the 32-bit x86 and 32-bit ARM architectures on OpenBSD + (the openbsd/386 and openbsd/arm ports). + This ensures forward-compatibility with future versions of + OpenBSD. +

+

TODO: complete the Ports section

From f8be906d7437f2528abc7cd1a57fe46aa9348b97 Mon Sep 17 00:00:00 2001 From: Joel Sing Date: Thu, 20 May 2021 02:23:04 +1000 Subject: [PATCH 12/40] test: re-enable test on riscv64 now that it supports external linking Update #36739 Change-Id: I14ab2cd0e29966b9a2f992e8c3bcb415203e63e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/321449 Trust: Joel Sing Reviewed-by: Cherry Mui Run-TryBot: Cherry Mui TryBot-Result: Go Bot --- test/fixedbugs/issue10607.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/fixedbugs/issue10607.go b/test/fixedbugs/issue10607.go index 448a37dcac..8a04bc9def 100644 --- a/test/fixedbugs/issue10607.go +++ b/test/fixedbugs/issue10607.go @@ -1,4 +1,4 @@ -// +build linux,!ppc64,!riscv64,gc +// +build linux,!ppc64,gc // run // Copyright 2015 The Go Authors. All rights reserved. @@ -8,9 +8,6 @@ // Test that a -B option is passed through when using both internal // and external linking mode. -// TODO(jsing): Re-enable on riscv64 when it has support for external -// linking - see golang.org/issue/36739 - package main import ( From ce9a3b79d5bb783f5f31c9d41665a488fe63f546 Mon Sep 17 00:00:00 2001 From: Lapo Luchini Date: Thu, 20 May 2021 15:41:02 +0000 Subject: [PATCH 13/40] crypto/x509: add new FreeBSD 12.2+ trusted certificate folder Up to FreeBSD 12.1 the package ca_root_nss was needed in order to have certificates under /usr/local/share/certs as the base system didn't have a system trusted certificate store. This has been fixed in FreeBSD 12.2 using /etc/ssl/certs: https://svnweb.freebsd.org/base?view=revision&revision=357082 Fixes #46284 Change-Id: I912b1bacc30cdf20d19e3ef9d09b69bb8055ff49 GitHub-Last-Rev: 0fa5542ea3c70ecb03e621381d7c34fbadf7ea47 GitHub-Pull-Request: golang/go#46276 Reviewed-on: https://go-review.googlesource.com/c/go/+/321190 Reviewed-by: Filippo Valsorda Run-TryBot: Filippo Valsorda TryBot-Result: Go Bot Trust: Tobias Klauser --- src/crypto/x509/root_bsd.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/crypto/x509/root_bsd.go b/src/crypto/x509/root_bsd.go index 822e8573ff..6712ea32a6 100644 --- a/src/crypto/x509/root_bsd.go +++ b/src/crypto/x509/root_bsd.go @@ -18,6 +18,7 @@ var certFiles = []string{ // Possible directories with certificate files; stop after successfully // reading at least one file from a directory. var certDirectories = []string{ + "/etc/ssl/certs", // FreeBSD 12.2+ "/usr/local/share/certs", // FreeBSD "/etc/openssl/certs", // NetBSD } From 7c692cc7ea69ba4d8603fbada112289443a6f526 Mon Sep 17 00:00:00 2001 From: Vishal Dalwadi Date: Fri, 7 May 2021 10:05:36 +0530 Subject: [PATCH 14/40] doc/go1.17: document changes to os package Documents the changes to File.WriteString method. For #44513. Fixes #46018. Change-Id: I3a8ef9df9f84662614d54802710bd705d626b995 Reviewed-on: https://go-review.googlesource.com/c/go/+/317910 Reviewed-by: Dmitri Shuralyov Trust: Cherry Mui --- doc/go1.17.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 97307bc508..48b5563602 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -428,7 +428,8 @@ Do not send CLs removing the interior tags from such phrases.
os

- TODO: https://golang.org/cl/268020: avoid allocation in File.WriteString + The File.WriteString method + has been optimized to no longer make a copy of the input string.

From baa934d26dd6e201dcda0962dd51b0e5f6f69c1b Mon Sep 17 00:00:00 2001 From: Tim King Date: Wed, 19 May 2021 18:45:53 -0700 Subject: [PATCH 15/40] cmd: go get golang.org/x/tools/analysis@49064d23 && go mod vendor This brings in CLs 312829, 317431, 319211. Fixes #40356. Fixes #46129. Change-Id: I2ee1f858b2a41ffa60d88b0c17511ccad57f1816 Reviewed-on: https://go-review.googlesource.com/c/go/+/321389 Reviewed-by: Dmitri Shuralyov Trust: Tim King Run-TryBot: Tim King TryBot-Result: Go Bot --- src/cmd/go.mod | 2 +- src/cmd/go.sum | 4 ++-- .../x/tools/go/analysis/passes/printf/printf.go | 2 +- .../analysis/passes/sigchanyzer/sigchanyzer.go | 11 +++++++++-- .../go/analysis/passes/stdmethods/stdmethods.go | 17 +++++++++++++++++ src/cmd/vendor/modules.txt | 2 +- 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/cmd/go.mod b/src/cmd/go.mod index 88f5f2883a..1aa0320d07 100644 --- a/src/cmd/go.mod +++ b/src/cmd/go.mod @@ -10,6 +10,6 @@ require ( golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 // indirect golang.org/x/term v0.0.0-20210503060354-a79de5458b56 - golang.org/x/tools v0.1.1-0.20210505014545-7cab0ef2e9a5 + golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect ) diff --git a/src/cmd/go.sum b/src/cmd/go.sum index 73750802bc..9af4978d66 100644 --- a/src/cmd/go.sum +++ b/src/cmd/go.sum @@ -16,7 +16,7 @@ golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 h1:yhBbb4IRs2HS9PPlAg6DMC6mU golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20210503060354-a79de5458b56 h1:b8jxX3zqjpqb2LklXPzKSGJhzyxCOZSz8ncv8Nv+y7w= golang.org/x/term v0.0.0-20210503060354-a79de5458b56/go.mod h1:tfny5GFUkzUvx4ps4ajbZsCe5lw1metzhBm9T3x7oIY= -golang.org/x/tools v0.1.1-0.20210505014545-7cab0ef2e9a5 h1:ImcI7RFHWLu2QWpFDXaReu0j+sQAHIy65vUFZImXiqY= -golang.org/x/tools v0.1.1-0.20210505014545-7cab0ef2e9a5/go.mod h1:sH/Eidr0EddymY8HZSakBo32zU3fG5ovDq874hJLjVg= +golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9 h1:2XlR/j4I4xz5GQZI7zBjqTfezYyRIE2jD5IMousB2rg= +golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go index ddad4c796c..822820f06e 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go @@ -590,7 +590,7 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.F } if state.verb == 'w' { if kind != KindErrorf { - pass.Reportf(call.Pos(), "%s call has error-wrapping directive %%w", state.name) + pass.Reportf(call.Pos(), "%s call has error-wrapping directive %%w, which is only supported by Errorf", state.name) return } if anyW { diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer/sigchanyzer.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer/sigchanyzer.go index b00aa7e144..0d6c8ebf16 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer/sigchanyzer.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer/sigchanyzer.go @@ -59,12 +59,19 @@ func run(pass *analysis.Pass) (interface{}, error) { if chanDecl == nil || len(chanDecl.Args) != 1 { return } - chanDecl.Args = append(chanDecl.Args, &ast.BasicLit{ + + // Make a copy of the channel's declaration to avoid + // mutating the AST. See https://golang.org/issue/46129. + chanDeclCopy := &ast.CallExpr{} + *chanDeclCopy = *chanDecl + chanDeclCopy.Args = append([]ast.Expr(nil), chanDecl.Args...) + chanDeclCopy.Args = append(chanDeclCopy.Args, &ast.BasicLit{ Kind: token.INT, Value: "1", }) + var buf bytes.Buffer - if err := format.Node(&buf, token.NewFileSet(), chanDecl); err != nil { + if err := format.Node(&buf, token.NewFileSet(), chanDeclCopy); err != nil { return } pass.Report(analysis.Diagnostic{ diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/stdmethods/stdmethods.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/stdmethods/stdmethods.go index 856c6ae0d8..64a28ac0b9 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/stdmethods/stdmethods.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/stdmethods/stdmethods.go @@ -61,10 +61,12 @@ var Analyzer = &analysis.Analyzer{ // we let it go. But if it does have a fmt.ScanState, then the // rest has to match. var canonicalMethods = map[string]struct{ args, results []string }{ + "As": {[]string{"interface{}"}, []string{"bool"}}, // errors.As // "Flush": {{}, {"error"}}, // http.Flusher and jpeg.writer conflict "Format": {[]string{"=fmt.State", "rune"}, []string{}}, // fmt.Formatter "GobDecode": {[]string{"[]byte"}, []string{"error"}}, // gob.GobDecoder "GobEncode": {[]string{}, []string{"[]byte", "error"}}, // gob.GobEncoder + "Is": {[]string{"error"}, []string{"bool"}}, // errors.Is "MarshalJSON": {[]string{}, []string{"[]byte", "error"}}, // json.Marshaler "MarshalXML": {[]string{"*xml.Encoder", "xml.StartElement"}, []string{"error"}}, // xml.Marshaler "ReadByte": {[]string{}, []string{"byte", "error"}}, // io.ByteReader @@ -76,6 +78,7 @@ var canonicalMethods = map[string]struct{ args, results []string }{ "UnmarshalXML": {[]string{"*xml.Decoder", "xml.StartElement"}, []string{"error"}}, // xml.Unmarshaler "UnreadByte": {[]string{}, []string{"error"}}, "UnreadRune": {[]string{}, []string{"error"}}, + "Unwrap": {[]string{}, []string{"error"}}, // errors.Unwrap "WriteByte": {[]string{"byte"}, []string{"error"}}, // jpeg.writer (matching bufio.Writer) "WriteTo": {[]string{"=io.Writer"}, []string{"int64", "error"}}, // io.WriterTo } @@ -123,6 +126,14 @@ func canonicalMethod(pass *analysis.Pass, id *ast.Ident) { return } + // Special case: Is, As and Unwrap only apply when type + // implements error. + if id.Name == "Is" || id.Name == "As" || id.Name == "Unwrap" { + if recv := sign.Recv(); recv == nil || !implementsError(recv.Type()) { + return + } + } + // Do the =s (if any) all match? if !matchParams(pass, expect.args, args, "=") || !matchParams(pass, expect.results, results, "=") { return @@ -185,3 +196,9 @@ func matchParamType(expect string, actual types.Type) bool { // Overkill but easy. return typeString(actual) == expect } + +var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface) + +func implementsError(actual types.Type) bool { + return types.Implements(actual, errorType) +} diff --git a/src/cmd/vendor/modules.txt b/src/cmd/vendor/modules.txt index 016ec011a9..9a1723d32c 100644 --- a/src/cmd/vendor/modules.txt +++ b/src/cmd/vendor/modules.txt @@ -48,7 +48,7 @@ golang.org/x/sys/windows # golang.org/x/term v0.0.0-20210503060354-a79de5458b56 ## explicit; go 1.17 golang.org/x/term -# golang.org/x/tools v0.1.1-0.20210505014545-7cab0ef2e9a5 +# golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9 ## explicit; go 1.17 golang.org/x/tools/cover golang.org/x/tools/go/analysis From 831573cd21e65c37b4e1bf5d44dc23b125084b7a Mon Sep 17 00:00:00 2001 From: Adam Mitha Date: Thu, 13 May 2021 20:52:41 -0700 Subject: [PATCH 16/40] io/fs: added an example for io/fs.WalkDir The documentation currently does not show how to get an `FS` instance for the operating system's filesystem. This example demonstrates how to accomplish this using the `os` package. Fixes #46083 Change-Id: I053111c12ab09ef13f0d04fcdff8a6ea0dccf379 Reviewed-on: https://go-review.googlesource.com/c/go/+/319989 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Emmanuel Odeke --- src/io/fs/example_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/io/fs/example_test.go diff --git a/src/io/fs/example_test.go b/src/io/fs/example_test.go new file mode 100644 index 0000000000..c9027034c4 --- /dev/null +++ b/src/io/fs/example_test.go @@ -0,0 +1,25 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + "fmt" + "io/fs" + "log" + "os" +) + +func ExampleWalkDir() { + root := "/usr/local/go/bin" + fileSystem := os.DirFS(root) + + fs.WalkDir(fileSystem, ".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + log.Fatal(err) + } + fmt.Println(path) + return nil + }) +} From 7e63c8b765c30823131bd136d190afbe4c21abc9 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 21 May 2021 12:01:39 +0200 Subject: [PATCH 17/40] runtime: wait for Go runtime to initialize in Windows signal test The test harness waits for "ready" as a sign that the Go runtime has installed its signal handler and is ready to be tested. But actually, while LoadLibrary starts the loading of the Go runtime, it does so asynchronously, so the "ready" sign is potentially premature and certainly racy. However, all exported cgo entry points make a call to _cgo_wait_runtime_init_done which waits for that asynchronous initialization to complete. Therefore, this commit fixes the test to call into the exported "Dummy" cgo function before emitting the "ready" sign, so that we're sure the Go runtime is actually loaded. Updates #45638. Change-Id: I9b12b172d45bdcc09d54dd301de3a3e499544834 Reviewed-on: https://go-review.googlesource.com/c/go/+/321769 Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Michael Knyszek --- src/runtime/testdata/testwinlibsignal/main.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/runtime/testdata/testwinlibsignal/main.c b/src/runtime/testdata/testwinlibsignal/main.c index 1787fef3b9..37f24823e6 100644 --- a/src/runtime/testdata/testwinlibsignal/main.c +++ b/src/runtime/testdata/testwinlibsignal/main.c @@ -19,13 +19,13 @@ int main(void) { waitForCtrlBreakEvent = CreateEvent(NULL, TRUE, FALSE, NULL); if (!waitForCtrlBreakEvent) { - fprintf(stderr, "ERROR: Could not create event"); + fprintf(stderr, "ERROR: Could not create event\n"); return 1; } if (!SetConsoleCtrlHandler(CtrlHandler, TRUE)) { - fprintf(stderr, "ERROR: Could not set control handler"); + fprintf(stderr, "ERROR: Could not set control handler\n"); return 1; } @@ -34,7 +34,14 @@ int main(void) // This way the library handler gets called first. HMODULE dummyDll = LoadLibrary("dummy.dll"); if (!dummyDll) { - fprintf(stderr, "ERROR: Could not load dummy.dll"); + fprintf(stderr, "ERROR: Could not load dummy.dll\n"); + return 1; + } + + // Call the Dummy function so that Go initialization completes, since + // all cgo entry points call out to _cgo_wait_runtime_init_done. + if (((int(*)(void))GetProcAddress(dummyDll, "Dummy"))() != 42) { + fprintf(stderr, "ERROR: Dummy function did not return 42\n"); return 1; } @@ -42,7 +49,7 @@ int main(void) fflush(stdout); if (WaitForSingleObject(waitForCtrlBreakEvent, 5000) != WAIT_OBJECT_0) { - fprintf(stderr, "FAILURE: No signal received"); + fprintf(stderr, "FAILURE: No signal received\n"); return 1; } From 3148694f607b77731420a20ef2117ac7d0d55ba3 Mon Sep 17 00:00:00 2001 From: Tim Heckman Date: Thu, 20 May 2021 16:51:28 -0700 Subject: [PATCH 18/40] cmd/go: remove warning from module deprecation notice printing Fixes #46294 Change-Id: I50023006dab83dee455f98a702ca1c72e61764ea Reviewed-on: https://go-review.googlesource.com/c/go/+/321649 Trust: Jay Conrod Trust: Michael Matloob Run-TryBot: Jay Conrod TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- src/cmd/go/internal/modget/get.go | 2 +- src/cmd/go/testdata/script/mod_deprecate_message.txt | 8 ++++---- src/cmd/go/testdata/script/mod_get_deprecated.txt | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go index 2a7fe5226f..563f1a988f 100644 --- a/src/cmd/go/internal/modget/get.go +++ b/src/cmd/go/internal/modget/get.go @@ -1598,7 +1598,7 @@ func (r *resolver) checkPackageProblems(ctx context.Context, pkgPatterns []strin // Report deprecations, then retractions. for _, mm := range deprecations { if mm.message != "" { - fmt.Fprintf(os.Stderr, "go: warning: module %s is deprecated: %s\n", mm.m.Path, mm.message) + fmt.Fprintf(os.Stderr, "go: module %s is deprecated: %s\n", mm.m.Path, mm.message) } } var retractPath string diff --git a/src/cmd/go/testdata/script/mod_deprecate_message.txt b/src/cmd/go/testdata/script/mod_deprecate_message.txt index 4a0674b808..567027935d 100644 --- a/src/cmd/go/testdata/script/mod_deprecate_message.txt +++ b/src/cmd/go/testdata/script/mod_deprecate_message.txt @@ -1,26 +1,26 @@ # When there is a short single-line message, 'go get' should print it all. go get -d short -stderr '^go: warning: module short is deprecated: short$' +stderr '^go: module short is deprecated: short$' go list -m -u -f '{{.Deprecated}}' short stdout '^short$' # When there is a multi-line message, 'go get' should print the first line. go get -d multiline -stderr '^go: warning: module multiline is deprecated: first line$' +stderr '^go: module multiline is deprecated: first line$' ! stderr 'second line' go list -m -u -f '{{.Deprecated}}' multiline stdout '^first line\nsecond line.$' # When there is a long message, 'go get' should print a placeholder. go get -d long -stderr '^go: warning: module long is deprecated: \(message omitted: too long\)$' +stderr '^go: module long is deprecated: \(message omitted: too long\)$' go list -m -u -f '{{.Deprecated}}' long stdout '^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$' # When a message contains unprintable chracters, 'go get' should say that # without printing the message. go get -d unprintable -stderr '^go: warning: module unprintable is deprecated: \(message omitted: contains non-printable characters\)$' +stderr '^go: module unprintable is deprecated: \(message omitted: contains non-printable characters\)$' go list -m -u -f '{{.Deprecated}}' unprintable stdout '^message contains ASCII BEL\x07$' diff --git a/src/cmd/go/testdata/script/mod_get_deprecated.txt b/src/cmd/go/testdata/script/mod_get_deprecated.txt index 4633009f69..7bdd7a58a8 100644 --- a/src/cmd/go/testdata/script/mod_get_deprecated.txt +++ b/src/cmd/go/testdata/script/mod_get_deprecated.txt @@ -4,14 +4,14 @@ go get -d ./use/nothing # 'go get pkg' should show a deprecation message for the module providing pkg. go get -d example.com/deprecated/a -stderr '^go: warning: module example.com/deprecated/a is deprecated: in example.com/deprecated/a@v1.9.0$' +stderr '^go: module example.com/deprecated/a is deprecated: in example.com/deprecated/a@v1.9.0$' go get -d example.com/deprecated/a@v1.0.0 -stderr '^go: warning: module example.com/deprecated/a is deprecated: in example.com/deprecated/a@v1.9.0$' +stderr '^go: module example.com/deprecated/a is deprecated: in example.com/deprecated/a@v1.9.0$' # 'go get pkg' should show a deprecation message for a module providing # packages directly imported by pkg. go get -d ./use/a -stderr '^go: warning: module example.com/deprecated/a is deprecated: in example.com/deprecated/a@v1.9.0$' +stderr '^go: module example.com/deprecated/a is deprecated: in example.com/deprecated/a@v1.9.0$' # 'go get pkg' may show a deprecation message for an indirectly required module # if it provides a package named on the command line. @@ -20,7 +20,7 @@ go get -d ./use/b go get -d local/use ! stderr 'module.*is deprecated' go get -d example.com/deprecated/b -stderr '^go: warning: module example.com/deprecated/b is deprecated: in example.com/deprecated/b@v1.9.0$' +stderr '^go: module example.com/deprecated/b is deprecated: in example.com/deprecated/b@v1.9.0$' # 'go get pkg' does not show a deprecation message for a module providing a # directly imported package if the module is no longer deprecated in its From 5fee772c872fcbf35c059b241d0b60c0aecd0f20 Mon Sep 17 00:00:00 2001 From: Aaron Sheah Date: Tue, 18 May 2021 14:54:02 +0000 Subject: [PATCH 19/40] doc/go1.17: document archive/zip changes for Go 1.17 For #44513. Fixes #46000 Change-Id: I299d0b5657f1f96174d6e35d60daac8b36e59d29 GitHub-Last-Rev: e63461bff042a8abe79e0ec3515eefbf56ba1d82 GitHub-Pull-Request: golang/go#46235 Reviewed-on: https://go-review.googlesource.com/c/go/+/320809 Reviewed-by: Heschi Kreinick Trust: Robert Findley --- doc/go1.17.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 48b5563602..ae9deabf65 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -272,7 +272,7 @@ Do not send CLs removing the interior tags from such phrases.
archive/zip

- TODO: https://golang.org/cl/312310: add File.OpenRaw, Writer.CreateRaw, Writer.Copy + The new methods File.OpenRaw, Writer.CreateRaw, Writer.Copy provide support for cases where performance is a primary concern.

From 8876b9bd6a8a0c206c0a5d59302ce5167f26e9f3 Mon Sep 17 00:00:00 2001 From: Adam Mitha Date: Fri, 14 May 2021 10:58:01 -0700 Subject: [PATCH 20/40] doc/go1.17: document io/fs changes for Go 1.17 For #44513 Fixes #46011 Change-Id: I862ef9a4314cd34fb8c828a8cd7d0a7b36c6f683 Reviewed-on: https://go-review.googlesource.com/c/go/+/320151 Reviewed-by: Heschi Kreinick Trust: Heschi Kreinick Trust: Robert Findley Trust: Dmitri Shuralyov --- doc/go1.17.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index ae9deabf65..4561b6ccf5 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -351,7 +351,7 @@ Do not send CLs removing the interior tags from such phrases.
io/fs

- TODO: https://golang.org/cl/293649: implement FileInfoToDirEntry + The new FileInfoToDirEntry function converts a FileInfo to a DirEntry.

From 4fda54ce3f6761104e835fc4e7847f89e34b7d6d Mon Sep 17 00:00:00 2001 From: sryoya Date: Sun, 16 May 2021 23:36:05 +0900 Subject: [PATCH 21/40] doc/go1.17: document database/sql changes for Go 1.17 For #44513 Fixes #46008 Change-Id: If80d484f73a0eb6946abdc654eb2c0d3dd6db416 Reviewed-on: https://go-review.googlesource.com/c/go/+/320251 Reviewed-by: Heschi Kreinick Trust: Robert Findley --- doc/go1.17.html | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 4561b6ccf5..3534f7be04 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -320,11 +320,19 @@ Do not send CLs removing the interior tags from such phrases.
database/sql

- TODO: https://golang.org/cl/258360: close driver.Connector if it implements io.Closer + The DB.Close method now closes + the connector field if the type in this field implements the + io.Closer interface.

- TODO: https://golang.org/cl/311572: add NullInt16 and NullByte + The new + NullInt16 + and + NullByte + structs represent the int16 and byte values that may be null. These can be used as + destinations of the Scan method, + similar to NullString.

From 4fb10b2118cb16445f2d089f79beb3d32db3db12 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Mon, 10 May 2021 18:10:18 -0400 Subject: [PATCH 22/40] cmd/go: in 'go mod download' without args, don't save module zip sums 'go mod download' without arguments is frequently used to populate the module cache. It tends to fetch a lot of extra files (for modules in the build list that aren't needed to build packages in the main module). It's annoying when sums are written for these extra files. 'go mod download mod@version' will still write sums for specific modules in the build list. 'go mod download all' still has the previous behavior. For now, all invocations of 'go mod download' still update go.mod and go.sum with changes needed to load the build list (1.15 behavior). Fixes #45332 Change-Id: I9e17d18a7466ac7271a0e1a2b663f6b3cb168c97 Reviewed-on: https://go-review.googlesource.com/c/go/+/318629 Trust: Jay Conrod Reviewed-by: Bryan C. Mills --- doc/go1.17.html | 11 ++++++ src/cmd/go/internal/modcmd/download.go | 29 +++++++++++++-- .../go/testdata/mod/rsc.io_sampler_v1.2.1.txt | 2 +- src/cmd/go/testdata/script/mod_download.txt | 37 +++++++++++++++++-- .../script/mod_get_trailing_slash.txt | 10 +++-- src/cmd/go/testdata/script/mod_query.txt | 5 ++- src/cmd/go/testdata/script/mod_retract.txt | 8 ++-- 7 files changed, 85 insertions(+), 17 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 3534f7be04..f00c649e04 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -186,6 +186,17 @@ Do not send CLs removing the interior tags from such phrases. password-protected SSH keys.

+

go mod download

+ +

+ When go mod download is invoked without + arguments, it will no longer save sums for downloaded module content to + go.sum. It may still make changes to go.mod and + go.sum needed to load the build list. This is the same as the + behavior in Go 1.15. To save sums for all modules, use go + mod download all. +

+

TODO: https://golang.org/cl/249759: cmd/cover: replace code using optimized golang.org/x/tools/cover

diff --git a/src/cmd/go/internal/modcmd/download.go b/src/cmd/go/internal/modcmd/download.go index a6c6d914e1..42b06dbc95 100644 --- a/src/cmd/go/internal/modcmd/download.go +++ b/src/cmd/go/internal/modcmd/download.go @@ -86,9 +86,11 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { if !modload.HasModRoot() && len(args) == 0 { base.Fatalf("go mod download: no modules specified (see 'go help mod download')") } - if len(args) == 0 { + haveExplicitArgs := len(args) > 0 + if !haveExplicitArgs { args = []string{"all"} - } else if modload.HasModRoot() { + } + if modload.HasModRoot() { modload.LoadModFile(ctx) // to fill Target targetAtUpgrade := modload.Target.Path + "@upgrade" targetAtPatch := modload.Target.Path + "@patch" @@ -135,6 +137,18 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { type token struct{} sem := make(chan token, runtime.GOMAXPROCS(0)) infos, infosErr := modload.ListModules(ctx, args, 0) + if !haveExplicitArgs { + // 'go mod download' is sometimes run without arguments to pre-populate + // the module cache. It may fetch modules that aren't needed to build + // packages in the main mdoule. This is usually not intended, so don't save + // sums for downloaded modules (golang.org/issue/45332). + // TODO(golang.org/issue/45551): For now, save sums needed to load the + // build list (same as 1.15 behavior). In the future, report an error if + // go.mod or go.sum need to be updated after loading the build list. + modload.WriteGoMod(ctx) + modload.DisallowWriteGoMod() + } + for _, info := range infos { if info.Replace != nil { info = info.Replace @@ -185,8 +199,15 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { base.ExitIfErrors() } - // Update go.mod and especially go.sum if needed. - modload.WriteGoMod(ctx) + // If there were explicit arguments, update go.mod and especially go.sum. + // 'go mod download mod@version' is a useful way to add a sum without using + // 'go get mod@version', which may have other side effects. We print this in + // some error message hints. + // + // Don't save sums for 'go mod download' without arguments; see comment above. + if haveExplicitArgs { + modload.WriteGoMod(ctx) + } // If there was an error matching some of the requested packages, emit it now // (after we've written the checksums for the modules that were downloaded diff --git a/src/cmd/go/testdata/mod/rsc.io_sampler_v1.2.1.txt b/src/cmd/go/testdata/mod/rsc.io_sampler_v1.2.1.txt index 00b71bf0d5..7982cccea1 100644 --- a/src/cmd/go/testdata/mod/rsc.io_sampler_v1.2.1.txt +++ b/src/cmd/go/testdata/mod/rsc.io_sampler_v1.2.1.txt @@ -5,7 +5,7 @@ module "rsc.io/sampler" require "golang.org/x/text" v0.0.0-20170915032832-14c0d48ead0c -- .info -- -{"Version":"v1.2.1","Name":"cac3af4f8a0ab40054fa6f8d423108a63a1255bb","Short":"cac3af4f8a0a","Time":"2018-02-13T18:16:22Z"}EOF +{"Version":"v1.2.1","Name":"cac3af4f8a0ab40054fa6f8d423108a63a1255bb","Short":"cac3af4f8a0a","Time":"2018-02-13T18:16:22Z"} -- hello.go -- // Copyright 2018 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style diff --git a/src/cmd/go/testdata/script/mod_download.txt b/src/cmd/go/testdata/script/mod_download.txt index 8a9faffe4e..ad640b45de 100644 --- a/src/cmd/go/testdata/script/mod_download.txt +++ b/src/cmd/go/testdata/script/mod_download.txt @@ -107,13 +107,28 @@ stderr '^go mod download: skipping argument m that resolves to the main module\n ! go mod download m@latest stderr '^go mod download: m@latest: malformed module path "m": missing dot in first path element$' -# download updates go.mod and populates go.sum +# download without arguments updates go.mod and go.sum after loading the +# build list, but does not save sums for downloaded zips. cd update +cp go.mod.orig go.mod ! exists go.sum go mod download +cmp go.mod.update go.mod +cmp go.sum.update go.sum +cp go.mod.orig go.mod +rm go.sum + +# download with arguments (even "all") does update go.mod and go.sum. +go mod download rsc.io/sampler +cmp go.mod.update go.mod grep '^rsc.io/sampler v1.3.0 ' go.sum -go list -m rsc.io/sampler -stdout '^rsc.io/sampler v1.3.0$' +cp go.mod.orig go.mod +rm go.sum + +go mod download all +cmp go.mod.update go.mod +grep '^rsc.io/sampler v1.3.0 ' go.sum +cd .. # allow go mod download without go.mod env GO111MODULE=auto @@ -131,7 +146,7 @@ stderr 'get '$GOPROXY -- go.mod -- module m --- update/go.mod -- +-- update/go.mod.orig -- module m go 1.16 @@ -140,3 +155,17 @@ require ( rsc.io/quote v1.5.2 rsc.io/sampler v1.2.1 // older version than in build list ) +-- update/go.mod.update -- +module m + +go 1.16 + +require ( + rsc.io/quote v1.5.2 + rsc.io/sampler v1.3.0 // older version than in build list +) +-- update/go.sum.update -- +golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0= +rsc.io/sampler v1.2.1/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= +rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/src/cmd/go/testdata/script/mod_get_trailing_slash.txt b/src/cmd/go/testdata/script/mod_get_trailing_slash.txt index 3b38d8ba7d..c536693537 100644 --- a/src/cmd/go/testdata/script/mod_get_trailing_slash.txt +++ b/src/cmd/go/testdata/script/mod_get_trailing_slash.txt @@ -1,6 +1,3 @@ -# Populate go.sum -go mod download - # go list should succeed to load a package ending with ".go" if the path does # not correspond to an existing local file. Listing a pattern ending with # ".go/" should try to list a package regardless of whether a file exists at the @@ -31,3 +28,10 @@ module m go 1.13 require example.com/dotgo.go v1.0.0 +-- go.sum -- +example.com/dotgo.go v1.0.0 h1:XKJfs0V8x2PvY2tX8bJBCEbCDLnt15ma2onwhVpew/I= +example.com/dotgo.go v1.0.0/go.mod h1:Qi6z/X3AC5vHiuMt6HF2ICx3KhIBGrMdrA7YoPDKqR0= +-- use.go -- +package use + +import _ "example.com/dotgo.go" diff --git a/src/cmd/go/testdata/script/mod_query.txt b/src/cmd/go/testdata/script/mod_query.txt index e10185709d..a75f86ed7c 100644 --- a/src/cmd/go/testdata/script/mod_query.txt +++ b/src/cmd/go/testdata/script/mod_query.txt @@ -1,9 +1,7 @@ env GO111MODULE=on -# Populate go.sum. # TODO(golang.org/issue/41297): we shouldn't need go.sum. None of the commands # below depend on the build list. -go mod download go list -m -versions rsc.io/quote stdout '^rsc.io/quote v1.0.0 v1.1.0 v1.2.0 v1.2.1 v1.3.0 v1.4.0 v1.5.0 v1.5.1 v1.5.2 v1.5.3-pre1$' @@ -36,6 +34,9 @@ stdout 'no matching versions for query ">v1.5.3"' module x require rsc.io/quote v1.0.0 +-- go.sum -- +rsc.io/quote v1.0.0 h1:kQ3IZQzPTiDJxSZI98YaWgxFEhlNdYASHvh+MplbViw= +rsc.io/quote v1.0.0/go.mod h1:v83Ri/njykPcgJltBc/gEkJTmjTsNgtO1Y7vyIK1CQA= -- use.go -- package use diff --git a/src/cmd/go/testdata/script/mod_retract.txt b/src/cmd/go/testdata/script/mod_retract.txt index a52e05bc72..4f95ece8d7 100644 --- a/src/cmd/go/testdata/script/mod_retract.txt +++ b/src/cmd/go/testdata/script/mod_retract.txt @@ -1,8 +1,5 @@ cp go.mod go.mod.orig -# Populate go.sum. -go mod download - # 'go list pkg' does not report an error when a retracted version is used. go list -e -f '{{if .Error}}{{.Error}}{{end}}' ./use ! stdout . @@ -32,6 +29,11 @@ go 1.15 require example.com/retract v1.0.0-bad +-- go.sum -- +example.com/retract v1.0.0-bad h1:liAW69rbtjY67x2CcNzat668L/w+YGgNX3lhJsWIJis= +example.com/retract v1.0.0-bad/go.mod h1:0DvGGofJ9hr1q63cBrOY/jSY52OwhRGA0K47NE80I5Y= +example.com/retract/self/prev v1.1.0 h1:0/8I/GTG+1eJTFeDQ/fUbgrMsVHHyKhh3Z8DSZp1fuA= +example.com/retract/self/prev v1.1.0/go.mod h1:xl2EcklWuZZHVtHWcpzfSJQmnzAGpKZYpA/Wto7SZN4= -- use/use.go -- package use From e4d7525c3e119de30490550fe2516fd6958eac30 Mon Sep 17 00:00:00 2001 From: Constantin Konstantinidis Date: Fri, 14 May 2021 15:00:46 +0200 Subject: [PATCH 23/40] cmd/dist: display first class port status in json output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #38874 Change-Id: I819dd008fd6869d335888b4aa03dcf739da9a9a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/320069 Run-TryBot: Dmitri Shuralyov TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov Trust: Daniel Martí --- src/cmd/dist/build.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go index 00e23ef179..1abb03bcc5 100644 --- a/src/cmd/dist/build.go +++ b/src/cmd/dist/build.go @@ -1607,6 +1607,18 @@ var incomplete = map[string]bool{ "linux/sparc64": true, } +// List of platforms which are first class ports. See golang.org/issue/38874. +var firstClass = map[string]bool{ + "darwin/amd64": true, + "darwin/arm64": true, + "linux/386": true, + "linux/amd64": true, + "linux/arm": true, + "linux/arm64": true, + "windows/386": true, + "windows/amd64": true, +} + func needCC() bool { switch os.Getenv("CGO_ENABLED") { case "1": @@ -1743,6 +1755,7 @@ func cmdlist() { GOOS string GOARCH string CgoSupported bool + FirstClass bool } var results []jsonResult for _, p := range plats { @@ -1750,7 +1763,8 @@ func cmdlist() { results = append(results, jsonResult{ GOOS: fields[0], GOARCH: fields[1], - CgoSupported: cgoEnabled[p]}) + CgoSupported: cgoEnabled[p], + FirstClass: firstClass[p]}) } out, err := json.MarshalIndent(results, "", "\t") if err != nil { From 76b2d6afed1e22556bd6c52e74b546eb8bf9a225 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 21 May 2021 12:50:52 -0700 Subject: [PATCH 24/40] os: document that StartProcess puts files into blocking mode Fixes #43894 Change-Id: I2add7b8a4f6ae69a5ef1c48703fde21a4b74307c Reviewed-on: https://go-review.googlesource.com/c/go/+/321852 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- src/os/exec.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/os/exec.go b/src/os/exec.go index edb773a092..bc75d4dd66 100644 --- a/src/os/exec.go +++ b/src/os/exec.go @@ -54,6 +54,9 @@ type ProcAttr struct { // standard error. An implementation may support additional entries, // depending on the underlying operating system. A nil entry corresponds // to that file being closed when the process starts. + // On Unix systems, StartProcess will change these File values + // to blocking mode, which means that SetDeadline will stop working + // and calling Close will not interrupt a Read or Write. Files []*File // Operating system-specific process creation attributes. From 3c656445f139d8b6def40aa7beffd5da9fceccdb Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 21 May 2021 16:33:42 -0400 Subject: [PATCH 25/40] cmd/go: in TestScript/mod_replace, download an explicit module path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As of CL 318629, 'go mod download' without arguments does not save checksums for module source code. Without a checksum, 'go list' will not report the location of the source code even if it is present, in order to prevent accidental access of mismatched code. Downloading an explicit module here also more clearly expresses the intent of the test (“download this module and see where it is”), and may be somewhat more efficient (since the test doesn't need source code for the other modules in the build list). Updates #45332 Change-Id: Ic589b22478e3ed140b95365bb6729101dd598ccc Reviewed-on: https://go-review.googlesource.com/c/go/+/321956 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills Reviewed-by: Dmitri Shuralyov TryBot-Result: Go Bot --- src/cmd/go/testdata/script/mod_replace.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/go/testdata/script/mod_replace.txt b/src/cmd/go/testdata/script/mod_replace.txt index dc9667f1d0..a0a367fb1d 100644 --- a/src/cmd/go/testdata/script/mod_replace.txt +++ b/src/cmd/go/testdata/script/mod_replace.txt @@ -48,7 +48,7 @@ stderr 'module rsc.io/quote/v3@upgrade found \(v3.0.0, replaced by ./local/rsc.i # The reported Dir and GoMod for a replaced module should be accurate. cp go.mod.orig go.mod go mod edit -replace=rsc.io/quote/v3=not-rsc.io/quote@v0.1.0-nomod -go mod download +go mod download rsc.io/quote/v3 go list -m -f '{{.Path}} {{.Version}} {{.Dir}} {{.GoMod}}{{with .Replace}} => {{.Path}} {{.Version}} {{.Dir}} {{.GoMod}}{{end}}' rsc.io/quote/v3 stdout '^rsc.io/quote/v3 v3.0.0 '$GOPATH'[/\\]pkg[/\\]mod[/\\]not-rsc.io[/\\]quote@v0.1.0-nomod '$GOPATH'[/\\]pkg[/\\]mod[/\\]cache[/\\]download[/\\]not-rsc.io[/\\]quote[/\\]@v[/\\]v0.1.0-nomod.mod => not-rsc.io/quote v0.1.0-nomod '$GOPATH'[/\\]pkg[/\\]mod[/\\]not-rsc.io[/\\]quote@v0.1.0-nomod '$GOPATH'[/\\]pkg[/\\]mod[/\\]cache[/\\]download[/\\]not-rsc.io[/\\]quote[/\\]@v[/\\]v0.1.0-nomod.mod$' From 217f5dd496e14f0e0617c9efe0509073dec95d61 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 12 May 2021 13:55:59 -0700 Subject: [PATCH 26/40] doc: document additional atomic.Value methods For #44513. Fixes #46022. Change-Id: Id1d87fbd4034461953760ce77201f87ed723ff88 Reviewed-on: https://go-review.googlesource.com/c/go/+/319549 Trust: Keith Randall Reviewed-by: Ian Lance Taylor --- doc/go1.17.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index f00c649e04..5ab99c29ed 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -507,7 +507,9 @@ Do not send CLs removing the interior tags from such phrases.
sync/atomic

- TODO: https://golang.org/cl/241678: add (*Value).Swap and (*Value).CompareAndSwap + atomic.Value now has Swap and + CompareAndSwap methods that provide + additional atomic operations.

From f87194cbd7e79eef07c897a6240fbf2dc115f9ff Mon Sep 17 00:00:00 2001 From: ian woolf Date: Fri, 7 May 2021 16:31:00 +0800 Subject: [PATCH 27/40] doc/go1.17: document changes to net/http package Changes include: * ReadRequest function now returns an error when a request has multiple Host headers. For #44513. Updates #46015. Change-Id: I48ea7c5cee3f1d1a247035fd37191362a53d1f04 Reviewed-on: https://go-review.googlesource.com/c/go/+/317914 Reviewed-by: Dmitri Shuralyov Trust: Heschi Kreinick --- doc/go1.17.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 5ab99c29ed..46ee1da6fa 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -423,7 +423,8 @@ Do not send CLs removing the interior tags from such phrases.

- TODO: https://golang.org/cl/308952: make ReadRequest return an error when requests have multiple Host headers + The ReadRequest function + now returns an error when the request has multiple Host headers.

From cca23a73733ff166722c69359f0bb45e12ccaa2b Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 21 May 2021 10:53:10 -0400 Subject: [PATCH 28/40] cmd/compile: revert CL/316890 This is a revert of https://go-review.googlesource.com/c/go/+/316890, which has positive effects on debugging + DWARF variable locations for register parameters when the reg abi is in effect, but also turns out to interact badly with the register allocator. Fixes #46304. Change-Id: I624bd980493411a9cde45d44fcd3c46cad796909 Reviewed-on: https://go-review.googlesource.com/c/go/+/321830 Trust: Than McIntosh Run-TryBot: Than McIntosh Reviewed-by: Cherry Mui TryBot-Result: Go Bot --- src/cmd/compile/internal/ssa/expand_calls.go | 16 ----- test/fixedbugs/issue46304.go | 76 ++++++++++++++++++++ 2 files changed, 76 insertions(+), 16 deletions(-) create mode 100644 test/fixedbugs/issue46304.go diff --git a/src/cmd/compile/internal/ssa/expand_calls.go b/src/cmd/compile/internal/ssa/expand_calls.go index d37d06f8e7..7e973ab205 100644 --- a/src/cmd/compile/internal/ssa/expand_calls.go +++ b/src/cmd/compile/internal/ssa/expand_calls.go @@ -1717,22 +1717,6 @@ func (x *expandState) newArgToMemOrRegs(baseArg, toReplace *Value, offset int64, } else { w = baseArg.Block.NewValue0IA(pos, op, t, auxInt, aux) } - // If we are creating an OpArgIntReg/OpArgFloatReg that - // corresponds to an in-param that fits entirely in a register, - // then enter it into the name/value table. The LocalSlot - // is somewhat fictitious, since there is no incoming live - // memory version of the parameter, but we need an entry in - // NamedValues in order for ssa debug tracking to include - // the value in the tracking analysis. - if len(pa.Registers) == 1 { - loc := LocalSlot{N: aux.Name, Type: t, Off: 0} - values, ok := x.f.NamedValues[loc] - if !ok { - ploc := x.f.localSlotAddr(loc) - x.f.Names = append(x.f.Names, ploc) - } - x.f.NamedValues[loc] = append(values, w) - } x.commonArgs[key] = w if toReplace != nil { toReplace.copyOf(w) diff --git a/test/fixedbugs/issue46304.go b/test/fixedbugs/issue46304.go new file mode 100644 index 0000000000..b8ecfc93a5 --- /dev/null +++ b/test/fixedbugs/issue46304.go @@ -0,0 +1,76 @@ +// run + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This testcase caused a crash when the register ABI was in effect, +// on amd64 (problem with register allocation). + +package main + +type Op struct { + tag string + _x []string + _q [20]uint64 + plist []P +} + +type P struct { + tag string + _x [10]uint64 + b bool +} + +type M int + +//go:noinline +func (w *M) walkP(p *P) *P { + np := &P{} + *np = *p + np.tag += "new" + return np +} + +func (w *M) walkOp(op *Op) *Op { + if op == nil { + return nil + } + + orig := op + cloned := false + clone := func() { + if !cloned { + cloned = true + op = &Op{} + *op = *orig + } + } + + pCloned := false + for i := range op.plist { + if s := w.walkP(&op.plist[i]); s != &op.plist[i] { + if !pCloned { + pCloned = true + clone() + op.plist = make([]P, len(orig.plist)) + copy(op.plist, orig.plist) + } + op.plist[i] = *s + } + } + + return op +} + +func main() { + var ww M + w := &ww + p1 := P{tag: "a"} + p1._x[1] = 9 + o := Op{tag: "old", plist: []P{p1}} + no := w.walkOp(&o) + if no.plist[0].tag != "anew" { + panic("bad") + } +} From 05819bc104c3021d20ad21aa685fb6b4db35ceb0 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 14 May 2021 23:14:22 -0400 Subject: [PATCH 29/40] cmd/go/internal/modcmd: factor out a type for flags whose arguments are Go versions For #46141 Updates #45094 Change-Id: I6553600c69273762a81795ef021c66f4e0872b6b Reviewed-on: https://go-review.googlesource.com/c/go/+/321069 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- src/cmd/go/internal/modcmd/edit.go | 2 +- src/cmd/go/internal/modcmd/tidy.go | 42 ++++++++++++++----- src/cmd/go/internal/modload/init.go | 10 ++--- src/cmd/go/internal/modload/load.go | 4 +- src/cmd/go/internal/modload/modfile.go | 2 +- .../go/testdata/script/mod_tidy_version.txt | 12 +++++- 6 files changed, 52 insertions(+), 20 deletions(-) diff --git a/src/cmd/go/internal/modcmd/edit.go b/src/cmd/go/internal/modcmd/edit.go index 79a93ca44b..e856e7c630 100644 --- a/src/cmd/go/internal/modcmd/edit.go +++ b/src/cmd/go/internal/modcmd/edit.go @@ -196,7 +196,7 @@ func runEdit(ctx context.Context, cmd *base.Command, args []string) { if *editGo != "" { if !modfile.GoVersionRE.MatchString(*editGo) { - base.Fatalf(`go mod: invalid -go option; expecting something like "-go 1.12"`) + base.Fatalf(`go mod: invalid -go option; expecting something like "-go %s"`, modload.LatestGoVersion()) } } diff --git a/src/cmd/go/internal/modcmd/tidy.go b/src/cmd/go/internal/modcmd/tidy.go index c72ec30a57..9af624028a 100644 --- a/src/cmd/go/internal/modcmd/tidy.go +++ b/src/cmd/go/internal/modcmd/tidy.go @@ -12,8 +12,10 @@ import ( "cmd/go/internal/imports" "cmd/go/internal/modload" "context" + "fmt" "golang.org/x/mod/modfile" + "golang.org/x/mod/semver" ) var cmdTidy = &base.Command{ @@ -44,28 +46,48 @@ See https://golang.org/ref/mod#go-mod-tidy for more about 'go mod tidy'. } var ( - tidyE bool // if true, report errors but proceed anyway. - tidyGo string // go version to write to the tidied go.mod file (toggles lazy loading) + tidyE bool // if true, report errors but proceed anyway. + tidyGo goVersionFlag // go version to write to the tidied go.mod file (toggles lazy loading) ) func init() { cmdTidy.Flag.BoolVar(&cfg.BuildV, "v", false, "") cmdTidy.Flag.BoolVar(&tidyE, "e", false, "") - cmdTidy.Flag.StringVar(&tidyGo, "go", "", "") + cmdTidy.Flag.Var(&tidyGo, "go", "") base.AddModCommonFlags(&cmdTidy.Flag) } +// A goVersionFlag is a flag.Value representing a supported Go version. +// +// (Note that the -go argument to 'go mod edit' is *not* a goVersionFlag. +// It intentionally allows newer-than-supported versions as arguments.) +type goVersionFlag struct { + v string +} + +func (f *goVersionFlag) String() string { return f.v } +func (f *goVersionFlag) Get() interface{} { return f.v } + +func (f *goVersionFlag) Set(s string) error { + if s != "" { + latest := modload.LatestGoVersion() + if !modfile.GoVersionRE.MatchString(s) { + return fmt.Errorf("expecting a Go version like %q", latest) + } + if semver.Compare("v"+s, "v"+latest) > 0 { + return fmt.Errorf("maximum supported Go version is %s", latest) + } + } + + f.v = s + return nil +} + func runTidy(ctx context.Context, cmd *base.Command, args []string) { if len(args) > 0 { base.Fatalf("go mod tidy: no arguments allowed") } - if tidyGo != "" { - if !modfile.GoVersionRE.MatchString(tidyGo) { - base.Fatalf(`go mod: invalid -go option %q; expecting something like "-go 1.17"`, tidyGo) - } - } - // Tidy aims to make 'go test' reproducible for any package in 'all', so we // need to include test dependencies. For modules that specify go 1.15 or // earlier this is a no-op (because 'all' saturates transitive test @@ -80,7 +102,7 @@ func runTidy(ctx context.Context, cmd *base.Command, args []string) { modload.RootMode = modload.NeedRoot modload.LoadPackages(ctx, modload.PackageOpts{ - GoVersion: tidyGo, + GoVersion: tidyGo.String(), Tags: imports.AnyTags(), Tidy: true, VendorModulesInGOROOTSrc: true, diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index 5cdea12cd3..e358230e74 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -405,7 +405,7 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) { if modRoot == "" { Target = module.Version{Path: "command-line-arguments"} targetPrefix = "command-line-arguments" - goVersion := latestGoVersion() + goVersion := LatestGoVersion() rawGoVersion.Store(Target, goVersion) requirements = newRequirements(modDepthFromGoVersion(goVersion), nil, nil) return requirements, false @@ -448,7 +448,7 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) { // TODO(#45551): Do something more principled instead of checking // cfg.CmdName directly here. if cfg.BuildMod == "mod" && cfg.CmdName != "mod graph" && cfg.CmdName != "mod why" { - addGoStmt(latestGoVersion()) + addGoStmt(LatestGoVersion()) if go117EnableLazyLoading { // We need to add a 'go' version to the go.mod file, but we must assume // that its existing contents match something between Go 1.11 and 1.16. @@ -500,7 +500,7 @@ func CreateModFile(ctx context.Context, modPath string) { modFile = new(modfile.File) modFile.AddModuleStmt(modPath) initTarget(modFile.Module.Mod) - addGoStmt(latestGoVersion()) // Add the go directive before converted module requirements. + addGoStmt(LatestGoVersion()) // Add the go directive before converted module requirements. convertedFrom, err := convertLegacyConfig(modPath) if convertedFrom != "" { @@ -793,9 +793,9 @@ func addGoStmt(v string) { rawGoVersion.Store(Target, v) } -// latestGoVersion returns the latest version of the Go language supported by +// LatestGoVersion returns the latest version of the Go language supported by // this toolchain, like "1.17". -func latestGoVersion() string { +func LatestGoVersion() string { tags := build.Default.ReleaseTags version := tags[len(tags)-1] if !strings.HasPrefix(version, "go") || !modfile.GoVersionRE.MatchString(version[2:]) { diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 83fc7c09c3..23ee3824f3 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -931,8 +931,8 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader { ld.allClosesOverTests = true } - if ld.Tidy && semver.Compare(goVersionV, "v"+latestGoVersion()) > 0 { - ld.errorf("go mod tidy: go.mod file indicates go %s, but maximum supported version is %s\n", params.GoVersion, latestGoVersion()) + if ld.Tidy && semver.Compare(goVersionV, "v"+LatestGoVersion()) > 0 { + ld.errorf("go mod tidy: go.mod file indicates go %s, but maximum supported version is %s\n", params.GoVersion, LatestGoVersion()) base.ExitIfErrors() } diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go index bafff3e080..a9c3a91d35 100644 --- a/src/cmd/go/internal/modload/modfile.go +++ b/src/cmd/go/internal/modload/modfile.go @@ -55,7 +55,7 @@ var modFile *modfile.File // in modFile are intepreted, or the latest Go version if modFile is nil. func modFileGoVersion() string { if modFile == nil { - return latestGoVersion() + return LatestGoVersion() } if modFile.Go == nil || modFile.Go.Version == "" { // The main module necessarily has a go.mod file, and that file lacks a diff --git a/src/cmd/go/testdata/script/mod_tidy_version.txt b/src/cmd/go/testdata/script/mod_tidy_version.txt index 5441d9cc06..eaa6ee7b0d 100644 --- a/src/cmd/go/testdata/script/mod_tidy_version.txt +++ b/src/cmd/go/testdata/script/mod_tidy_version.txt @@ -32,12 +32,22 @@ cp go.mod go.mod.orig + # An invalid argument should be rejected. ! go mod tidy -go=bananas -stderr '^go mod: invalid -go option "bananas"; expecting something like "-go 1.17"$' +stderr '^invalid value "bananas" for flag -go: expecting a Go version like "'$goversion'"$' cmp go.mod go.mod.orig +! go mod tidy -go=0.9 +stderr '^invalid value "0.9" for flag -go: expecting a Go version like "'$goversion'"$' + +! go mod tidy -go=2000.0 +stderr '^invalid value "2000.0" for flag -go: maximum supported Go version is '$goversion'$' + + +# Supported versions should change the go.mod file to be tidy according to the +# indicated version. go mod tidy -go=1.15 cmp go.mod go.mod.115 From 52d7033ff6d56094b7fa852bbdf51b4525bd6bb2 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 14 May 2021 16:51:57 -0400 Subject: [PATCH 30/40] cmd/go/internal/modload: set the default GoVersion in a single location For #46141 Updates #36460 Change-Id: Ie4c13c73a451650d1e8abb8e5cebfc30d0a71a70 Reviewed-on: https://go-review.googlesource.com/c/go/+/321070 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- src/cmd/go/internal/modload/load.go | 41 ++++++++++++----------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 23ee3824f3..37b0032d43 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -314,10 +314,6 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma initialRS, _ := loadModFile(ctx) // Ignore needCommit — we're going to commit at the end regardless. - if opts.GoVersion == "" { - opts.GoVersion = modFileGoVersion() - } - ld := loadFromRoots(ctx, loaderParams{ PackageOpts: opts, requirements: initialRS, @@ -380,7 +376,7 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma // Success! Update go.mod and go.sum (if needed) and return the results. loaded = ld - commitRequirements(ctx, opts.GoVersion, loaded.requirements) + commitRequirements(ctx, loaded.GoVersion, loaded.requirements) for _, pkg := range ld.pkgs { if !pkg.isTest() { @@ -605,10 +601,8 @@ func ImportFromFiles(ctx context.Context, gofiles []string) { base.Fatalf("go: %v", err) } - goVersion := modFileGoVersion() loaded = loadFromRoots(ctx, loaderParams{ PackageOpts: PackageOpts{ - GoVersion: goVersion, Tags: tags, ResolveMissingImports: true, SilencePackageErrors: true, @@ -620,7 +614,7 @@ func ImportFromFiles(ctx context.Context, gofiles []string) { return roots }, }) - commitRequirements(ctx, goVersion, loaded.requirements) + commitRequirements(ctx, loaded.GoVersion, loaded.requirements) } // DirImportPath returns the effective import path for dir, @@ -921,26 +915,25 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader { work: par.NewQueue(runtime.GOMAXPROCS(0)), } - if params.GoVersion != "" { - goVersionV := "v" + params.GoVersion - if semver.Compare(goVersionV, narrowAllVersionV) < 0 && !ld.UseVendorAll { - // The module's go version explicitly predates the change in "all" for lazy - // loading, so continue to use the older interpretation. - // (If params.GoVersion is empty, we are probably not in any module at all - // and should use the latest semantics.) - ld.allClosesOverTests = true - } + if ld.GoVersion == "" { + ld.GoVersion = modFileGoVersion() - if ld.Tidy && semver.Compare(goVersionV, "v"+LatestGoVersion()) > 0 { - ld.errorf("go mod tidy: go.mod file indicates go %s, but maximum supported version is %s\n", params.GoVersion, LatestGoVersion()) + if ld.Tidy && semver.Compare("v"+ld.GoVersion, "v"+LatestGoVersion()) > 0 { + ld.errorf("go mod tidy: go.mod file indicates go %s, but maximum supported version is %s\n", ld.GoVersion, LatestGoVersion()) base.ExitIfErrors() } + } - var err error - ld.requirements, err = convertDepth(ctx, ld.requirements, modDepthFromGoVersion(params.GoVersion)) - if err != nil { - ld.errorf("go: %v\n", err) - } + if semver.Compare("v"+ld.GoVersion, narrowAllVersionV) < 0 && !ld.UseVendorAll { + // The module's go version explicitly predates the change in "all" for lazy + // loading, so continue to use the older interpretation. + ld.allClosesOverTests = true + } + + var err error + ld.requirements, err = convertDepth(ctx, ld.requirements, modDepthFromGoVersion(ld.GoVersion)) + if err != nil { + ld.errorf("go: %v\n", err) } if ld.requirements.depth == eager { From 4356e7e85fcd8f59de6bc1fd1db6e4f01a92f19e Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 21 May 2021 20:30:02 +0000 Subject: [PATCH 31/40] runtime: account for spill slots in Windows callback compilation The Go ABI, as it stands, requires spill space to be reserved for register arguments. syscall.NewCallback (because of compileCallback) does not actually reserve this space, leading to issues if the Go code it invokes actually makes use of it. Fixes #46301. Change-Id: Idbc3578accaaaa29e4ba32291ef08d464da0b7b0 Reviewed-on: https://go-review.googlesource.com/c/go/+/322029 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Egon Elbre Reviewed-by: Cherry Mui --- src/runtime/syscall_windows.go | 23 ++++++++++++++++++++--- src/runtime/syscall_windows_test.go | 15 +++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/runtime/syscall_windows.go b/src/runtime/syscall_windows.go index 6b9195bcd5..4763a440e7 100644 --- a/src/runtime/syscall_windows.go +++ b/src/runtime/syscall_windows.go @@ -64,6 +64,7 @@ type abiDesc struct { srcStackSize uintptr // stdcall/fastcall stack space tracking dstStackSize uintptr // Go stack space used + dstSpill uintptr // Extra stack space for argument spill slots dstRegisters int // Go ABI int argument registers used // retOffset is the offset of the uintptr-sized result in the Go @@ -110,7 +111,14 @@ func (p *abiDesc) assignArg(t *_type) { // arguments. The same is true on arm. oldParts := p.parts - if !p.tryRegAssignArg(t, 0) { + if p.tryRegAssignArg(t, 0) { + // Account for spill space. + // + // TODO(mknyszek): Remove this when we no longer have + // caller reserved spill space. + p.dstSpill = alignUp(p.dstSpill, uintptr(t.align)) + p.dstSpill += t.size + } else { // Register assignment failed. // Undo the work and stack assign. p.parts = oldParts @@ -277,7 +285,11 @@ func compileCallback(fn eface, cdecl bool) (code uintptr) { abiMap.dstStackSize += sys.PtrSize } - if abiMap.dstStackSize > callbackMaxFrame { + // TODO(mknyszek): Remove dstSpill from this calculation when we no longer have + // caller reserved spill space. + frameSize := alignUp(abiMap.dstStackSize, sys.PtrSize) + frameSize += abiMap.dstSpill + if frameSize > callbackMaxFrame { panic("compileCallback: function argument frame too large") } @@ -356,9 +368,14 @@ func callbackWrap(a *callbackArgs) { } } + // TODO(mknyszek): Remove this when we no longer have + // caller reserved spill space. + frameSize := alignUp(c.abiMap.dstStackSize, sys.PtrSize) + frameSize += c.abiMap.dstSpill + // Even though this is copying back results, we can pass a nil // type because those results must not require write barriers. - reflectcall(nil, unsafe.Pointer(c.fn), noescape(goArgs), uint32(c.abiMap.dstStackSize), uint32(c.abiMap.retOffset), uint32(c.abiMap.dstStackSize), ®s) + reflectcall(nil, unsafe.Pointer(c.fn), noescape(goArgs), uint32(c.abiMap.dstStackSize), uint32(c.abiMap.retOffset), uint32(frameSize), ®s) // Extract the result. // diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go index 5e9694d444..e3f772ac4b 100644 --- a/src/runtime/syscall_windows_test.go +++ b/src/runtime/syscall_windows_test.go @@ -389,6 +389,10 @@ var cbFuncs = []cbFunc{ {func(i1, i2, i3, i4, i5 uint8Pair) uintptr { return uintptr(i1.x + i1.y + i2.x + i2.y + i3.x + i3.y + i4.x + i4.y + i5.x + i5.y) }}, + {func(i1, i2, i3, i4, i5, i6, i7, i8, i9 uint32) uintptr { + runtime.GC() + return uintptr(i1 + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9) + }}, } //go:registerparams @@ -461,6 +465,16 @@ func sum5andPair(i1, i2, i3, i4, i5 uint8Pair) uintptr { return uintptr(i1.x + i1.y + i2.x + i2.y + i3.x + i3.y + i4.x + i4.y + i5.x + i5.y) } +// This test forces a GC. The idea is to have enough arguments +// that insufficient spill slots allocated (according to the ABI) +// may cause compiler-generated spills to clobber the return PC. +// Then, the GC stack scanning will catch that. +//go:registerparams +func sum9andGC(i1, i2, i3, i4, i5, i6, i7, i8, i9 uint32) uintptr { + runtime.GC() + return uintptr(i1 + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9) +} + // TODO(register args): Remove this once we switch to using the register // calling convention by default, since this is redundant with the existing // tests. @@ -479,6 +493,7 @@ var cbFuncsRegABI = []cbFunc{ {sum9int8}, {sum5mix}, {sum5andPair}, + {sum9andGC}, } func getCallbackTestFuncs() []cbFunc { From a22e3172200d4bdd0afcbbe6564dbb67fea4b03a Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Sat, 22 May 2021 21:59:00 -0700 Subject: [PATCH 32/40] cmd/compile: always include underlying type for map types This is a different fix for #37716. Should help make the fix for #46283 easier, since we will no longer need to keep compiler-generated hash functions and the runtime hash function in sync. Change-Id: I84cb93144e425dcd03afc552b5fbd0f2d2cc6d39 Reviewed-on: https://go-review.googlesource.com/c/go/+/322150 Trust: Keith Randall Run-TryBot: Keith Randall TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- .../compile/internal/reflectdata/reflect.go | 9 +++ src/cmd/internal/objabi/reloctype.go | 3 + src/cmd/internal/objabi/reloctype_string.go | 71 ++++++++++--------- src/runtime/alg.go | 19 +---- src/runtime/export_test.go | 25 ------- src/runtime/hash_test.go | 49 ------------- 6 files changed, 49 insertions(+), 127 deletions(-) diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go index 8c0e33f6df..b3688fca67 100644 --- a/src/cmd/compile/internal/reflectdata/reflect.go +++ b/src/cmd/compile/internal/reflectdata/reflect.go @@ -1112,6 +1112,15 @@ func writeType(t *types.Type) *obj.LSym { } ot = objw.Uint32(lsym, ot, flags) ot = dextratype(lsym, ot, t, 0) + if u := t.Underlying(); u != t { + // If t is a named map type, also keep the underlying map + // type live in the binary. This is important to make sure that + // a named map and that same map cast to its underlying type via + // reflection, use the same hash function. See issue 37716. + r := obj.Addrel(lsym) + r.Sym = writeType(u) + r.Type = objabi.R_KEEP + } case types.TPTR: if t.Elem().Kind() == types.TANY { diff --git a/src/cmd/internal/objabi/reloctype.go b/src/cmd/internal/objabi/reloctype.go index ea55fa3b0a..52827a6dee 100644 --- a/src/cmd/internal/objabi/reloctype.go +++ b/src/cmd/internal/objabi/reloctype.go @@ -101,6 +101,9 @@ const ( // *rtype, and may be set to zero by the linker if it determines the method // text is unreachable by the linked program. R_METHODOFF + // R_KEEP tells the linker to keep the referred-to symbol in the final binary + // if the symbol containing the R_KEEP relocation is in the final binary. + R_KEEP R_POWER_TOC R_GOTPCREL // R_JMPMIPS (only used on mips64) resolves to non-PC-relative target address diff --git a/src/cmd/internal/objabi/reloctype_string.go b/src/cmd/internal/objabi/reloctype_string.go index 8882d19f88..4638ef14d9 100644 --- a/src/cmd/internal/objabi/reloctype_string.go +++ b/src/cmd/internal/objabi/reloctype_string.go @@ -34,44 +34,45 @@ func _() { _ = x[R_USEIFACE-24] _ = x[R_USEIFACEMETHOD-25] _ = x[R_METHODOFF-26] - _ = x[R_POWER_TOC-27] - _ = x[R_GOTPCREL-28] - _ = x[R_JMPMIPS-29] - _ = x[R_DWARFSECREF-30] - _ = x[R_DWARFFILEREF-31] - _ = x[R_ARM64_TLS_LE-32] - _ = x[R_ARM64_TLS_IE-33] - _ = x[R_ARM64_GOTPCREL-34] - _ = x[R_ARM64_GOT-35] - _ = x[R_ARM64_PCREL-36] - _ = x[R_ARM64_LDST8-37] - _ = x[R_ARM64_LDST16-38] - _ = x[R_ARM64_LDST32-39] - _ = x[R_ARM64_LDST64-40] - _ = x[R_ARM64_LDST128-41] - _ = x[R_POWER_TLS_LE-42] - _ = x[R_POWER_TLS_IE-43] - _ = x[R_POWER_TLS-44] - _ = x[R_ADDRPOWER_DS-45] - _ = x[R_ADDRPOWER_GOT-46] - _ = x[R_ADDRPOWER_PCREL-47] - _ = x[R_ADDRPOWER_TOCREL-48] - _ = x[R_ADDRPOWER_TOCREL_DS-49] - _ = x[R_RISCV_PCREL_ITYPE-50] - _ = x[R_RISCV_PCREL_STYPE-51] - _ = x[R_RISCV_TLS_IE_ITYPE-52] - _ = x[R_RISCV_TLS_IE_STYPE-53] - _ = x[R_PCRELDBL-54] - _ = x[R_ADDRMIPSU-55] - _ = x[R_ADDRMIPSTLS-56] - _ = x[R_ADDRCUOFF-57] - _ = x[R_WASMIMPORT-58] - _ = x[R_XCOFFREF-59] + _ = x[R_KEEP-27] + _ = x[R_POWER_TOC-28] + _ = x[R_GOTPCREL-29] + _ = x[R_JMPMIPS-30] + _ = x[R_DWARFSECREF-31] + _ = x[R_DWARFFILEREF-32] + _ = x[R_ARM64_TLS_LE-33] + _ = x[R_ARM64_TLS_IE-34] + _ = x[R_ARM64_GOTPCREL-35] + _ = x[R_ARM64_GOT-36] + _ = x[R_ARM64_PCREL-37] + _ = x[R_ARM64_LDST8-38] + _ = x[R_ARM64_LDST16-39] + _ = x[R_ARM64_LDST32-40] + _ = x[R_ARM64_LDST64-41] + _ = x[R_ARM64_LDST128-42] + _ = x[R_POWER_TLS_LE-43] + _ = x[R_POWER_TLS_IE-44] + _ = x[R_POWER_TLS-45] + _ = x[R_ADDRPOWER_DS-46] + _ = x[R_ADDRPOWER_GOT-47] + _ = x[R_ADDRPOWER_PCREL-48] + _ = x[R_ADDRPOWER_TOCREL-49] + _ = x[R_ADDRPOWER_TOCREL_DS-50] + _ = x[R_RISCV_PCREL_ITYPE-51] + _ = x[R_RISCV_PCREL_STYPE-52] + _ = x[R_RISCV_TLS_IE_ITYPE-53] + _ = x[R_RISCV_TLS_IE_STYPE-54] + _ = x[R_PCRELDBL-55] + _ = x[R_ADDRMIPSU-56] + _ = x[R_ADDRMIPSTLS-57] + _ = x[R_ADDRCUOFF-58] + _ = x[R_WASMIMPORT-59] + _ = x[R_XCOFFREF-60] } -const _RelocType_name = "R_ADDRR_ADDRPOWERR_ADDRARM64R_ADDRMIPSR_ADDROFFR_SIZER_CALLR_CALLARMR_CALLARM64R_CALLINDR_CALLPOWERR_CALLMIPSR_CALLRISCVR_CONSTR_PCRELR_TLS_LER_TLS_IER_GOTOFFR_PLT0R_PLT1R_PLT2R_USEFIELDR_USETYPER_USEIFACER_USEIFACEMETHODR_METHODOFFR_POWER_TOCR_GOTPCRELR_JMPMIPSR_DWARFSECREFR_DWARFFILEREFR_ARM64_TLS_LER_ARM64_TLS_IER_ARM64_GOTPCRELR_ARM64_GOTR_ARM64_PCRELR_ARM64_LDST8R_ARM64_LDST16R_ARM64_LDST32R_ARM64_LDST64R_ARM64_LDST128R_POWER_TLS_LER_POWER_TLS_IER_POWER_TLSR_ADDRPOWER_DSR_ADDRPOWER_GOTR_ADDRPOWER_PCRELR_ADDRPOWER_TOCRELR_ADDRPOWER_TOCREL_DSR_RISCV_PCREL_ITYPER_RISCV_PCREL_STYPER_RISCV_TLS_IE_ITYPER_RISCV_TLS_IE_STYPER_PCRELDBLR_ADDRMIPSUR_ADDRMIPSTLSR_ADDRCUOFFR_WASMIMPORTR_XCOFFREF" +const _RelocType_name = "R_ADDRR_ADDRPOWERR_ADDRARM64R_ADDRMIPSR_ADDROFFR_SIZER_CALLR_CALLARMR_CALLARM64R_CALLINDR_CALLPOWERR_CALLMIPSR_CALLRISCVR_CONSTR_PCRELR_TLS_LER_TLS_IER_GOTOFFR_PLT0R_PLT1R_PLT2R_USEFIELDR_USETYPER_USEIFACER_USEIFACEMETHODR_METHODOFFR_KEEPR_POWER_TOCR_GOTPCRELR_JMPMIPSR_DWARFSECREFR_DWARFFILEREFR_ARM64_TLS_LER_ARM64_TLS_IER_ARM64_GOTPCRELR_ARM64_GOTR_ARM64_PCRELR_ARM64_LDST8R_ARM64_LDST16R_ARM64_LDST32R_ARM64_LDST64R_ARM64_LDST128R_POWER_TLS_LER_POWER_TLS_IER_POWER_TLSR_ADDRPOWER_DSR_ADDRPOWER_GOTR_ADDRPOWER_PCRELR_ADDRPOWER_TOCRELR_ADDRPOWER_TOCREL_DSR_RISCV_PCREL_ITYPER_RISCV_PCREL_STYPER_RISCV_TLS_IE_ITYPER_RISCV_TLS_IE_STYPER_PCRELDBLR_ADDRMIPSUR_ADDRMIPSTLSR_ADDRCUOFFR_WASMIMPORTR_XCOFFREF" -var _RelocType_index = [...]uint16{0, 6, 17, 28, 38, 47, 53, 59, 68, 79, 88, 99, 109, 120, 127, 134, 142, 150, 158, 164, 170, 176, 186, 195, 205, 221, 232, 243, 253, 262, 275, 289, 303, 317, 333, 344, 357, 370, 384, 398, 412, 427, 441, 455, 466, 480, 495, 512, 530, 551, 570, 589, 609, 629, 639, 650, 663, 674, 686, 696} +var _RelocType_index = [...]uint16{0, 6, 17, 28, 38, 47, 53, 59, 68, 79, 88, 99, 109, 120, 127, 134, 142, 150, 158, 164, 170, 176, 186, 195, 205, 221, 232, 238, 249, 259, 268, 281, 295, 309, 323, 339, 350, 363, 376, 390, 404, 418, 433, 447, 461, 472, 486, 501, 518, 536, 557, 576, 595, 615, 635, 645, 656, 669, 680, 692, 702} func (i RelocType) String() string { i -= 1 diff --git a/src/runtime/alg.go b/src/runtime/alg.go index 1b3bf1180d..39c7426842 100644 --- a/src/runtime/alg.go +++ b/src/runtime/alg.go @@ -178,28 +178,11 @@ func typehash(t *_type, p unsafe.Pointer, h uintptr) uintptr { return h case kindStruct: s := (*structtype)(unsafe.Pointer(t)) - memStart := uintptr(0) - memEnd := uintptr(0) for _, f := range s.fields { - if memEnd > memStart && (f.name.isBlank() || f.offset() != memEnd || f.typ.tflag&tflagRegularMemory == 0) { - // flush any pending regular memory hashing - h = memhash(add(p, memStart), h, memEnd-memStart) - memStart = memEnd - } if f.name.isBlank() { continue } - if f.typ.tflag&tflagRegularMemory == 0 { - h = typehash(f.typ, add(p, f.offset()), h) - continue - } - if memStart == memEnd { - memStart = f.offset() - } - memEnd = f.offset() + f.typ.size - } - if memEnd > memStart { - h = memhash(add(p, memStart), h, memEnd-memStart) + h = typehash(f.typ, add(p, f.offset()), h) } return h default: diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index a6fc1e4785..c8d01fbb15 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -1148,31 +1148,6 @@ func SemNwait(addr *uint32) uint32 { return atomic.Load(&root.nwait) } -// MapHashCheck computes the hash of the key k for the map m, twice. -// Method 1 uses the built-in hasher for the map. -// Method 2 uses the typehash function (the one used by reflect). -// Returns the two hash values, which should always be equal. -func MapHashCheck(m interface{}, k interface{}) (uintptr, uintptr) { - // Unpack m. - mt := (*maptype)(unsafe.Pointer(efaceOf(&m)._type)) - mh := (*hmap)(efaceOf(&m).data) - - // Unpack k. - kt := efaceOf(&k)._type - var p unsafe.Pointer - if isDirectIface(kt) { - q := efaceOf(&k).data - p = unsafe.Pointer(&q) - } else { - p = efaceOf(&k).data - } - - // Compute the hash functions. - x := mt.hasher(noescape(p), uintptr(mh.hash0)) - y := typehash(kt, noescape(p), uintptr(mh.hash0)) - return x, y -} - // mspan wrapper for testing. //go:notinheap type MSpan mspan diff --git a/src/runtime/hash_test.go b/src/runtime/hash_test.go index 502383557b..7048874a71 100644 --- a/src/runtime/hash_test.go +++ b/src/runtime/hash_test.go @@ -8,7 +8,6 @@ import ( "fmt" "math" "math/rand" - "reflect" . "runtime" "strings" "testing" @@ -49,54 +48,6 @@ func TestMemHash64Equality(t *testing.T) { } } -func TestCompilerVsRuntimeHash(t *testing.T) { - // Test to make sure the compiler's hash function and the runtime's hash function agree. - // See issue 37716. - for _, m := range []interface{}{ - map[bool]int{}, - map[int8]int{}, - map[uint8]int{}, - map[int16]int{}, - map[uint16]int{}, - map[int32]int{}, - map[uint32]int{}, - map[int64]int{}, - map[uint64]int{}, - map[int]int{}, - map[uint]int{}, - map[uintptr]int{}, - map[*byte]int{}, - map[chan int]int{}, - map[unsafe.Pointer]int{}, - map[float32]int{}, - map[float64]int{}, - map[complex64]int{}, - map[complex128]int{}, - map[string]int{}, - //map[interface{}]int{}, - //map[interface{F()}]int{}, - map[[8]uint64]int{}, - map[[8]string]int{}, - map[struct{ a, b, c, d int32 }]int{}, // Note: tests AMEM128 - map[struct{ a, b, _, d int32 }]int{}, - map[struct { - a, b int32 - c float32 - d, e [8]byte - }]int{}, - map[struct { - a int16 - b int64 - }]int{}, - } { - k := reflect.New(reflect.TypeOf(m).Key()).Elem().Interface() // the zero key - x, y := MapHashCheck(m, k) - if x != y { - t.Errorf("hashes did not match (%x vs %x) for map %T", x, y, m) - } - } -} - // Smhasher is a torture test for hash functions. // https://code.google.com/p/smhasher/ // This code is a port of some of the Smhasher tests to Go. From b83610699a4ea7da22a146c0eefe0ae4d5ac4610 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Sun, 9 May 2021 09:05:45 +0200 Subject: [PATCH 33/40] cmd/compile: record regabi status in DW_AT_producer Records if regabi was enabled during compilation in the DW_AT_producer attribute of each compile unit. This is useful to debuggers that support the debugCall protocol. Change-Id: I5ad2c48ebf126aeb8bfb459b53a1a5304550036a Reviewed-on: https://go-review.googlesource.com/c/go/+/318050 Trust: Dmitri Shuralyov Reviewed-by: Than McIntosh Reviewed-by: Cherry Mui Run-TryBot: Than McIntosh TryBot-Result: Go Bot --- src/cmd/compile/internal/dwarfgen/dwarf.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/cmd/compile/internal/dwarfgen/dwarf.go b/src/cmd/compile/internal/dwarfgen/dwarf.go index 5d7dc320aa..0e22b61bc3 100644 --- a/src/cmd/compile/internal/dwarfgen/dwarf.go +++ b/src/cmd/compile/internal/dwarfgen/dwarf.go @@ -531,6 +531,14 @@ func RecordFlags(flags ...string) { fmt.Fprintf(&cmd, " -%s=%v", f.Name, getter.Get()) } + // Adds flag to producer string singalling whether regabi is turned on or + // off. + // Once regabi is turned on across the board and the relative GOEXPERIMENT + // knobs no longer exist this code should be removed. + if buildcfg.Experiment.RegabiArgs { + cmd.Write([]byte(" regabi")) + } + if cmd.Len() == 0 { return } From 873401df5b202a751523b8cbd92bf3a8aaf989c8 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Sun, 23 May 2021 12:38:59 -0700 Subject: [PATCH 34/40] cmd/compile: ensure equal functions don't do unaligned loads On architectures which don't support unaligned loads, make sure we don't generate code that requires them. Generated hash functions also matter in this respect, but they all look ok. Update #37716 Fixes #46283 Change-Id: I6197fdfe04da4428092c99bd871d93738789e16b Reviewed-on: https://go-review.googlesource.com/c/go/+/322151 Trust: Keith Randall Trust: Josh Bleecher Snyder Run-TryBot: Keith Randall Reviewed-by: Cherry Mui Reviewed-by: Josh Bleecher Snyder Reviewed-by: eric fang TryBot-Result: Go Bot --- src/cmd/compile/internal/reflectdata/alg.go | 20 +++++ src/cmd/compile/internal/test/align_test.go | 96 +++++++++++++++++++++ src/cmd/internal/sys/arch.go | 19 ++++ 3 files changed, 135 insertions(+) create mode 100644 src/cmd/compile/internal/test/align_test.go diff --git a/src/cmd/compile/internal/reflectdata/alg.go b/src/cmd/compile/internal/reflectdata/alg.go index d12d9ca0a7..0707e0b61c 100644 --- a/src/cmd/compile/internal/reflectdata/alg.go +++ b/src/cmd/compile/internal/reflectdata/alg.go @@ -6,6 +6,7 @@ package reflectdata import ( "fmt" + "math/bits" "sort" "cmd/compile/internal/base" @@ -47,6 +48,11 @@ func eqCanPanic(t *types.Type) bool { func AlgType(t *types.Type) types.AlgKind { a, _ := types.AlgType(t) if a == types.AMEM { + if t.Alignment() < int64(base.Ctxt.Arch.Alignment) && t.Alignment() < t.Width { + // For example, we can't treat [2]int16 as an int32 if int32s require + // 4-byte alignment. See issue 46283. + return a + } switch t.Width { case 0: return types.AMEM0 @@ -769,6 +775,20 @@ func memrun(t *types.Type, start int) (size int64, next int) { if f := t.Field(next); f.Sym.IsBlank() || !isRegularMemory(f.Type) { break } + // For issue 46283, don't combine fields if the resulting load would + // require a larger alignment than the component fields. + if base.Ctxt.Arch.Alignment > 1 { + align := t.Alignment() + if off := t.Field(start).Offset; off&(align-1) != 0 { + // Offset is less aligned than the containing type. + // Use offset to determine alignment. + align = 1 << uint(bits.TrailingZeros64(uint64(off))) + } + size := t.Field(next).End() - t.Field(start).Offset + if size > align { + break + } + } } return t.Field(next-1).End() - t.Field(start).Offset, next } diff --git a/src/cmd/compile/internal/test/align_test.go b/src/cmd/compile/internal/test/align_test.go new file mode 100644 index 0000000000..32afc92973 --- /dev/null +++ b/src/cmd/compile/internal/test/align_test.go @@ -0,0 +1,96 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Test to make sure that equality functions (and hash +// functions) don't do unaligned reads on architectures +// that can't do unaligned reads. See issue 46283. + +package test + +import "testing" + +type T1 struct { + x float32 + a, b, c, d int16 // memequal64 +} +type T2 struct { + x float32 + a, b, c, d int32 // memequal128 +} + +type A2 [2]byte // eq uses a 2-byte load +type A4 [4]byte // eq uses a 4-byte load +type A8 [8]byte // eq uses an 8-byte load + +//go:noinline +func cmpT1(p, q *T1) { + if *p != *q { + panic("comparison test wrong") + } +} + +//go:noinline +func cmpT2(p, q *T2) { + if *p != *q { + panic("comparison test wrong") + } +} + +//go:noinline +func cmpA2(p, q *A2) { + if *p != *q { + panic("comparison test wrong") + } +} + +//go:noinline +func cmpA4(p, q *A4) { + if *p != *q { + panic("comparison test wrong") + } +} + +//go:noinline +func cmpA8(p, q *A8) { + if *p != *q { + panic("comparison test wrong") + } +} + +func TestAlignEqual(t *testing.T) { + cmpT1(&T1{}, &T1{}) + cmpT2(&T2{}, &T2{}) + + m1 := map[T1]bool{} + m1[T1{}] = true + m1[T1{}] = false + if len(m1) != 1 { + t.Fatalf("len(m1)=%d, want 1", len(m1)) + } + m2 := map[T2]bool{} + m2[T2{}] = true + m2[T2{}] = false + if len(m2) != 1 { + t.Fatalf("len(m2)=%d, want 1", len(m2)) + } + + type X2 struct { + y byte + z A2 + } + var x2 X2 + cmpA2(&x2.z, &A2{}) + type X4 struct { + y byte + z A4 + } + var x4 X4 + cmpA4(&x4.z, &A4{}) + type X8 struct { + y byte + z A8 + } + var x8 X8 + cmpA8(&x8.z, &A8{}) +} diff --git a/src/cmd/internal/sys/arch.go b/src/cmd/internal/sys/arch.go index e8687363de..a3e39768b6 100644 --- a/src/cmd/internal/sys/arch.go +++ b/src/cmd/internal/sys/arch.go @@ -40,6 +40,12 @@ type Arch struct { // MinLC is the minimum length of an instruction code. MinLC int + + // Alignment is maximum alignment required by the architecture + // for any (compiler-generated) load or store instruction. + // Loads or stores smaller than Alignment must be naturally aligned. + // Loads or stores larger than Alignment need only be Alignment-aligned. + Alignment int8 } // InFamily reports whether a is a member of any of the specified @@ -60,6 +66,7 @@ var Arch386 = &Arch{ PtrSize: 4, RegSize: 4, MinLC: 1, + Alignment: 1, } var ArchAMD64 = &Arch{ @@ -69,6 +76,7 @@ var ArchAMD64 = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 1, + Alignment: 1, } var ArchARM = &Arch{ @@ -78,6 +86,7 @@ var ArchARM = &Arch{ PtrSize: 4, RegSize: 4, MinLC: 4, + Alignment: 4, // TODO: just for arm5? } var ArchARM64 = &Arch{ @@ -87,6 +96,7 @@ var ArchARM64 = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 4, + Alignment: 1, } var ArchMIPS = &Arch{ @@ -96,6 +106,7 @@ var ArchMIPS = &Arch{ PtrSize: 4, RegSize: 4, MinLC: 4, + Alignment: 4, } var ArchMIPSLE = &Arch{ @@ -105,6 +116,7 @@ var ArchMIPSLE = &Arch{ PtrSize: 4, RegSize: 4, MinLC: 4, + Alignment: 4, } var ArchMIPS64 = &Arch{ @@ -114,6 +126,7 @@ var ArchMIPS64 = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 4, + Alignment: 8, } var ArchMIPS64LE = &Arch{ @@ -123,6 +136,7 @@ var ArchMIPS64LE = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 4, + Alignment: 8, } var ArchPPC64 = &Arch{ @@ -132,6 +146,7 @@ var ArchPPC64 = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 4, + Alignment: 1, } var ArchPPC64LE = &Arch{ @@ -141,6 +156,7 @@ var ArchPPC64LE = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 4, + Alignment: 1, } var ArchRISCV64 = &Arch{ @@ -150,6 +166,7 @@ var ArchRISCV64 = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 4, + Alignment: 8, // riscv unaligned loads work, but are really slow (trap + simulated by OS) } var ArchS390X = &Arch{ @@ -159,6 +176,7 @@ var ArchS390X = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 2, + Alignment: 1, } var ArchWasm = &Arch{ @@ -168,6 +186,7 @@ var ArchWasm = &Arch{ PtrSize: 8, RegSize: 8, MinLC: 1, + Alignment: 1, } var Archs = [...]*Arch{ From 15d9d4a009e3d2c9f1ad501143ed97a8b2c6f2c4 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 14 May 2021 16:01:17 -0400 Subject: [PATCH 35/40] cmd/go: add tests illustrating what happens when Go 1.16 is used in a Go 1.17 main module For #46141 Updates #46160 Change-Id: Ib22435b8051aaf3fa74d43d3b7f2d091e67f05e2 Reviewed-on: https://go-review.googlesource.com/c/go/+/320172 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- .../go/testdata/script/mod_tidy_compat.txt | 100 +++++++++++++++ .../script/mod_tidy_compat_implicit.txt | 120 ++++++++++++++++++ .../script/mod_tidy_compat_incompatible.txt | 116 +++++++++++++++++ .../script/mod_tidy_compat_irrelevant.txt | 105 +++++++++++++++ 4 files changed, 441 insertions(+) create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat.txt create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt diff --git a/src/cmd/go/testdata/script/mod_tidy_compat.txt b/src/cmd/go/testdata/script/mod_tidy_compat.txt new file mode 100644 index 0000000000..e6f88dc779 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat.txt @@ -0,0 +1,100 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + + +# This module has the same module dependency graph in Go 1.16 as in Go 1.17, +# but in 1.16 requires (checksums for) additional (irrelevant) go.mod files. +# +# The module graph under both versions looks like: +# +# m ---- example.com/version v1.1.0 +# | +# + ---- example.net/lazy v0.1.0 ---- example.com/version v1.0.1 +# +# Go 1.17 avoids loading the go.mod file for example.com/version v1.0.1 +# (because it is lower than the verison explicitly required by m, +# and the module that requires it — m — specifies 'go 1.17'). +# +# That go.mod file happens not to affect the final 1.16 module graph anyway, +# so the pruned graph is equivalent to the unpruned one. + +cp go.mod go.mod.orig +go mod tidy +cmp go.mod go.mod.orig + +go list -m all +cmp stdout m_all.txt + +go mod edit -go=1.16 +! go list -m all +stderr '^go list -m: example.net/lazy@v0.1.0 requires\n\texample.com/version@v1.0.1: missing go.sum entry; to add it:\n\tgo mod download example.com/version$' + + +# If we combine a Go 1.16 go.sum file... +go mod tidy -go=1.16 + +# ...with a Go 1.17 go.mod file... +cp go.mod.orig go.mod + +# ...then Go 1.17 continues to work... +go list -m all +cmp stdout m_all.txt + +# ...and now 1.16 can load the same build list! +go mod edit -go=1.16 +go list -m all +cmp stdout m_all.txt + + +# TODO(#46141): Add a cleaner way to tidy a Go 1.17 module while preserving +# the checksums needed to work within it with Go 1.16. + + +-- go.mod -- +// Module m happens to have the exact same build list as what would be +// selected under Go 1.16, but computes that build list without looking at +// as many go.mod files. +module example.com/m + +go 1.17 + +replace example.net/lazy v0.1.0 => ./lazy + +require ( + example.com/version v1.1.0 + example.net/lazy v0.1.0 +) +-- m_all.txt -- +example.com/m +example.com/version v1.1.0 +example.net/lazy v0.1.0 => ./lazy +-- compatible.go -- +package compatible + +import ( + _ "example.com/version" + _ "example.net/lazy" +) +-- lazy/go.mod -- +// Module lazy requires example.com/version v1.0.1. +// +// However, since this module is lazy, its dependents +// should not need checksums for that version of the module +// unless they actually import packages from it. +module example.net/lazy + +go 1.17 + +require example.com/version v1.0.1 +-- lazy/lazy.go -- +package lazy + +import _ "example.com/version" diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt b/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt new file mode 100644 index 0000000000..baaa5d63e3 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt @@ -0,0 +1,120 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + + +# For this module, Go 1.16 selects the same versions of all explicit dependencies +# as Go 1.17 does. However, Go 1.16 selects a higher version of an *implicit* +# dependency, imported by a test of one of the (external) imported packages. +# As a result, Go 1.16 also needs checksums for the module sources for that higher +# version. +# +# The Go 1.16 module graph looks like: +# +# m ---- lazy v0.1.0 ---- incompatible v1.0.0 +# | +# + ------------- requireincompatible v0.1.0 ---- incompatible v2.0.0+incompatible +# +# The Go 1.17 module graph is the same except that the dependencies of +# requireincompatible are pruned out (because the module that requires +# it — lazy v0.1.0 — specifies 'go 1.17', and it is not otherwise relevant to +# the main module). + +cp go.mod go.mod.orig +go mod tidy +cmp go.mod go.mod.orig + +go list -deps -test -f $MODFMT all +stdout '^example\.com/retract/incompatible v1\.0\.0$' + +go mod edit -go=1.16 +! go list -deps -test -f $MODFMT all + + # TODO(#46160): -count=1 instead of -count=2. +stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.com/retract/incompatible@v1\.0\.0: missing go\.sum entry; to add it:\n\tgo mod download example\.com/retract/incompatible$' + + +# If we combine a Go 1.16 go.sum file... +go mod tidy -go=1.16 + +# ...with a Go 1.17 go.mod file... +cp go.mod.orig go.mod + +# ...then Go 1.17 no longer works. 😞 +! go list -deps -test -f $MODFMT all +stderr -count=1 '^can''t load test package: lazy/lazy_test.go:3:8: missing go\.sum entry for module providing package example\.com/retract/incompatible \(imported by example\.net/lazy\); to add:\n\tgo get -t example.net/lazy@v0\.1\.0$' + + +# However, if we take the union of the go.sum files... +go list -mod=mod -deps -test all +cmp go.mod go.mod.orig + +# ...then Go 1.17 continues to work... +go list -deps -test -f $MODFMT all +stdout '^example\.com/retract/incompatible v1\.0\.0$' + +# ...and 1.16 also works(‽), but selects a different version for the +# external-test dependency. +go mod edit -go=1.16 +go list -deps -test -f $MODFMT all +stdout '^example\.com/retract/incompatible v2\.0\.0\+incompatible$' + + +# TODO(#46100): In compatibility mode, should we reject the above difference as +# incompatible, or save checksums for both possible versions of the test +# dependency? + + +-- go.mod -- +// Module m imports packages from the same versions under Go 1.17 +// as under Go 1.16, but under 1.16 its (implicit) external test dependencies +// are higher. +module example.com/m + +go 1.17 + +replace ( + example.net/lazy v0.1.0 => ./lazy + example.net/requireincompatible v0.1.0 => ./requireincompatible +) + +require example.net/lazy v0.1.0 +-- implicit.go -- +package implicit + +import _ "example.net/lazy" +-- lazy/go.mod -- +// Module lazy requires example.com/retract/incompatible v1.0.0. +// +// When viewed from the outside it also has a transitive dependency +// on v2.0.0+incompatible, but in lazy mode that transitive dependency +// is pruned out. +module example.net/lazy + +go 1.17 + +exclude example.com/retract/incompatible v2.0.0+incompatible + +require ( + example.com/retract/incompatible v1.0.0 + example.net/requireincompatible v0.1.0 +) +-- lazy/lazy.go -- +package lazy +-- lazy/lazy_test.go -- +package lazy_test + +import _ "example.com/retract/incompatible" +-- requireincompatible/go.mod -- +module example.net/requireincompatible + +go 1.15 + +require example.com/retract/incompatible v2.0.0+incompatible diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt b/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt new file mode 100644 index 0000000000..0861352408 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt @@ -0,0 +1,116 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + + +# For this module, Go 1.17 prunes out a (transitive and otherwise-irrelevant) +# requirement on a retracted higher version of a dependency. +# However, when Go 1.16 reads the same requirements from the go.mod file, +# it does not prune out that requirement, and selects the retracted version. +# +# The Go 1.16 module graph looks like: +# +# m ---- lazy v0.1.0 ---- requireincompatible v0.1.0 ---- incompatible v2.0.0+incompatible +# | | +# + -------+------------- incompatible v1.0.0 +# +# The Go 1.17 module graph is the same except that the dependencies of +# requireincompatible are pruned out (because the module that requires +# it — lazy v0.1.0 — specifies 'go 1.17', and it is not otherwise relevant to +# the main module). + + +# TODO(#46141): 'go mod tidy' should by default diagnose the difference in +# dependencies as an error, but it should still be possible to simply drop +# compatibility with Go 1.16 by passing an appropriate '-compat' flag. + +cp go.mod go.mod.orig +go mod tidy +cmp go.mod go.mod.orig + +go mod edit -go=1.16 +! go list -f $MODFMT -deps ./... + # TODO(#46160): -count=1 instead of -count=2. +stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.net/requireincompatible@v0\.1\.0 requires\n\texample\.com/retract/incompatible@v2\.0\.0\+incompatible: missing go.sum entry; to add it:\n\tgo mod download example.com/retract/incompatible$' + + +# There are two ways for the module author to bring the two into alignment. +# One is to *explicitly* 'exclude' the version that is already *implicitly* +# pruned out under 1.17. +go mod edit -exclude=example.com/retract/incompatible@v2.0.0+incompatible +go list -f $MODFMT -deps ./... +stdout '^example.com/retract/incompatible v1\.0\.0$' +! stdout 'v2\.0\.0' + + +# The other is to explicitly upgrade the version required under Go 1.17 +# to match the version selected by Go 1.16. +cp go.mod.orig go.mod + +go get -d example.com/retract/incompatible@v2.0.0+incompatible + # Note that we are not running 'go mod tidy' here: we need to preserve + # the checksum for v1.0.0 because it is also still in the module graph + # as seen by Go 1.16. + +go mod edit -go=1.16 +go list -f $MODFMT -deps ./... +stdout '^example.com/retract/incompatible v2\.0\.0\+incompatible$' +! stdout 'v1\.0\.0' + + +-- go.mod -- +// Module m indirectly imports a package from +// example.com/retract/incompatible. Its selected version of +// that module is lower under Go 1.17 semantics than under Go 1.16. +module example.com/m + +go 1.17 + +replace ( + example.net/lazy v0.1.0 => ./lazy + example.net/requireincompatible v0.1.0 => ./requireincompatible +) + +require ( + example.com/retract/incompatible v1.0.0 // indirect + example.net/lazy v0.1.0 +) +-- incompatible.go -- +package incompatible + +import _ "example.net/lazy" + +-- lazy/go.mod -- +// Module lazy requires example.com/retract/incompatible v1.0.0. +// +// When viewed from the outside it also has a transitive dependency +// on v2.0.0+incompatible, but in lazy mode that transitive dependency +// is pruned out. +module example.net/lazy + +go 1.17 + +exclude example.com/retract/incompatible v2.0.0+incompatible + +require ( + example.com/retract/incompatible v1.0.0 + example.net/requireincompatible v0.1.0 +) +-- lazy/lazy.go -- +package lazy + +import _ "example.com/retract/incompatible" + +-- requireincompatible/go.mod -- +module example.net/requireincompatible + +go 1.15 + +require example.com/retract/incompatible v2.0.0+incompatible diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt b/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt new file mode 100644 index 0000000000..d4371e0f7d --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt @@ -0,0 +1,105 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + + +# This module selects the same versions in Go 1.16 and 1.17 for all modules +# that provide packages (or test dependencies of packages) imported by the +# main module. However, in Go 1.16 it selects a higher version of a +# transitive module dependency that is not otherwise relevant to the main module. +# As a result, Go 1.16 needs an additional checksum for the go.mod file of +# that irrelevant dependency. +# +# The Go 1.16 module graph looks like: +# +# m ---- lazy v0.1.0 ---- incompatible v1.0.0 +# | +# + ------------- requireincompatible v0.1.0 ---- incompatible v2.0.0+incompatible + +cp go.mod go.mod.orig +go mod tidy +cmp go.mod go.mod.orig + +go list -m all +stdout '^example\.com/retract/incompatible v1\.0\.0$' + +go mod edit -go=1.16 +! go list -deps -test -f $MODFMT all + # TODO(#46160): -count=1 instead of -count=2. +stderr -count=2 '^go: example.net/lazy@v0.1.0 requires\n\texample.com/retract/incompatible@v1.0.0: missing go.sum entry; to add it:\n\tgo mod download example.com/retract/incompatible$' + + +# If we combine a Go 1.16 go.sum file... +go mod tidy -go=1.16 + +# ...with a Go 1.17 go.mod file... +cp go.mod.orig go.mod + +# ...then Go 1.17 continues to work... +go list -deps -test -f $MODFMT all +cp stdout out-117.txt + +# ...and 1.16 also works, and selects the same versions for all packages +# even remotely relevant to the main module. +go mod edit -go=1.16 +go list -deps -test -f $MODFMT all +cmp stdout out-117.txt + + +# TODO(#46160): Add a cleaner way to tidy a Go 1.17 module while preserving +# the checksums needed to work within it with Go 1.16. + + +-- go.mod -- +// Module m imports packages from the same versions under Go 1.17 +// as under Go 1.16, but under 1.16 its (implicit) external test dependencies +// are higher. +module example.com/m + +go 1.17 + +replace ( + example.net/lazy v0.1.0 => ./lazy + example.net/requireincompatible v0.1.0 => ./requireincompatible +) + +require example.net/lazy v0.1.0 +-- m.go -- +package m + +import _ "example.net/lazy" +-- lazy/go.mod -- +// Module lazy requires example.com/retract/incompatible v1.0.0. +// +// When viewed from the outside it also has a transitive dependency +// on v2.0.0+incompatible, but in lazy mode that transitive dependency +// is pruned out. +module example.net/lazy + +go 1.17 + +exclude example.com/retract/incompatible v2.0.0+incompatible + +require ( + example.com/retract/incompatible v1.0.0 + example.net/requireincompatible v0.1.0 +) +-- lazy/lazy.go -- +package lazy +-- lazy/unimported/unimported.go -- +package unimported + +import _ "example.com/retract/incompatible" +-- requireincompatible/go.mod -- +module example.net/requireincompatible + +go 1.15 + +require example.com/retract/incompatible v2.0.0+incompatible From 32b73ae18026e8a9dc4c5aa49999b1ea445bc68c Mon Sep 17 00:00:00 2001 From: Constantin Konstantinidis Date: Sun, 16 May 2021 11:03:34 +0200 Subject: [PATCH 36/40] cmd/go: align checks of module path during initialization. Fixes #45025. Change-Id: I70c2b745f764484e4b3a2824adc470f168fb2c50 Reviewed-on: https://go-review.googlesource.com/c/go/+/320310 Trust: Jay Conrod Trust: Bryan C. Mills Run-TryBot: Jay Conrod Reviewed-by: Bryan C. Mills Reviewed-by: Jay Conrod TryBot-Result: Go Bot --- src/cmd/go/internal/modload/init.go | 68 ++++--------------- src/cmd/go/testdata/script/mod_init_path.txt | 2 +- .../go/testdata/script/mod_invalid_path.txt | 9 +-- 3 files changed, 19 insertions(+), 60 deletions(-) diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index e358230e74..df9f48e8ea 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -432,7 +432,10 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) { initTarget(f.Module.Mod) index = indexModFile(data, f, fixed) - if err := checkModulePathLax(f.Module.Mod.Path); err != nil { + if err := module.CheckImportPath(f.Module.Mod.Path); err != nil { + if pathErr, ok := err.(*module.InvalidPathError); ok { + pathErr.Kind = "module" + } base.Fatalf("go: %v", err) } @@ -492,7 +495,15 @@ func CreateModFile(ctx context.Context, modPath string) { if err != nil { base.Fatalf("go: %v", err) } - } else if err := checkModulePathLax(modPath); err != nil { + } else if err := module.CheckImportPath(modPath); err != nil { + if pathErr, ok := err.(*module.InvalidPathError); ok { + pathErr.Kind = "module" + // Same as build.IsLocalPath() + if pathErr.Path == "." || pathErr.Path == ".." || + strings.HasPrefix(pathErr.Path, "./") || strings.HasPrefix(pathErr.Path, "../") { + pathErr.Err = errors.New("is a local import path") + } + } base.Fatalf("go: %v", err) } @@ -536,49 +547,6 @@ func CreateModFile(ctx context.Context, modPath string) { } } -// checkModulePathLax checks that the path meets some minimum requirements -// to avoid confusing users or the module cache. The requirements are weaker -// than those of module.CheckPath to allow room for weakening module path -// requirements in the future, but strong enough to help users avoid significant -// problems. -func checkModulePathLax(p string) error { - // TODO(matloob): Replace calls of this function in this CL with calls - // to module.CheckImportPath once it's been laxened, if it becomes laxened. - // See golang.org/issue/29101 for a discussion about whether to make CheckImportPath - // more lax or more strict. - - errorf := func(format string, args ...interface{}) error { - return fmt.Errorf("invalid module path %q: %s", p, fmt.Sprintf(format, args...)) - } - - // Disallow shell characters " ' * < > ? ` | to avoid triggering bugs - // with file systems and subcommands. Disallow file path separators : and \ - // because path separators other than / will confuse the module cache. - // See fileNameOK in golang.org/x/mod/module/module.go. - shellChars := "`" + `"'*<>?|` - fsChars := `\:` - if i := strings.IndexAny(p, shellChars); i >= 0 { - return errorf("contains disallowed shell character %q", p[i]) - } - if i := strings.IndexAny(p, fsChars); i >= 0 { - return errorf("contains disallowed path separator character %q", p[i]) - } - - // Ensure path.IsAbs and build.IsLocalImport are false, and that the path is - // invariant under path.Clean, also to avoid confusing the module cache. - if path.IsAbs(p) { - return errorf("is an absolute path") - } - if build.IsLocalImport(p) { - return errorf("is a local import path") - } - if path.Clean(p) != p { - return errorf("is not clean") - } - - return nil -} - // fixVersion returns a modfile.VersionFixer implemented using the Query function. // // It resolves commit hashes and branch names to versions, @@ -918,14 +886,8 @@ func findModulePath(dir string) (string, error) { } if rel := search.InDir(dir, filepath.Join(gpdir, "src")); rel != "" && rel != "." { path := filepath.ToSlash(rel) - // TODO(matloob): replace this with module.CheckImportPath - // once it's been laxened. - // Only checkModulePathLax here. There are some unpublishable - // module names that are compatible with checkModulePathLax - // but they already work in GOPATH so don't break users - // trying to do a build with modules. gorelease will alert users - // publishing their modules to fix their paths. - if err := checkModulePathLax(path); err != nil { + // gorelease will alert users publishing their modules to fix their paths. + if err := module.CheckImportPath(path); err != nil { badPathErr = err break } diff --git a/src/cmd/go/testdata/script/mod_init_path.txt b/src/cmd/go/testdata/script/mod_init_path.txt index ccdfc92317..e5fd4ddbcb 100644 --- a/src/cmd/go/testdata/script/mod_init_path.txt +++ b/src/cmd/go/testdata/script/mod_init_path.txt @@ -1,7 +1,7 @@ env GO111MODULE=on ! go mod init . -stderr '^go: invalid module path "\.": is a local import path$' +stderr '^go: malformed module path ".": is a local import path$' cd x go mod init example.com/x diff --git a/src/cmd/go/testdata/script/mod_invalid_path.txt b/src/cmd/go/testdata/script/mod_invalid_path.txt index c8c075daae..333a3ffa35 100644 --- a/src/cmd/go/testdata/script/mod_invalid_path.txt +++ b/src/cmd/go/testdata/script/mod_invalid_path.txt @@ -8,11 +8,8 @@ stderr '^go: no module declaration in go.mod. To specify the module path:\n\tgo # Test that go mod init in GOPATH doesn't add a module declaration # with a path that can't possibly be a module path, because # it isn't even a valid import path. -# The single quote and backtick are the only characters we don't allow -# in checkModulePathLax, but is allowed in a Windows file name. -# TODO(matloob): choose a different character once -# module.CheckImportPath is laxened and replaces -# checkModulePathLax. +# The single quote and backtick are the only characters which are not allowed +# but are a valid Windows file name. cd $WORK/'gopath/src/m''d' ! go mod init stderr 'cannot determine module path' @@ -21,7 +18,7 @@ stderr 'cannot determine module path' # possibly be a module path, because it isn't even a valid import path cd $WORK/gopath/src/badname ! go list . -stderr 'invalid module path' +stderr 'malformed module path' # Test that an import path containing an element with a leading dot is valid, # but such a module path is not. From 08a8fa9c471603c7ec44895392c6bfa31a8ddcb6 Mon Sep 17 00:00:00 2001 From: Richard Musiol Date: Sun, 23 May 2021 23:06:43 +0200 Subject: [PATCH 37/40] misc/wasm: ensure correct stack pointer in catch clauses The stack pointer may have changed after a call from JavaScript into Go code because of stack growth. The normal case already updated the sp variable accordingly, but the catch case did not yet. Fixes #45433 Change-Id: I3e0a33381929626f6b21902948935eb5ffb26c96 Reviewed-on: https://go-review.googlesource.com/c/go/+/321936 Trust: Richard Musiol Run-TryBot: Richard Musiol TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- misc/wasm/wasm_exec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/misc/wasm/wasm_exec.js b/misc/wasm/wasm_exec.js index 3e41e628ef..231185a123 100644 --- a/misc/wasm/wasm_exec.js +++ b/misc/wasm/wasm_exec.js @@ -401,6 +401,7 @@ storeValue(sp + 56, result); this.mem.setUint8(sp + 64, 1); } catch (err) { + sp = this._inst.exports.getsp() >>> 0; // see comment above storeValue(sp + 56, err); this.mem.setUint8(sp + 64, 0); } @@ -417,6 +418,7 @@ storeValue(sp + 40, result); this.mem.setUint8(sp + 48, 1); } catch (err) { + sp = this._inst.exports.getsp() >>> 0; // see comment above storeValue(sp + 40, err); this.mem.setUint8(sp + 48, 0); } @@ -433,6 +435,7 @@ storeValue(sp + 40, result); this.mem.setUint8(sp + 48, 1); } catch (err) { + sp = this._inst.exports.getsp() >>> 0; // see comment above storeValue(sp + 40, err); this.mem.setUint8(sp + 48, 0); } From c89f1224a544cde464fcb86e78ebb0cc97eedba2 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 17 May 2021 10:06:51 -0700 Subject: [PATCH 38/40] net: verify results from Lookup* are valid domain names For the methods LookupCNAME, LookupSRV, LookupMX, LookupNS, and LookupAddr check that the returned domain names are in fact valid DNS names using the existing isDomainName function. Thanks to Philipp Jeitner and Haya Shulman from Fraunhofer SIT for reporting this issue. Fixes #46241 Fixes CVE-2021-33195 Change-Id: Icf231acd93178a3b6aec3f178cff7e693f74ef8c Reviewed-on: https://go-review.googlesource.com/c/go/+/320949 Trust: Roland Shoemaker Trust: Katie Hockman Run-TryBot: Roland Shoemaker TryBot-Result: Go Bot Reviewed-by: Katie Hockman --- src/net/dnsclient_unix_test.go | 121 +++++++++++++++++++++++++++++++++ src/net/lookup.go | 98 +++++++++++++++++++++++--- 2 files changed, 211 insertions(+), 8 deletions(-) diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index ec690a1c0c..69a9b972f0 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -1799,3 +1799,124 @@ func TestPTRandNonPTR(t *testing.T) { t.Errorf("names = %q; want %q", names, want) } } + +func TestCVE202133195(t *testing.T) { + fake := fakeDNSServer{ + rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) { + r := dnsmessage.Message{ + Header: dnsmessage.Header{ + ID: q.Header.ID, + Response: true, + RCode: dnsmessage.RCodeSuccess, + RecursionAvailable: true, + }, + Questions: q.Questions, + } + switch q.Questions[0].Type { + case dnsmessage.TypeCNAME: + r.Answers = []dnsmessage.Resource{} + case dnsmessage.TypeA: // CNAME lookup uses a A/AAAA as a proxy + r.Answers = append(r.Answers, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(".golang.org."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.AResource{ + A: TestAddr, + }, + }, + ) + case dnsmessage.TypeSRV: + n := q.Questions[0].Name + if n.String() == "_hdr._tcp.golang.org." { + n = dnsmessage.MustNewName(".golang.org.") + } + r.Answers = append(r.Answers, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: n, + Type: dnsmessage.TypeSRV, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.SRVResource{ + Target: dnsmessage.MustNewName(".golang.org."), + }, + }, + ) + case dnsmessage.TypeMX: + r.Answers = append(r.Answers, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(".golang.org."), + Type: dnsmessage.TypeMX, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.MXResource{ + MX: dnsmessage.MustNewName(".golang.org."), + }, + }, + ) + case dnsmessage.TypeNS: + r.Answers = append(r.Answers, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(".golang.org."), + Type: dnsmessage.TypeNS, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.NSResource{ + NS: dnsmessage.MustNewName(".golang.org."), + }, + }, + ) + case dnsmessage.TypePTR: + r.Answers = append(r.Answers, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(".golang.org."), + Type: dnsmessage.TypePTR, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.PTRResource{ + PTR: dnsmessage.MustNewName(".golang.org."), + }, + }, + ) + } + return r, nil + }, + } + r := Resolver{PreferGo: true, Dial: fake.DialContext} + + _, err := r.LookupCNAME(context.Background(), "golang.org") + if expected := "lookup golang.org: CNAME target is invalid"; err.Error() != expected { + t.Errorf("LookupCNAME returned unexpected error, got %q, want %q", err.Error(), expected) + } + _, _, err = r.LookupSRV(context.Background(), "target", "tcp", "golang.org") + if expected := "lookup golang.org: SRV target is invalid"; err.Error() != expected { + t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected) + } + _, _, err = r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org") + if expected := "lookup golang.org: SRV header name is invalid"; err.Error() != expected { + t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected) + } + _, err = r.LookupMX(context.Background(), "golang.org") + if expected := "lookup golang.org: MX target is invalid"; err.Error() != expected { + t.Errorf("LookupMX returned unexpected error, got %q, want %q", err.Error(), expected) + } + _, err = r.LookupNS(context.Background(), "golang.org") + if expected := "lookup golang.org: NS target is invalid"; err.Error() != expected { + t.Errorf("LookupNS returned unexpected error, got %q, want %q", err.Error(), expected) + } + _, err = r.LookupAddr(context.Background(), "1.2.3.4") + if expected := "lookup 1.2.3.4: PTR target is invalid"; err.Error() != expected { + t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err.Error(), expected) + } +} diff --git a/src/net/lookup.go b/src/net/lookup.go index 03599503bd..39d33796d5 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -396,6 +396,9 @@ func (r *Resolver) LookupPort(ctx context.Context, network, service string) (por // contain DNS "CNAME" records, as long as host resolves to // address records. // +// The returned canonical name is validated to be a properly +// formatted presentation-format domain name. +// // LookupCNAME uses context.Background internally; to specify the context, use // Resolver.LookupCNAME. func LookupCNAME(host string) (cname string, err error) { @@ -412,8 +415,18 @@ func LookupCNAME(host string) (cname string, err error) { // LookupCNAME does not return an error if host does not // contain DNS "CNAME" records, as long as host resolves to // address records. -func (r *Resolver) LookupCNAME(ctx context.Context, host string) (cname string, err error) { - return r.lookupCNAME(ctx, host) +// +// The returned canonical name is validated to be a properly +// formatted presentation-format domain name. +func (r *Resolver) LookupCNAME(ctx context.Context, host string) (string, error) { + cname, err := r.lookupCNAME(ctx, host) + if err != nil { + return "", err + } + if !isDomainName(cname) { + return "", &DNSError{Err: "CNAME target is invalid", Name: host} + } + return cname, nil } // LookupSRV tries to resolve an SRV query of the given service, @@ -425,6 +438,9 @@ func (r *Resolver) LookupCNAME(ctx context.Context, host string) (cname string, // That is, it looks up _service._proto.name. To accommodate services // publishing SRV records under non-standard names, if both service // and proto are empty strings, LookupSRV looks up name directly. +// +// The returned service names are validated to be properly +// formatted presentation-format domain names. func LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err error) { return DefaultResolver.lookupSRV(context.Background(), service, proto, name) } @@ -438,12 +454,33 @@ func LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err err // That is, it looks up _service._proto.name. To accommodate services // publishing SRV records under non-standard names, if both service // and proto are empty strings, LookupSRV looks up name directly. -func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (cname string, addrs []*SRV, err error) { - return r.lookupSRV(ctx, service, proto, name) +// +// The returned service names are validated to be properly +// formatted presentation-format domain names. +func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (string, []*SRV, error) { + cname, addrs, err := r.lookupSRV(ctx, service, proto, name) + if err != nil { + return "", nil, err + } + if cname != "" && !isDomainName(cname) { + return "", nil, &DNSError{Err: "SRV header name is invalid", Name: name} + } + for _, addr := range addrs { + if addr == nil { + continue + } + if !isDomainName(addr.Target) { + return "", nil, &DNSError{Err: "SRV target is invalid", Name: name} + } + } + return cname, addrs, nil } // LookupMX returns the DNS MX records for the given domain name sorted by preference. // +// The returned mail server names are validated to be properly +// formatted presentation-format domain names. +// // LookupMX uses context.Background internally; to specify the context, use // Resolver.LookupMX. func LookupMX(name string) ([]*MX, error) { @@ -451,12 +488,30 @@ func LookupMX(name string) ([]*MX, error) { } // LookupMX returns the DNS MX records for the given domain name sorted by preference. +// +// The returned mail server names are validated to be properly +// formatted presentation-format domain names. func (r *Resolver) LookupMX(ctx context.Context, name string) ([]*MX, error) { - return r.lookupMX(ctx, name) + records, err := r.lookupMX(ctx, name) + if err != nil { + return nil, err + } + for _, mx := range records { + if mx == nil { + continue + } + if !isDomainName(mx.Host) { + return nil, &DNSError{Err: "MX target is invalid", Name: name} + } + } + return records, nil } // LookupNS returns the DNS NS records for the given domain name. // +// The returned name server names are validated to be properly +// formatted presentation-format domain names. +// // LookupNS uses context.Background internally; to specify the context, use // Resolver.LookupNS. func LookupNS(name string) ([]*NS, error) { @@ -464,8 +519,23 @@ func LookupNS(name string) ([]*NS, error) { } // LookupNS returns the DNS NS records for the given domain name. +// +// The returned name server names are validated to be properly +// formatted presentation-format domain names. func (r *Resolver) LookupNS(ctx context.Context, name string) ([]*NS, error) { - return r.lookupNS(ctx, name) + records, err := r.lookupNS(ctx, name) + if err != nil { + return nil, err + } + for _, ns := range records { + if ns == nil { + continue + } + if !isDomainName(ns.Host) { + return nil, &DNSError{Err: "NS target is invalid", Name: name} + } + } + return records, nil } // LookupTXT returns the DNS TXT records for the given domain name. @@ -495,6 +565,18 @@ func LookupAddr(addr string) (names []string, err error) { // LookupAddr performs a reverse lookup for the given address, returning a list // of names mapping to that address. -func (r *Resolver) LookupAddr(ctx context.Context, addr string) (names []string, err error) { - return r.lookupAddr(ctx, addr) +// +// The returned names are validated to be properly +// formatted presentation-format domain names. +func (r *Resolver) LookupAddr(ctx context.Context, addr string) ([]string, error) { + names, err := r.lookupAddr(ctx, addr) + if err != nil { + return nil, err + } + for _, name := range names { + if !isDomainName(name) { + return nil, &DNSError{Err: "PTR target is invalid", Name: addr} + } + } + return names, nil } From 8b462d75670dcd8b6a08cf9af225eb8e7628d412 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 14 May 2021 16:53:06 -0400 Subject: [PATCH 39/40] cmd/go: add a -compat flag to 'go mod tidy' Fixes #46141 Change-Id: I9d4032e75252ade9eaa937389ea97ef3fb287499 Reviewed-on: https://go-review.googlesource.com/c/go/+/321071 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- doc/go1.17.html | 5 + src/cmd/go.sum | 23 ++ src/cmd/go/alldocs.go | 10 +- src/cmd/go/internal/modcmd/tidy.go | 17 +- src/cmd/go/internal/modload/init.go | 24 +- src/cmd/go/internal/modload/load.go | 254 +++++++++++++++++- .../go/testdata/script/mod_tidy_compat.txt | 35 ++- .../testdata/script/mod_tidy_compat_added.txt | 105 ++++++++ .../script/mod_tidy_compat_ambiguous.txt | 98 +++++++ .../script/mod_tidy_compat_deleted.txt | 128 +++++++++ .../script/mod_tidy_compat_implicit.txt | 23 +- .../script/mod_tidy_compat_incompatible.txt | 37 ++- .../script/mod_tidy_compat_irrelevant.txt | 34 +-- src/cmd/go/testdata/script/mod_tidy_oldgo.txt | 21 ++ src/go.sum | 7 + 15 files changed, 758 insertions(+), 63 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat_added.txt create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat_ambiguous.txt create mode 100644 src/cmd/go/testdata/script/mod_tidy_compat_deleted.txt create mode 100644 src/cmd/go/testdata/script/mod_tidy_oldgo.txt diff --git a/doc/go1.17.html b/doc/go1.17.html index 46ee1da6fa..6dd1d0d1db 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -106,6 +106,11 @@ Do not send CLs removing the interior tags from such phrases. go mod tidy -go=1.17 +

+ TODO: Describe the -compat flag + for go mod tidy. +

+

Module deprecation comments

diff --git a/src/cmd/go.sum b/src/cmd/go.sum index 9af4978d66..eeb625fcf8 100644 --- a/src/cmd/go.sum +++ b/src/cmd/go.sum @@ -5,18 +5,41 @@ github.com/google/pprof v0.0.0-20210506205249-923b5ab0fc1a h1:jmAp/2PZAScNd62lTD github.com/google/pprof v0.0.0-20210506205249-923b5ab0fc1a/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639 h1:mV02weKRL81bEnm8A0HT1/CAelMQDBuQIfLw8n+d6xI= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= golang.org/x/arch v0.0.0-20210502124803-cbf565b21d1e h1:pv3V0NlNSh5Q6AX/StwGLBjcLS7UN4m4Gq+V+uSecqM= golang.org/x/arch v0.0.0-20210502124803-cbf565b21d1e/go.mod h1:5om86z9Hs0C8fWVUuoMHwpExlXzs5Tkyp9hOrfG7pp8= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e h1:8foAy0aoO5GkqCvAEJ4VC4P3zksTg4X4aJCDpZzmgQI= golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd h1:CuRnpyMrCCBulv0d/y0CswR4K0vGydgE3DZ2wYPIOo8= golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 h1:yhBbb4IRs2HS9PPlAg6DMC6mUOKexJBNsLf4Z+6En1Q= golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210503060354-a79de5458b56 h1:b8jxX3zqjpqb2LklXPzKSGJhzyxCOZSz8ncv8Nv+y7w= golang.org/x/term v0.0.0-20210503060354-a79de5458b56/go.mod h1:tfny5GFUkzUvx4ps4ajbZsCe5lw1metzhBm9T3x7oIY= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9 h1:2XlR/j4I4xz5GQZI7zBjqTfezYyRIE2jD5IMousB2rg= golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index fcc7f36335..bad2b7f16e 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1221,7 +1221,7 @@ // // Usage: // -// go mod tidy [-e] [-v] [-go=version] +// go mod tidy [-e] [-v] [-go=version] [-compat=version] // // Tidy makes sure go.mod matches the source code in the module. // It adds any missing modules necessary to build the current module's @@ -1241,6 +1241,14 @@ // (Go versions 1.17 and higher retain more requirements in order to // support lazy module loading.) // +// The -compat flag preserves any additional checksums needed for the +// 'go' command from the indicated major Go release to successfully load +// the module graph, and causes tidy to error out if that version of the +// 'go' command would load any imported package from a different module +// version. By default, tidy acts as if the -compat flag were set to the +// version prior to the one indicated by the 'go' directive in the go.mod +// file. +// // See https://golang.org/ref/mod#go-mod-tidy for more about 'go mod tidy'. // // diff --git a/src/cmd/go/internal/modcmd/tidy.go b/src/cmd/go/internal/modcmd/tidy.go index 9af624028a..fe25507e94 100644 --- a/src/cmd/go/internal/modcmd/tidy.go +++ b/src/cmd/go/internal/modcmd/tidy.go @@ -19,7 +19,7 @@ import ( ) var cmdTidy = &base.Command{ - UsageLine: "go mod tidy [-e] [-v] [-go=version]", + UsageLine: "go mod tidy [-e] [-v] [-go=version] [-compat=version]", Short: "add missing and remove unused modules", Long: ` Tidy makes sure go.mod matches the source code in the module. @@ -40,20 +40,30 @@ are retained as explicit requirements in the go.mod file. (Go versions 1.17 and higher retain more requirements in order to support lazy module loading.) +The -compat flag preserves any additional checksums needed for the +'go' command from the indicated major Go release to successfully load +the module graph, and causes tidy to error out if that version of the +'go' command would load any imported package from a different module +version. By default, tidy acts as if the -compat flag were set to the +version prior to the one indicated by the 'go' directive in the go.mod +file. + See https://golang.org/ref/mod#go-mod-tidy for more about 'go mod tidy'. `, Run: runTidy, } var ( - tidyE bool // if true, report errors but proceed anyway. - tidyGo goVersionFlag // go version to write to the tidied go.mod file (toggles lazy loading) + tidyE bool // if true, report errors but proceed anyway. + tidyGo goVersionFlag // go version to write to the tidied go.mod file (toggles lazy loading) + tidyCompat goVersionFlag // go version for which the tidied go.mod and go.sum files should be “compatible” ) func init() { cmdTidy.Flag.BoolVar(&cfg.BuildV, "v", false, "") cmdTidy.Flag.BoolVar(&tidyE, "e", false, "") cmdTidy.Flag.Var(&tidyGo, "go", "") + cmdTidy.Flag.Var(&tidyCompat, "compat", "") base.AddModCommonFlags(&cmdTidy.Flag) } @@ -105,6 +115,7 @@ func runTidy(ctx context.Context, cmd *base.Command, args []string) { GoVersion: tidyGo.String(), Tags: imports.AnyTags(), Tidy: true, + TidyCompatibleVersion: tidyCompat.String(), VendorModulesInGOROOTSrc: true, ResolveMissingImports: true, LoadTests: true, diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index df9f48e8ea..86c0db3fe4 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -767,11 +767,33 @@ func LatestGoVersion() string { tags := build.Default.ReleaseTags version := tags[len(tags)-1] if !strings.HasPrefix(version, "go") || !modfile.GoVersionRE.MatchString(version[2:]) { - base.Fatalf("go: unrecognized default version %q", version) + base.Fatalf("go: internal error: unrecognized default version %q", version) } return version[2:] } +// priorGoVersion returns the Go major release immediately preceding v, +// or v itself if v is the first Go major release (1.0) or not a supported +// Go version. +func priorGoVersion(v string) string { + vTag := "go" + v + tags := build.Default.ReleaseTags + for i, tag := range tags { + if tag == vTag { + if i == 0 { + return v + } + + version := tags[i-1] + if !strings.HasPrefix(version, "go") || !modfile.GoVersionRE.MatchString(version[2:]) { + base.Fatalf("go: internal error: unrecognized version %q", version) + } + return version[2:] + } + } + return v +} + var altConfigs = []string{ "Gopkg.lock", diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 37b0032d43..a9d1777125 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -152,6 +152,13 @@ type PackageOpts struct { // packages. Tidy bool + // TidyCompatibleVersion is the oldest Go version that must be able to + // reproducibly reload the requested packages. + // + // If empty, the compatible version is the Go version immediately prior to the + // 'go' version listed in the go.mod file. + TidyCompatibleVersion string + // VendorModulesInGOROOTSrc indicates that if we are within a module in // GOROOT/src, packages in the module's vendor directory should be resolved as // actual module dependencies (instead of standard-library packages). @@ -371,7 +378,26 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma } } - modfetch.TrimGoSum(keepSums(ctx, ld, ld.requirements, loadedZipSumsOnly)) + keep := keepSums(ctx, ld, ld.requirements, loadedZipSumsOnly) + if compatDepth := modDepthFromGoVersion(ld.TidyCompatibleVersion); compatDepth != ld.requirements.depth { + compatRS := newRequirements(compatDepth, ld.requirements.rootModules, ld.requirements.direct) + ld.checkTidyCompatibility(ctx, compatRS) + + for m := range keepSums(ctx, ld, compatRS, loadedZipSumsOnly) { + keep[m] = true + } + } + + if allowWriteGoMod { + modfetch.TrimGoSum(keep) + + // commitRequirements below will also call WriteGoSum, but the "keep" map + // we have here could be strictly larger: commitRequirements only commits + // loaded.requirements, but here we may have also loaded (and want to + // preserve checksums for) additional entities from compatRS, which are + // only needed for compatibility with ld.TidyCompatibleVersion. + modfetch.WriteGoSum(keep) + } } // Success! Update go.mod and go.sum (if needed) and return the results. @@ -924,6 +950,17 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader { } } + if ld.Tidy { + if ld.TidyCompatibleVersion == "" { + ld.TidyCompatibleVersion = priorGoVersion(ld.GoVersion) + } else if semver.Compare("v"+ld.TidyCompatibleVersion, "v"+ld.GoVersion) > 0 { + // Each version of the Go toolchain knows how to interpret go.mod and + // go.sum files produced by all previous versions, so a compatibility + // version higher than the go.mod version adds nothing. + ld.TidyCompatibleVersion = ld.GoVersion + } + } + if semver.Compare("v"+ld.GoVersion, narrowAllVersionV) < 0 && !ld.UseVendorAll { // The module's go version explicitly predates the change in "all" for lazy // loading, so continue to use the older interpretation. @@ -1072,7 +1109,7 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader { // If that is not the case, there is a bug in the loading loop above. for _, m := range rs.rootModules { if v, ok := ld.requirements.rootSelected(m.Path); !ok || v != m.Version { - ld.errorf("go: internal error: a requirement on %v is needed but was not added during package loading\n", m) + ld.errorf("go mod tidy: internal error: a requirement on %v is needed but was not added during package loading\n", m) base.ExitIfErrors() } } @@ -1743,6 +1780,219 @@ func (ld *loader) checkMultiplePaths() { } } +// checkTidyCompatibility emits an error if any package would be loaded from a +// different module under rs than under ld.requirements. +func (ld *loader) checkTidyCompatibility(ctx context.Context, rs *Requirements) { + suggestUpgrade := false + suggestEFlag := false + suggestFixes := func() { + if ld.AllowErrors { + // The user is explicitly ignoring these errors, so don't bother them with + // other options. + return + } + + // We print directly to os.Stderr because this information is advice about + // how to fix errors, not actually an error itself. + // (The actual errors should have been logged already.) + + fmt.Fprintln(os.Stderr) + + goFlag := "" + if ld.GoVersion != modFileGoVersion() { + goFlag = " -go=" + ld.GoVersion + } + + compatFlag := "" + if ld.TidyCompatibleVersion != priorGoVersion(ld.GoVersion) { + compatFlag = " -compat=" + ld.TidyCompatibleVersion + } + if suggestUpgrade { + eDesc := "" + eFlag := "" + if suggestEFlag { + eDesc = ", leaving some packages unresolved" + eFlag = " -e" + } + fmt.Fprintf(os.Stderr, "To upgrade to the versions selected by go %s%s:\n\tgo mod tidy%s -go=%s && go mod tidy%s -go=%s%s\n", ld.TidyCompatibleVersion, eDesc, eFlag, ld.TidyCompatibleVersion, eFlag, ld.GoVersion, compatFlag) + } else if suggestEFlag { + // If some packages are missing but no package is upgraded, then we + // shouldn't suggest upgrading to the Go 1.16 versions explicitly — that + // wouldn't actually fix anything for Go 1.16 users, and *would* break + // something for Go 1.17 users. + fmt.Fprintf(os.Stderr, "To proceed despite packages unresolved in go %s:\n\tgo mod tidy -e%s%s\n", ld.TidyCompatibleVersion, goFlag, compatFlag) + } + + fmt.Fprintf(os.Stderr, "If reproducibility with go %s is not needed:\n\tgo mod tidy%s -compat=%s\n", ld.TidyCompatibleVersion, goFlag, ld.GoVersion) + + // TODO(#46141): Populate the linked wiki page. + fmt.Fprintf(os.Stderr, "For other options, see:\n\thttps://golang.org/wiki/PruningModules\n") + } + + mg, err := rs.Graph(ctx) + if err != nil { + ld.errorf("go mod tidy: error loading go %s module graph: %v\n", ld.TidyCompatibleVersion, err) + suggestFixes() + return + } + + // Re-resolve packages in parallel. + // + // We re-resolve each package — rather than just checking versions — to ensure + // that we have fetched module source code (and, importantly, checksums for + // that source code) for all modules that are necessary to ensure that imports + // are unambiguous. That also produces clearer diagnostics, since we can say + // exactly what happened to the package if it became ambiguous or disappeared + // entirely. + // + // We re-resolve the packages in parallel because this process involves disk + // I/O to check for package sources, and because the process of checking for + // ambiguous imports may require us to download additional modules that are + // otherwise pruned out in Go 1.17 — we don't want to block progress on other + // packages while we wait for a single new download. + type mismatch struct { + mod module.Version + err error + } + mismatchMu := make(chan map[*loadPkg]mismatch, 1) + mismatchMu <- map[*loadPkg]mismatch{} + for _, pkg := range ld.pkgs { + if pkg.mod.Path == "" && pkg.err == nil { + // This package is from the standard library (which does not vary based on + // the module graph). + continue + } + + pkg := pkg + ld.work.Add(func() { + mod, _, err := importFromModules(ctx, pkg.path, rs, mg) + if mod != pkg.mod { + mismatches := <-mismatchMu + mismatches[pkg] = mismatch{mod: mod, err: err} + mismatchMu <- mismatches + } + }) + } + <-ld.work.Idle() + + mismatches := <-mismatchMu + if len(mismatches) == 0 { + // Since we're running as part of 'go mod tidy', the roots of the module + // graph should contain only modules that are relevant to some package in + // the package graph. We checked every package in the package graph and + // didn't find any mismatches, so that must mean that all of the roots of + // the module graph are also consistent. + // + // If we're wrong, Go 1.16 in -mod=readonly mode will error out with + // "updates to go.mod needed", which would be very confusing. So instead, + // we'll double-check that our reasoning above actually holds — if it + // doesn't, we'll emit an internal error and hopefully the user will report + // it as a bug. + for _, m := range ld.requirements.rootModules { + if v := mg.Selected(m.Path); v != m.Version { + fmt.Fprintln(os.Stderr) + base.Fatalf("go: internal error: failed to diagnose selected-version mismatch for module %s: go %s selects %s, but go %s selects %s\n\tPlease report this at https://golang.org/issue.", m.Path, ld.GoVersion, m.Version, ld.TidyCompatibleVersion, v) + } + } + return + } + + // Iterate over the packages (instead of the mismatches map) to emit errors in + // deterministic order. + for _, pkg := range ld.pkgs { + mismatch, ok := mismatches[pkg] + if !ok { + continue + } + + if pkg.isTest() { + // We already did (or will) report an error for the package itself, + // so don't report a duplicate (and more vebose) error for its test. + if _, ok := mismatches[pkg.testOf]; !ok { + base.Fatalf("go: internal error: mismatch recorded for test %s, but not its non-test package", pkg.path) + } + continue + } + + switch { + case mismatch.err != nil: + // pkg resolved successfully, but errors out using the requirements in rs. + // + // This could occur because the import is provided by a single lazy root + // (and is thus unambiguous in lazy mode) and also one or more + // transitive dependencies (and is ambiguous in eager mode). + // + // It could also occur because some transitive dependency upgrades the + // module that previously provided the package to a version that no + // longer does, or to a version for which the module source code (but + // not the go.mod file in isolation) has a checksum error. + if missing := (*ImportMissingError)(nil); errors.As(mismatch.err, &missing) { + selected := module.Version{ + Path: pkg.mod.Path, + Version: mg.Selected(pkg.mod.Path), + } + ld.errorf("%s loaded from %v,\n\tbut go %s would fail to locate it in %s\n", pkg.stackText(), pkg.mod, ld.TidyCompatibleVersion, selected) + } else { + if ambiguous := (*AmbiguousImportError)(nil); errors.As(mismatch.err, &ambiguous) { + // TODO: Is this check needed? + } + ld.errorf("%s loaded from %v,\n\tbut go %s would fail to locate it:\n\t%v\n", pkg.stackText(), pkg.mod, ld.TidyCompatibleVersion, mismatch.err) + } + + suggestEFlag = true + + // Even if we press ahead with the '-e' flag, the older version will + // error out in readonly mode if it thinks the go.mod file contains + // any *explicit* dependency that is not at its selected version, + // even if that dependency is not relevant to any package being loaded. + // + // We check for that condition here. If all of the roots are consistent + // the '-e' flag suffices, but otherwise we need to suggest an upgrade. + if !suggestUpgrade { + for _, m := range ld.requirements.rootModules { + if v := mg.Selected(m.Path); v != m.Version { + suggestUpgrade = true + break + } + } + } + + case pkg.err != nil: + // pkg had an error in lazy mode (presumably suppressed with the -e flag), + // but not in eager mode. + // + // This is possible, if, say, the import is unresolved in lazy mode + // (because the "latest" version of each candidate module either is + // unavailable or does not contain the package), but is resolved in + // eager mode due to a newer-than-latest dependency that is normally + // runed out of the module graph. + // + // This could also occur if the source code for the module providing the + // package in lazy mode has a checksum error, but eager mode upgrades + // that module to a version with a correct checksum. + // + // pkg.err should have already been logged elsewhere — along with a + // stack trace — so log only the import path and non-error info here. + suggestUpgrade = true + ld.errorf("%s failed to load from any module,\n\tbut go %s would load it from %v\n", pkg.path, ld.TidyCompatibleVersion, mismatch.mod) + + case pkg.mod != mismatch.mod: + // The package is loaded successfully by both Go versions, but from a + // different module in each. This could lead to subtle (and perhaps even + // unnoticed!) variations in behavior between builds with different + // toolchains. + suggestUpgrade = true + ld.errorf("%s loaded from %v,\n\tbut go %s would select %v\n", pkg.stackText(), pkg.mod, ld.TidyCompatibleVersion, mismatch.mod.Version) + + default: + base.Fatalf("go: internal error: mismatch recorded for package %s, but no differences found", pkg.path) + } + } + + suggestFixes() + base.ExitIfErrors() +} + // scanDir is like imports.ScanDir but elides known magic imports from the list, // so that we do not go looking for packages that don't really exist. // diff --git a/src/cmd/go/testdata/script/mod_tidy_compat.txt b/src/cmd/go/testdata/script/mod_tidy_compat.txt index e6f88dc779..e6edef5ee3 100644 --- a/src/cmd/go/testdata/script/mod_tidy_compat.txt +++ b/src/cmd/go/testdata/script/mod_tidy_compat.txt @@ -33,31 +33,26 @@ cmp go.mod go.mod.orig go list -m all cmp stdout m_all.txt +go mod edit -go=1.16 +go list -m all +cmp stdout m_all.txt + + +# If we explicitly drop compatibility with 1.16, we retain fewer checksums, +# which gives a cleaner go.sum file but causes 1.16 to fail in readonly mode. + +cp go.mod.orig go.mod +go mod tidy -compat=1.17 +cmp go.mod go.mod.orig + +go list -m all +cmp stdout m_all.txt + go mod edit -go=1.16 ! go list -m all stderr '^go list -m: example.net/lazy@v0.1.0 requires\n\texample.com/version@v1.0.1: missing go.sum entry; to add it:\n\tgo mod download example.com/version$' -# If we combine a Go 1.16 go.sum file... -go mod tidy -go=1.16 - -# ...with a Go 1.17 go.mod file... -cp go.mod.orig go.mod - -# ...then Go 1.17 continues to work... -go list -m all -cmp stdout m_all.txt - -# ...and now 1.16 can load the same build list! -go mod edit -go=1.16 -go list -m all -cmp stdout m_all.txt - - -# TODO(#46141): Add a cleaner way to tidy a Go 1.17 module while preserving -# the checksums needed to work within it with Go 1.16. - - -- go.mod -- // Module m happens to have the exact same build list as what would be // selected under Go 1.16, but computes that build list without looking at diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_added.txt b/src/cmd/go/testdata/script/mod_tidy_compat_added.txt new file mode 100644 index 0000000000..94fa79bc9f --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat_added.txt @@ -0,0 +1,105 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + + +# For this module, Go 1.17 produces an error for one module, and Go 1.16 +# produces a different error for a different module. + +cp go.mod go.mod.orig + +! go mod tidy + +stderr '^example\.com/m imports\n\texample\.net/added: module example\.net/added@latest found \(v0\.3\.0, replaced by \./a1\), but does not contain package example\.net/added$' + +cmp go.mod go.mod.orig + + +# When we run 'go mod tidy -e', we should proceed past the first error and follow +# it with a second error describing the version descrepancy. +# +# We should not provide advice on how to push past the version descrepancy, +# because the '-e' flag should already do that, writing out an otherwise-tidied +# go.mod file. + +go mod tidy -e + +stderr '^example\.com/m imports\n\texample\.net/added: module example\.net/added@latest found \(v0\.3\.0, replaced by \./a1\), but does not contain package example\.net/added\nexample\.net/added failed to load from any module,\n\tbut go 1\.16 would load it from example\.net/added@v0\.2\.0$' + +! stderr '\n\tgo mod tidy' + +cmp go.mod go.mod.tidy + + +-- go.mod -- +module example.com/m + +go 1.17 + +replace ( + example.net/added v0.1.0 => ./a1 + example.net/added v0.2.0 => ./a2 + example.net/added v0.3.0 => ./a1 + example.net/lazy v0.1.0 => ./lazy + example.net/pruned v0.1.0 => ./pruned +) + +require ( + example.net/added v0.1.0 + example.net/lazy v0.1.0 +) +-- go.mod.tidy -- +module example.com/m + +go 1.17 + +replace ( + example.net/added v0.1.0 => ./a1 + example.net/added v0.2.0 => ./a2 + example.net/added v0.3.0 => ./a1 + example.net/lazy v0.1.0 => ./lazy + example.net/pruned v0.1.0 => ./pruned +) + +require example.net/lazy v0.1.0 +-- m.go -- +package m + +import ( + _ "example.net/added" + _ "example.net/lazy" +) + +-- a1/go.mod -- +module example.net/added + +go 1.17 +-- a2/go.mod -- +module example.net/added + +go 1.17 +-- a2/added.go -- +package added + +-- lazy/go.mod -- +module example.net/lazy + +go 1.17 + +require example.net/pruned v0.1.0 +-- lazy/lazy.go -- +package lazy + +-- pruned/go.mod -- +module example.net/pruned + +go 1.17 + +require example.net/added v0.2.0 diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_ambiguous.txt b/src/cmd/go/testdata/script/mod_tidy_compat_ambiguous.txt new file mode 100644 index 0000000000..ed1dd53eff --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat_ambiguous.txt @@ -0,0 +1,98 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + +# For this module, the dependency providing package +# example.net/ambiguous/nested/pkg is unambiguous in Go 1.17 (because only one +# root of the module graph contains the package), whereas it is ambiguous in +# Go 1.16 (because two different modules contain plausible packages and Go 1.16 +# does not privilege roots above other dependencies). +# +# However, the overall build list is identical for both versions. + +cp go.mod go.mod.orig + +! go mod tidy + +stderr '^example\.com/m imports\n\texample\.net/indirect imports\n\texample\.net/ambiguous/nested/pkg loaded from example\.net/ambiguous/nested@v0\.1\.0,\n\tbut go 1.16 would fail to locate it:\n\tambiguous import: found package example\.net/ambiguous/nested/pkg in multiple modules:\n\texample\.net/ambiguous v0.1.0 \(.*\)\n\texample\.net/ambiguous/nested v0.1.0 \(.*\)\n\n' + +stderr '\n\nTo proceed despite packages unresolved in go 1\.16:\n\tgo mod tidy -e\nIf reproducibility with go 1.16 is not needed:\n\tgo mod tidy -compat=1\.17\nFor other options, see:\n\thttps://golang\.org/wiki/PruningModules\n' + +cmp go.mod go.mod.orig + + +# If we run 'go mod tidy -e', we should still save enough checksums to run +# 'go list -m all' reproducibly with go 1.16, even though we can't list +# the specific package. + +go mod tidy -e +! stderr '\n\tgo mod tidy' +cmp go.mod go.mod.orig + +go list -m all +cmp stdout all-m.txt + +go list -f $MODFMT example.net/ambiguous/nested/pkg +stdout '^example.net/ambiguous/nested v0\.1\.0$' +! stderr . + +go mod edit -go=1.16 +go list -m all +cmp stdout all-m.txt + +! go list -f $MODFMT example.net/ambiguous/nested/pkg +stderr '^ambiguous import: found package example\.net/ambiguous/nested/pkg in multiple modules:\n\texample\.net/ambiguous v0\.1\.0 \(.*\)\n\texample\.net/ambiguous/nested v0\.1\.0 \(.*\)\n' + + +# On the other hand, if we use -compat=1.17, 1.16 can't even load +# the build list (due to missing checksums). + +cp go.mod.orig go.mod +go mod tidy -compat=1.17 +! stderr . +go list -m all +cmp stdout all-m.txt + +go mod edit -go=1.16 +! go list -m all +stderr '^go list -m: example\.net/indirect@v0\.1\.0 requires\n\texample\.net/ambiguous@v0\.1\.0: missing go\.sum entry; to add it:\n\tgo mod download example\.net/ambiguous\n' + + +-- go.mod -- +module example.com/m + +go 1.17 + +replace example.net/indirect v0.1.0 => ./indirect + +require ( + example.net/ambiguous/nested v0.1.0 // indirect + example.net/indirect v0.1.0 +) +-- all-m.txt -- +example.com/m +example.net/ambiguous v0.1.0 +example.net/ambiguous/nested v0.1.0 +example.net/indirect v0.1.0 => ./indirect +-- m.go -- +package m + +import _ "example.net/indirect" + +-- indirect/go.mod -- +module example.net/indirect + +go 1.17 + +require example.net/ambiguous v0.1.0 +-- indirect/indirect.go -- +package indirect + +import _ "example.net/ambiguous/nested/pkg" diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_deleted.txt b/src/cmd/go/testdata/script/mod_tidy_compat_deleted.txt new file mode 100644 index 0000000000..3aacde2025 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_compat_deleted.txt @@ -0,0 +1,128 @@ +# https://golang.org/issue/46141: 'go mod tidy' for a Go 1.17 module should by +# default preserve enough checksums for the module to be used by Go 1.16. +# +# We don't have a copy of Go 1.16 handy, but we can simulate it by editing the +# 'go' version in the go.mod file to 1.16, without actually updating the +# requirements to match. + +[short] skip + +env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' + + +# For this module, the "deleted" dependency contains an imported package, but +# Go 1.16 selects a higher version (in which that package has been deleted). + +cp go.mod go.mod.orig + +! go mod tidy + +stderr '^example\.com/m imports\n\texample\.net/deleted loaded from example\.net/deleted@v0\.1\.0,\n\tbut go 1\.16 would fail to locate it in example\.net/deleted@v0\.2\.0\n\n' + +stderr '\n\nTo upgrade to the versions selected by go 1.16, leaving some packages unresolved:\n\tgo mod tidy -e -go=1\.16 && go mod tidy -e -go=1\.17\nIf reproducibility with go 1.16 is not needed:\n\tgo mod tidy -compat=1\.17\nFor other options, see:\n\thttps://golang\.org/wiki/PruningModules\n' + + +# The suggested 'go mod tidy -e' command should proceed anyway. + +go mod tidy -e +cmp go.mod go.mod.tidy + + +# In 'go 1.16' mode we should error out in the way we claimed. + +cd 116-outside +! go list -deps -f $MODFMT example.com/m +stderr '^\.\.[/\\]m\.go:4:2: no required module provides package example\.net/deleted; to add it:\n\tgo get example\.net/deleted$' +cd .. + +go mod edit -go=1.16 +! go list -deps -f $MODFMT example.com/m +stderr '^go: updates to go\.mod needed; to update it:\n\tgo mod tidy$' + +! go mod tidy +stderr '^example\.com/m imports\n\texample\.net/deleted: module example\.net/deleted@latest found \(v0\.2\.0, replaced by \./d2\), but does not contain package example\.net/deleted$' + + +-- go.mod -- +module example.com/m + +go 1.17 + +replace ( + example.net/deleted v0.1.0 => ./d1 + example.net/deleted v0.2.0 => ./d2 + example.net/lazy v0.1.0 => ./lazy + example.net/pruned v0.1.0 => ./pruned +) + +require ( + example.net/deleted v0.1.0 + example.net/deleted v0.1.0 // redundant + example.net/lazy v0.1.0 +) +-- go.mod.tidy -- +module example.com/m + +go 1.17 + +replace ( + example.net/deleted v0.1.0 => ./d1 + example.net/deleted v0.2.0 => ./d2 + example.net/lazy v0.1.0 => ./lazy + example.net/pruned v0.1.0 => ./pruned +) + +require ( + example.net/deleted v0.1.0 + example.net/lazy v0.1.0 +) +-- 116-outside/go.mod -- +module outside + +go 1.16 + +replace ( + example.com/m => ../ + example.net/deleted v0.1.0 => ../d1 + example.net/deleted v0.2.0 => ../d2 + example.net/lazy v0.1.0 => ../lazy + example.net/pruned v0.1.0 => ../pruned +) + +require example.com/m v0.1.0 +-- m.go -- +package m + +import ( + _ "example.net/deleted" + _ "example.net/lazy" +) + +-- d1/go.mod -- +module example.net/deleted + +go 1.17 +-- d1/deleted.go -- +package deleted +-- d2/go.mod -- +module example.net/deleted + +go 1.17 +-- d2/README -- +There is no longer a Go package here. + +-- lazy/go.mod -- +module example.net/lazy + +go 1.17 + +require example.net/pruned v0.1.0 +-- lazy/lazy.go -- +package lazy + +-- pruned/go.mod -- +module example.net/pruned + +go 1.17 + +require example.net/deleted v0.2.0 diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt b/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt index baaa5d63e3..e00aea930e 100644 --- a/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt +++ b/src/cmd/go/testdata/script/mod_tidy_compat_implicit.txt @@ -27,8 +27,22 @@ env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' # it — lazy v0.1.0 — specifies 'go 1.17', and it is not otherwise relevant to # the main module). +# 'go mod tidy' should by default diagnose the difference in dependencies as an +# error, with useful suggestions about how to resolve it. + cp go.mod go.mod.orig -go mod tidy +! go mod tidy +stderr '^example\.com/m imports\n\texample\.net/lazy tested by\n\texample\.net/lazy.test imports\n\texample\.com/retract/incompatible loaded from example\.com/retract/incompatible@v1\.0\.0,\n\tbut go 1\.16 would select v2\.0\.0\+incompatible\n\n' +stderr '\n\nTo upgrade to the versions selected by go 1\.16:\n\tgo mod tidy -go=1\.16 && go mod tidy -go=1\.17\nIf reproducibility with go 1.16 is not needed:\n\tgo mod tidy -compat=1.17\nFor other options, see:\n\thttps://golang\.org/wiki/PruningModules\n' + +cmp go.mod go.mod.orig + +# The suggested '-compat' flag to ignore differences should silence the error +# and leave go.mod unchanged, resulting in checksum errors when Go 1.16 tries +# to load a module pruned out by Go 1.17. + +go mod tidy -compat=1.17 +! stderr . cmp go.mod go.mod.orig go list -deps -test -f $MODFMT all @@ -49,7 +63,7 @@ cp go.mod.orig go.mod # ...then Go 1.17 no longer works. 😞 ! go list -deps -test -f $MODFMT all -stderr -count=1 '^can''t load test package: lazy/lazy_test.go:3:8: missing go\.sum entry for module providing package example\.com/retract/incompatible \(imported by example\.net/lazy\); to add:\n\tgo get -t example.net/lazy@v0\.1\.0$' +stderr -count=1 '^can''t load test package: lazy[/\\]lazy_test.go:3:8: missing go\.sum entry for module providing package example\.com/retract/incompatible \(imported by example\.net/lazy\); to add:\n\tgo get -t example.net/lazy@v0\.1\.0$' # However, if we take the union of the go.sum files... @@ -67,11 +81,6 @@ go list -deps -test -f $MODFMT all stdout '^example\.com/retract/incompatible v2\.0\.0\+incompatible$' -# TODO(#46100): In compatibility mode, should we reject the above difference as -# incompatible, or save checksums for both possible versions of the test -# dependency? - - -- go.mod -- // Module m imports packages from the same versions under Go 1.17 // as under Go 1.16, but under 1.16 its (implicit) external test dependencies diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt b/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt index 0861352408..2d8726544a 100644 --- a/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt +++ b/src/cmd/go/testdata/script/mod_tidy_compat_incompatible.txt @@ -27,12 +27,23 @@ env MODFMT='{{with .Module}}{{.Path}} {{.Version}}{{end}}' # the main module). -# TODO(#46141): 'go mod tidy' should by default diagnose the difference in -# dependencies as an error, but it should still be possible to simply drop -# compatibility with Go 1.16 by passing an appropriate '-compat' flag. +# 'go mod tidy' should by default diagnose the difference in dependencies as an +# error, with useful suggestions about how to resolve it. cp go.mod go.mod.orig -go mod tidy +! go mod tidy +stderr '^example\.com/m imports\n\texample\.net/lazy imports\n\texample\.com/retract/incompatible loaded from example\.com/retract/incompatible@v1\.0\.0,\n\tbut go 1\.16 would select v2\.0\.0\+incompatible\n\n' +stderr '\n\nTo upgrade to the versions selected by go 1\.16:\n\tgo mod tidy -go=1\.16 && go mod tidy -go=1\.17\nIf reproducibility with go 1\.16 is not needed:\n\tgo mod tidy -compat=1.17\nFor other options, see:\n\thttps://golang\.org/wiki/PruningModules\n' + +cmp go.mod go.mod.orig + + +# The suggested '-compat' flag to ignore differences should silence the error +# and leave go.mod unchanged, resulting in checksum errors when Go 1.16 tries +# to load a module pruned out by Go 1.17. + +go mod tidy -compat=1.17 +! stderr . cmp go.mod go.mod.orig go mod edit -go=1.16 @@ -44,6 +55,7 @@ stderr -count=2 '^go: example\.net/lazy@v0\.1\.0 requires\n\texample\.net/requir # There are two ways for the module author to bring the two into alignment. # One is to *explicitly* 'exclude' the version that is already *implicitly* # pruned out under 1.17. + go mod edit -exclude=example.com/retract/incompatible@v2.0.0+incompatible go list -f $MODFMT -deps ./... stdout '^example.com/retract/incompatible v1\.0\.0$' @@ -51,13 +63,20 @@ stdout '^example.com/retract/incompatible v1\.0\.0$' # The other is to explicitly upgrade the version required under Go 1.17 -# to match the version selected by Go 1.16. +# to match the version selected by Go 1.16. The commands suggested by +# 'go mod tidy' should do exactly that. + cp go.mod.orig go.mod -go get -d example.com/retract/incompatible@v2.0.0+incompatible - # Note that we are not running 'go mod tidy' here: we need to preserve - # the checksum for v1.0.0 because it is also still in the module graph - # as seen by Go 1.16. +go mod tidy -go=1.16 +go list -f $MODFMT -deps ./... +stdout '^example.com/retract/incompatible v2\.0\.0\+incompatible$' +! stdout 'v1\.0\.0' + +go mod tidy -go=1.17 +go list -f $MODFMT -deps ./... +stdout '^example.com/retract/incompatible v2\.0\.0\+incompatible$' +! stdout 'v1\.0\.0' go mod edit -go=1.16 go list -f $MODFMT -deps ./... diff --git a/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt b/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt index d4371e0f7d..7c22fca6c0 100644 --- a/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt +++ b/src/cmd/go/testdata/script/mod_tidy_compat_irrelevant.txt @@ -27,34 +27,28 @@ cp go.mod go.mod.orig go mod tidy cmp go.mod go.mod.orig -go list -m all -stdout '^example\.com/retract/incompatible v1\.0\.0$' - -go mod edit -go=1.16 -! go list -deps -test -f $MODFMT all - # TODO(#46160): -count=1 instead of -count=2. -stderr -count=2 '^go: example.net/lazy@v0.1.0 requires\n\texample.com/retract/incompatible@v1.0.0: missing go.sum entry; to add it:\n\tgo mod download example.com/retract/incompatible$' - - -# If we combine a Go 1.16 go.sum file... -go mod tidy -go=1.16 - -# ...with a Go 1.17 go.mod file... -cp go.mod.orig go.mod - -# ...then Go 1.17 continues to work... go list -deps -test -f $MODFMT all cp stdout out-117.txt -# ...and 1.16 also works, and selects the same versions for all packages -# even remotely relevant to the main module. go mod edit -go=1.16 go list -deps -test -f $MODFMT all cmp stdout out-117.txt -# TODO(#46160): Add a cleaner way to tidy a Go 1.17 module while preserving -# the checksums needed to work within it with Go 1.16. +# If we explicitly drop compatibility with 1.16, we retain fewer checksums, +# which gives a cleaner go.sum file but causes 1.16 to fail in readonly mode. + +cp go.mod.orig go.mod +go mod tidy -compat=1.17 +cmp go.mod go.mod.orig + +go list -deps -test -f $MODFMT all +cmp stdout out-117.txt + +go mod edit -go=1.16 +! go list -deps -test -f $MODFMT all + # TODO(#46160): -count=1 instead of -count=2. +stderr -count=2 '^go: example.net/lazy@v0.1.0 requires\n\texample.com/retract/incompatible@v1.0.0: missing go.sum entry; to add it:\n\tgo mod download example.com/retract/incompatible$' -- go.mod -- diff --git a/src/cmd/go/testdata/script/mod_tidy_oldgo.txt b/src/cmd/go/testdata/script/mod_tidy_oldgo.txt new file mode 100644 index 0000000000..0e88b068a7 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_tidy_oldgo.txt @@ -0,0 +1,21 @@ +# Modules were introduced in Go 1.11, but for various reasons users may +# decide to declare a (much!) older go version in their go.mod file. +# Modules with very old versions should not be rejected, and should have +# the same module-graph semantics as in Go 1.11. + +cp go.mod go.mod.orig +go mod tidy +cmp go.mod go.mod.orig + +-- go.mod -- +module example.com/legacy/go1 + +go 1.0 + +require golang.org/x/text v0.3.0 +-- main.go -- +package main + +import _ "golang.org/x/text/language" + +func main() {} diff --git a/src/go.sum b/src/go.sum index b3de6c526c..6e869b96f7 100644 --- a/src/go.sum +++ b/src/go.sum @@ -1,8 +1,15 @@ golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e h1:8foAy0aoO5GkqCvAEJ4VC4P3zksTg4X4aJCDpZzmgQI= golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210510120150-4163338589ed h1:p9UgmWI9wKpfYmgaV/IZKGdXc5qEK45tDwwwDyjS26I= golang.org/x/net v0.0.0-20210510120150-4163338589ed/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 h1:yhBbb4IRs2HS9PPlAg6DMC6mUOKexJBNsLf4Z+6En1Q= golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f h1:yQJrRE0hDxDFmZLlRaw+3vusO4fwNHgHIjUOMO7bHYI= golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= From f22ec51debeddc0903096e66bfaf641568bede3b Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Sat, 22 May 2021 13:40:20 -0700 Subject: [PATCH 40/40] doc: add Go 1.17 release note about inlining functions with closures Fixes #45781 Change-Id: Ia5bc2845f7f94aff4f3f0ff15533feb148223adb Reviewed-on: https://go-review.googlesource.com/c/go/+/322089 Trust: Dan Scales Reviewed-by: Cherry Mui --- doc/go1.17.html | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 6dd1d0d1db..c2317a4035 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -228,7 +228,14 @@ Do not send CLs removing the interior tags from such phrases.

Compiler

-

+

+ + Functions containing closures can now be inlined. One effect of this change is + that a function with a closure may actually produce a distinct closure function + for each place that the function is inlined. Hence, this change could reveal + bugs where Go functions are compared (incorrectly) by pointer value. Go + functions are by definition not comparable. + TODO: complete the Compiler section, or delete if not needed