From 072c7d4969862c84430cc2daef20a8f7f3ba78a2 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 19 Apr 2022 19:41:40 -0400 Subject: [PATCH] cmd/compile,cmd/link: hooks for identifying coverage counters Add a new "coverage counter" classification for variables to be used for storing code coverage counter values (somewhat in the same way that we identify fuzzer counters). Tagging such variables allows us to aggregate them in the linker, and to treat updates specially. Updates #51430. Change-Id: Ib49fb05736ffece98bcc2f7a7c37e991b7f67bbb Reviewed-on: https://go-review.googlesource.com/c/go/+/401235 Reviewed-by: David Chase Run-TryBot: Than McIntosh TryBot-Result: Gopher Robot --- src/cmd/compile/internal/gc/obj.go | 6 +++ src/cmd/compile/internal/inline/inl.go | 23 ++++++++++ src/cmd/compile/internal/ir/name.go | 6 +++ src/cmd/compile/internal/ssa/writebarrier.go | 3 +- src/cmd/internal/objabi/symkind.go | 5 ++- src/cmd/internal/objabi/symkind_string.go | 6 ++- src/cmd/link/internal/ld/data.go | 13 ++++++ src/cmd/link/internal/ld/symtab.go | 6 ++- src/cmd/link/internal/sym/symkind.go | 4 ++ src/cmd/link/internal/sym/symkind_string.go | 46 ++++++++++---------- src/runtime/symtab.go | 1 + 11 files changed, 92 insertions(+), 27 deletions(-) diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index 715b8ee263..504072bb17 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -195,6 +195,9 @@ func dumpGlobal(n *ir.Name) { } types.CalcSize(n.Type()) ggloblnod(n) + if n.CoverageCounter() || n.CoverageAuxVar() { + return + } base.Ctxt.DwarfGlobal(base.Ctxt.Pkgpath, types.TypeSymName(n.Type()), n.Linksym()) } @@ -316,6 +319,9 @@ func ggloblnod(nam *ir.Name) { if nam.Libfuzzer8BitCounter() { s.Type = objabi.SLIBFUZZER_8BIT_COUNTER } + if nam.CoverageCounter() { + s.Type = objabi.SCOVERAGE_COUNTER + } if nam.Sym().Linkname != "" { // Make sure linkname'd symbol is non-package. When a symbol is // both imported and linkname'd, s.Pkg may not set to "_" in diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index a7fd704b85..14adbf5d43 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -37,6 +37,7 @@ import ( "cmd/compile/internal/typecheck" "cmd/compile/internal/types" "cmd/internal/obj" + "cmd/internal/objabi" "cmd/internal/src" ) @@ -471,6 +472,28 @@ func (v *hairyVisitor) doNode(n ir.Node) bool { } } } + + case ir.OAS: + // Special case for coverage counter updates and coverage + // function registrations. Although these correspond to real + // operations, we treat them as zero cost for the moment. This + // is primarily due to the existence of tests that are + // sensitive to inlining-- if the insertion of coverage + // instrumentation happens to tip a given function over the + // threshold and move it from "inlinable" to "not-inlinable", + // this can cause changes in allocation behavior, which can + // then result in test failures (a good example is the + // TestAllocations in crypto/ed25519). + n := n.(*ir.AssignStmt) + if n.X.Op() == ir.OINDEX { + n := n.X.(*ir.IndexExpr) + if n.X.Op() == ir.ONAME && n.X.Type().IsArray() { + n := n.X.(*ir.Name) + if n.Linksym().Type == objabi.SCOVERAGE_COUNTER { + return false + } + } + } } v.budget-- diff --git a/src/cmd/compile/internal/ir/name.go b/src/cmd/compile/internal/ir/name.go index 310481f6f0..f537ba4981 100644 --- a/src/cmd/compile/internal/ir/name.go +++ b/src/cmd/compile/internal/ir/name.go @@ -236,6 +236,8 @@ const ( nameInlLocal // PAUTO created by inliner, derived from callee local nameOpenDeferSlot // if temporary var storing info for open-coded defers nameLibfuzzer8BitCounter // if PEXTERN should be assigned to __sancov_cntrs section + nameCoverageCounter // instrumentation counter var for cmd/cover + nameCoverageAuxVar // instrumentation pkg ID variable cmd/cover nameAlias // is type name an alias ) @@ -251,6 +253,8 @@ func (n *Name) InlFormal() bool { return n.flags&nameInlFormal != func (n *Name) InlLocal() bool { return n.flags&nameInlLocal != 0 } func (n *Name) OpenDeferSlot() bool { return n.flags&nameOpenDeferSlot != 0 } func (n *Name) Libfuzzer8BitCounter() bool { return n.flags&nameLibfuzzer8BitCounter != 0 } +func (n *Name) CoverageCounter() bool { return n.flags&nameCoverageCounter != 0 } +func (n *Name) CoverageAuxVar() bool { return n.flags&nameCoverageAuxVar != 0 } func (n *Name) setReadonly(b bool) { n.flags.set(nameReadonly, b) } func (n *Name) SetNeedzero(b bool) { n.flags.set(nameNeedzero, b) } @@ -264,6 +268,8 @@ func (n *Name) SetInlFormal(b bool) { n.flags.set(nameInlFormal, func (n *Name) SetInlLocal(b bool) { n.flags.set(nameInlLocal, b) } func (n *Name) SetOpenDeferSlot(b bool) { n.flags.set(nameOpenDeferSlot, b) } func (n *Name) SetLibfuzzer8BitCounter(b bool) { n.flags.set(nameLibfuzzer8BitCounter, b) } +func (n *Name) SetCoverageCounter(b bool) { n.flags.set(nameCoverageCounter, b) } +func (n *Name) SetCoverageAuxVar(b bool) { n.flags.set(nameCoverageAuxVar, b) } // OnStack reports whether variable n may reside on the stack. func (n *Name) OnStack() bool { diff --git a/src/cmd/compile/internal/ssa/writebarrier.go b/src/cmd/compile/internal/ssa/writebarrier.go index cb8c0a5e0e..f5a7ed5928 100644 --- a/src/cmd/compile/internal/ssa/writebarrier.go +++ b/src/cmd/compile/internal/ssa/writebarrier.go @@ -652,7 +652,8 @@ func IsSanitizerSafeAddr(v *Value) bool { // read-only once initialized. return true case OpAddr: - return v.Aux.(*obj.LSym).Type == objabi.SRODATA || v.Aux.(*obj.LSym).Type == objabi.SLIBFUZZER_8BIT_COUNTER + vt := v.Aux.(*obj.LSym).Type + return vt == objabi.SRODATA || vt == objabi.SLIBFUZZER_8BIT_COUNTER || vt == objabi.SCOVERAGE_COUNTER || vt == objabi.SCOVERAGE_AUXVAR } return false } diff --git a/src/cmd/internal/objabi/symkind.go b/src/cmd/internal/objabi/symkind.go index ba1e4d50d6..a58816e292 100644 --- a/src/cmd/internal/objabi/symkind.go +++ b/src/cmd/internal/objabi/symkind.go @@ -68,6 +68,9 @@ const ( SDWARFLINES // Coverage instrumentation counter for libfuzzer. SLIBFUZZER_8BIT_COUNTER - // Update cmd/link/internal/sym/AbiSymKindToSymKind for new SymKind values. + // Coverage instrumentation counter, aux variable for cmd/cover + SCOVERAGE_COUNTER + SCOVERAGE_AUXVAR + // Update cmd/link/internal/sym/AbiSymKindToSymKind for new SymKind values. ) diff --git a/src/cmd/internal/objabi/symkind_string.go b/src/cmd/internal/objabi/symkind_string.go index d0606aa2da..c0b84030f7 100644 --- a/src/cmd/internal/objabi/symkind_string.go +++ b/src/cmd/internal/objabi/symkind_string.go @@ -26,11 +26,13 @@ func _() { _ = x[SDWARFLOC-15] _ = x[SDWARFLINES-16] _ = x[SLIBFUZZER_8BIT_COUNTER-17] + _ = x[SCOVERAGE_COUNTER-18] + _ = x[SCOVERAGE_AUXVAR-19] } -const _SymKind_name = "SxxxSTEXTSRODATASNOPTRDATASDATASBSSSNOPTRBSSSTLSBSSSDWARFCUINFOSDWARFCONSTSDWARFFCNSDWARFABSFCNSDWARFTYPESDWARFVARSDWARFRANGESDWARFLOCSDWARFLINESSLIBFUZZER_8BIT_COUNTER" +const _SymKind_name = "SxxxSTEXTSRODATASNOPTRDATASDATASBSSSNOPTRBSSSTLSBSSSDWARFCUINFOSDWARFCONSTSDWARFFCNSDWARFABSFCNSDWARFTYPESDWARFVARSDWARFRANGESDWARFLOCSDWARFLINESSLIBFUZZER_8BIT_COUNTERSCOVERAGE_COUNTERSCOVERAGE_AUXVAR" -var _SymKind_index = [...]uint8{0, 4, 9, 16, 26, 31, 35, 44, 51, 63, 74, 83, 95, 105, 114, 125, 134, 145, 168} +var _SymKind_index = [...]uint8{0, 4, 9, 16, 26, 31, 35, 44, 51, 63, 74, 83, 95, 105, 114, 125, 134, 145, 168, 185, 201} func (i SymKind) String() string { if i >= SymKind(len(_SymKind_index)-1) { diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go index c2668b29a4..c23eac08a4 100644 --- a/src/cmd/link/internal/ld/data.go +++ b/src/cmd/link/internal/ld/data.go @@ -1074,6 +1074,8 @@ func dwarfblk(ctxt *Link, out *OutBuf, addr int64, size int64) { writeBlocks(ctxt, out, ctxt.outSem, ctxt.loader, syms, addr, size, zeros[:]) } +var covCounterDataStartOff, covCounterDataLen uint64 + var zeros [512]byte var ( @@ -1781,6 +1783,15 @@ func (state *dodataState) allocateDataSections(ctxt *Link) { ldr.SetSymSect(ldr.LookupOrCreateSym("runtime.enoptrbss", 0), sect) ldr.SetSymSect(ldr.LookupOrCreateSym("runtime.end", 0), sect) + // Code coverage counters are assigned to the .noptrbss section. + // We assign them in a separate pass so that they stay aggregated + // together in a single blob (coverage runtime depends on this). + covCounterDataStartOff = sect.Length + state.assignToSection(sect, sym.SCOVERAGE_COUNTER, sym.SNOPTRBSS) + covCounterDataLen = sect.Length - covCounterDataStartOff + ldr.SetSymSect(ldr.LookupOrCreateSym("runtime.covctrs", 0), sect) + ldr.SetSymSect(ldr.LookupOrCreateSym("runtime.ecovctrs", 0), sect) + // Coverage instrumentation counters for libfuzzer. if len(state.data[sym.SLIBFUZZER_8BIT_COUNTER]) > 0 { sect := state.allocateNamedSectionAndAssignSyms(&Segdata, "__sancov_cntrs", sym.SLIBFUZZER_8BIT_COUNTER, sym.Sxxx, 06) @@ -2682,6 +2693,8 @@ func (ctxt *Link) address() []*sym.Segment { ctxt.xdefine("runtime.edata", sym.SDATA, int64(data.Vaddr+data.Length)) ctxt.xdefine("runtime.noptrbss", sym.SNOPTRBSS, int64(noptrbss.Vaddr)) ctxt.xdefine("runtime.enoptrbss", sym.SNOPTRBSS, int64(noptrbss.Vaddr+noptrbss.Length)) + ctxt.xdefine("runtime.covctrs", sym.SCOVERAGE_COUNTER, int64(noptrbss.Vaddr+covCounterDataStartOff)) + ctxt.xdefine("runtime.ecovctrs", sym.SCOVERAGE_COUNTER, int64(noptrbss.Vaddr+covCounterDataStartOff+covCounterDataLen)) ctxt.xdefine("runtime.end", sym.SBSS, int64(Segdata.Vaddr+Segdata.Length)) if fuzzCounters != nil { diff --git a/src/cmd/link/internal/ld/symtab.go b/src/cmd/link/internal/ld/symtab.go index 5074ffa8c9..21a1466c49 100644 --- a/src/cmd/link/internal/ld/symtab.go +++ b/src/cmd/link/internal/ld/symtab.go @@ -453,6 +453,8 @@ func (ctxt *Link) symtab(pcln *pclntab) []sym.SymKind { ctxt.xdefine("runtime.ebss", sym.SBSS, 0) ctxt.xdefine("runtime.noptrbss", sym.SNOPTRBSS, 0) ctxt.xdefine("runtime.enoptrbss", sym.SNOPTRBSS, 0) + ctxt.xdefine("runtime.covctrs", sym.SNOPTRBSS, 0) + ctxt.xdefine("runtime.ecovctrs", sym.SNOPTRBSS, 0) ctxt.xdefine("runtime.end", sym.SBSS, 0) ctxt.xdefine("runtime.epclntab", sym.SRODATA, 0) ctxt.xdefine("runtime.esymtab", sym.SRODATA, 0) @@ -529,7 +531,7 @@ func (ctxt *Link) symtab(pcln *pclntab) []sym.SymKind { nsym := loader.Sym(ldr.NSym()) symGroupType := make([]sym.SymKind, nsym) for s := loader.Sym(1); s < nsym; s++ { - if !ctxt.IsExternal() && ldr.IsFileLocal(s) && !ldr.IsFromAssembly(s) && ldr.SymPkg(s) != "" { + if (!ctxt.IsExternal() && ldr.IsFileLocal(s) && !ldr.IsFromAssembly(s) && ldr.SymPkg(s) != "") || (ctxt.LinkMode == LinkInternal && ldr.SymType(s) == sym.SCOVERAGE_COUNTER) { ldr.SetAttrNotInSymbolTable(s, true) } if !ldr.AttrReachable(s) || ldr.AttrSpecial(s) || (ldr.SymType(s) != sym.SRODATA && ldr.SymType(s) != sym.SGOFUNC) { @@ -672,6 +674,8 @@ func (ctxt *Link) symtab(pcln *pclntab) []sym.SymKind { moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.ebss", 0)) moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.noptrbss", 0)) moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.enoptrbss", 0)) + moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.covctrs", 0)) + moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.ecovctrs", 0)) moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.end", 0)) moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.gcdata", 0)) moduledata.AddAddr(ctxt.Arch, ldr.Lookup("runtime.gcbss", 0)) diff --git a/src/cmd/link/internal/sym/symkind.go b/src/cmd/link/internal/sym/symkind.go index 0f8fbed878..2f8e8fe133 100644 --- a/src/cmd/link/internal/sym/symkind.go +++ b/src/cmd/link/internal/sym/symkind.go @@ -98,6 +98,8 @@ const ( SBSS SNOPTRBSS SLIBFUZZER_8BIT_COUNTER + SCOVERAGE_COUNTER + SCOVERAGE_AUXVAR STLSBSS SXREF SMACHOSYMSTR @@ -144,6 +146,8 @@ var AbiSymKindToSymKind = [...]SymKind{ objabi.SDWARFLOC: SDWARFLOC, objabi.SDWARFLINES: SDWARFLINES, objabi.SLIBFUZZER_8BIT_COUNTER: SLIBFUZZER_8BIT_COUNTER, + objabi.SCOVERAGE_COUNTER: SCOVERAGE_COUNTER, + objabi.SCOVERAGE_AUXVAR: SCOVERAGE_AUXVAR, } // ReadOnly are the symbol kinds that form read-only sections. In some diff --git a/src/cmd/link/internal/sym/symkind_string.go b/src/cmd/link/internal/sym/symkind_string.go index 14b57db41f..1cd7ab17ef 100644 --- a/src/cmd/link/internal/sym/symkind_string.go +++ b/src/cmd/link/internal/sym/symkind_string.go @@ -45,31 +45,33 @@ func _() { _ = x[SBSS-34] _ = x[SNOPTRBSS-35] _ = x[SLIBFUZZER_8BIT_COUNTER-36] - _ = x[STLSBSS-37] - _ = x[SXREF-38] - _ = x[SMACHOSYMSTR-39] - _ = x[SMACHOSYMTAB-40] - _ = x[SMACHOINDIRECTPLT-41] - _ = x[SMACHOINDIRECTGOT-42] - _ = x[SFILEPATH-43] - _ = x[SDYNIMPORT-44] - _ = x[SHOSTOBJ-45] - _ = x[SUNDEFEXT-46] - _ = x[SDWARFSECT-47] - _ = x[SDWARFCUINFO-48] - _ = x[SDWARFCONST-49] - _ = x[SDWARFFCN-50] - _ = x[SDWARFABSFCN-51] - _ = x[SDWARFTYPE-52] - _ = x[SDWARFVAR-53] - _ = x[SDWARFRANGE-54] - _ = x[SDWARFLOC-55] - _ = x[SDWARFLINES-56] + _ = x[SCOVERAGE_COUNTER-37] + _ = x[SCOVERAGE_AUXVAR-38] + _ = x[STLSBSS-39] + _ = x[SXREF-40] + _ = x[SMACHOSYMSTR-41] + _ = x[SMACHOSYMTAB-42] + _ = x[SMACHOINDIRECTPLT-43] + _ = x[SMACHOINDIRECTGOT-44] + _ = x[SFILEPATH-45] + _ = x[SDYNIMPORT-46] + _ = x[SHOSTOBJ-47] + _ = x[SUNDEFEXT-48] + _ = x[SDWARFSECT-49] + _ = x[SDWARFCUINFO-50] + _ = x[SDWARFCONST-51] + _ = x[SDWARFFCN-52] + _ = x[SDWARFABSFCN-53] + _ = x[SDWARFTYPE-54] + _ = x[SDWARFVAR-55] + _ = x[SDWARFRANGE-56] + _ = x[SDWARFLOC-57] + _ = x[SDWARFLINES-58] } -const _SymKind_name = "SxxxSTEXTSELFRXSECTSMACHOPLTSTYPESSTRINGSGOSTRINGSGOFUNCSGCBITSSRODATASFUNCTABSELFROSECTSTYPERELROSSTRINGRELROSGOSTRINGRELROSGOFUNCRELROSGCBITSRELROSRODATARELROSFUNCTABRELROSTYPELINKSITABLINKSSYMTABSPCLNTABSFirstWritableSBUILDINFOSELFSECTSMACHOSMACHOGOTSWINDOWSSELFGOTSNOPTRDATASINITARRSDATASXCOFFTOCSBSSSNOPTRBSSSLIBFUZZER_8BIT_COUNTERSTLSBSSSXREFSMACHOSYMSTRSMACHOSYMTABSMACHOINDIRECTPLTSMACHOINDIRECTGOTSFILEPATHSDYNIMPORTSHOSTOBJSUNDEFEXTSDWARFSECTSDWARFCUINFOSDWARFCONSTSDWARFFCNSDWARFABSFCNSDWARFTYPESDWARFVARSDWARFRANGESDWARFLOCSDWARFLINES" +const _SymKind_name = "SxxxSTEXTSELFRXSECTSMACHOPLTSTYPESSTRINGSGOSTRINGSGOFUNCSGCBITSSRODATASFUNCTABSELFROSECTSTYPERELROSSTRINGRELROSGOSTRINGRELROSGOFUNCRELROSGCBITSRELROSRODATARELROSFUNCTABRELROSTYPELINKSITABLINKSSYMTABSPCLNTABSFirstWritableSBUILDINFOSELFSECTSMACHOSMACHOGOTSWINDOWSSELFGOTSNOPTRDATASINITARRSDATASXCOFFTOCSBSSSNOPTRBSSSLIBFUZZER_8BIT_COUNTERSCOVERAGE_COUNTERSCOVERAGE_AUXVARSTLSBSSSXREFSMACHOSYMSTRSMACHOSYMTABSMACHOINDIRECTPLTSMACHOINDIRECTGOTSFILEPATHSDYNIMPORTSHOSTOBJSUNDEFEXTSDWARFSECTSDWARFCUINFOSDWARFCONSTSDWARFFCNSDWARFABSFCNSDWARFTYPESDWARFVARSDWARFRANGESDWARFLOCSDWARFLINES" -var _SymKind_index = [...]uint16{0, 4, 9, 19, 28, 33, 40, 49, 56, 63, 70, 78, 88, 98, 110, 124, 136, 148, 160, 173, 182, 191, 198, 206, 220, 230, 238, 244, 253, 261, 268, 278, 286, 291, 300, 304, 313, 336, 343, 348, 360, 372, 389, 406, 415, 425, 433, 442, 452, 464, 475, 484, 496, 506, 515, 526, 535, 546} +var _SymKind_index = [...]uint16{0, 4, 9, 19, 28, 33, 40, 49, 56, 63, 70, 78, 88, 98, 110, 124, 136, 148, 160, 173, 182, 191, 198, 206, 220, 230, 238, 244, 253, 261, 268, 278, 286, 291, 300, 304, 313, 336, 353, 369, 376, 381, 393, 405, 422, 439, 448, 458, 466, 475, 485, 497, 508, 517, 529, 539, 548, 559, 568, 579} func (i SymKind) String() string { if i >= SymKind(len(_SymKind_index)-1) { diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index 79ca5cfc44..2da9a59b7e 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -427,6 +427,7 @@ type moduledata struct { data, edata uintptr bss, ebss uintptr noptrbss, enoptrbss uintptr + covctrs, ecovctrs uintptr end, gcdata, gcbss uintptr types, etypes uintptr rodata uintptr