[go] cmd/compile: enable Asan check for global variables
fannie zhang (Gerrit)
fannie zhang has uploaded this change for review.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/pkginit/init.go
M src/cmd/compile/internal/typecheck/subr.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
10 files changed, 279 insertions(+), 8 deletions(-)
diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go
index 0b10cb8..aabbdd9 100644
--- a/src/cmd/compile/internal/gc/obj.go
+++ b/src/cmd/compile/internal/gc/obj.go
@@ -275,7 +275,17 @@
if nam.Type() != nil && !nam.Type().HasPointers() {
flags |= obj.NOPTR
}
- base.Ctxt.Globl(s, nam.Type().Width, flags)
+ size := nam.Type().Width
+
+ if base.Flag.ASan && typecheck.ShouldInstrumentGlobals[s.Name] != nil {
+ // Write the new size of instrumented global variables that have
+ // trailing redzones into object file.
+ rzSize := typecheck.GetRedzoneSizeForGlobal(size)
+ sizeWithRZ := rzSize + size
+ base.Ctxt.Globl(s, sizeWithRZ, flags)
+ } else {
+ base.Ctxt.Globl(s, size, flags)
+ }
if nam.LibfuzzerExtraCounter() {
s.Type = objabi.SLIBFUZZER_EXTRA_COUNTER
}
diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go
index 4c7c9fc..7768392 100644
--- a/src/cmd/compile/internal/noder/noder.go
+++ b/src/cmd/compile/internal/noder/noder.go
@@ -549,7 +549,7 @@
if fun.Recv == nil {
if name.Name == "init" {
- name = renameinit()
+ name = Renameinit()
if len(t.Params) > 0 || len(t.Results) > 0 {
base.ErrorfAt(f.Pos(), "func init must have no arguments and no return values")
}
@@ -1809,7 +1809,7 @@
// the name, normally "pkg.init", is altered to "pkg.init.0".
var renameinitgen int
-func renameinit() *types.Sym {
+func Renameinit() *types.Sym {
s := typecheck.LookupNum("init.", renameinitgen)
renameinitgen++
return s
diff --git a/src/cmd/compile/internal/noder/object.go b/src/cmd/compile/internal/noder/object.go
index 82cce1a..fd1b0c3 100644
--- a/src/cmd/compile/internal/noder/object.go
+++ b/src/cmd/compile/internal/noder/object.go
@@ -84,7 +84,7 @@
var typ *types.Type
if recv := sig.Recv(); recv == nil {
if obj.Name() == "init" {
- sym = renameinit()
+ sym = Renameinit()
} else {
sym = g.sym(obj)
}
diff --git a/src/cmd/compile/internal/pkginit/init.go b/src/cmd/compile/internal/pkginit/init.go
index 7cad262..98acd63 100644
--- a/src/cmd/compile/internal/pkginit/init.go
+++ b/src/cmd/compile/internal/pkginit/init.go
@@ -8,6 +8,7 @@
"cmd/compile/internal/base"
"cmd/compile/internal/deadcode"
"cmd/compile/internal/ir"
+ "cmd/compile/internal/noder"
"cmd/compile/internal/objw"
"cmd/compile/internal/typecheck"
"cmd/compile/internal/types"
@@ -66,6 +67,57 @@
base.Fatalf("InitTodoFunc still has declarations")
}
typecheck.InitTodoFunc = nil
+ if base.Flag.ASan {
+ // Make a initialization function to call runtime.asanregisterglobals to register an
+ // array of instrumented global variables when -asan is enabled. An instrumented global
+ // variable is described by a structure.
+ // See the _asan_global structure declared in src/runtime/asan/asan.go.
+ //
+ // func init {
+ // var globals []_asan_global
+ // asanregisterglobals(globals, len(globals))
+ // }
+ for _, n := range typecheck.Target.Externs {
+ if typecheck.CanInstrumentGlobal(n) {
+ symName := n.(*ir.Name).Linksym().Name
+ typecheck.ShouldInstrumentGlobals[symName] = n
+ }
+ }
+ ni := len(typecheck.ShouldInstrumentGlobals)
+ if ni != 0 {
+ // Make an init._ function.
+ base.Pos = base.AutogeneratedPos
+ typecheck.DeclContext = ir.PEXTERN
+ name := noder.Renameinit()
+ fnInit := typecheck.DeclFunc(name, ir.NewFuncType(base.Pos, nil, nil, nil))
+
+ // Get an array of intrumented global variables.
+ globals := typecheck.InstrumentGlobals(fnInit)
+
+ // Call runtime.asanregisterglobals function to poison redzones.
+ // runtime.asanregisterglobals(unsafe.Pointer(&globals[0]), ni)
+ asanf := typecheck.NewName(ir.Pkgs.Runtime.Lookup("asanregisterglobals"))
+ ir.MarkFunc(asanf)
+ asanf.SetType(types.NewSignature(types.NoPkg, nil, nil, []*types.Field{
+ types.NewField(base.Pos, nil, types.Types[types.TUNSAFEPTR]),
+ types.NewField(base.Pos, nil, types.Types[types.TUINTPTR]),
+ }, nil))
+ asancall := ir.NewCallExpr(base.Pos, ir.OCALL, asanf, nil)
+ asancall.Args.Append(typecheck.ConvNop(typecheck.NodAddr(
+ ir.NewIndexExpr(base.Pos, globals, ir.NewInt(0))), types.Types[types.TUNSAFEPTR]))
+ asancall.Args.Append(typecheck.ConvNop(ir.NewInt(int64(ni)), types.Types[types.TUINTPTR]))
+
+ fnInit.Body.Append(asancall)
+ typecheck.FinishFuncBody()
+ typecheck.Func(fnInit)
+ ir.CurFunc = fnInit
+ typecheck.Stmts(fnInit.Body)
+ ir.CurFunc = nil
+
+ typecheck.Target.Decls = append(typecheck.Target.Decls, fnInit)
+ typecheck.Target.Inits = append(typecheck.Target.Inits, fnInit)
+ }
+ }
// Record user init functions.
for _, fn := range typecheck.Target.Inits {
diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go
index 9ee7a94..2d683b9 100644
--- a/src/cmd/compile/internal/typecheck/subr.go
+++ b/src/cmd/compile/internal/typecheck/subr.go
@@ -874,3 +874,161 @@
type symlink struct {
field *types.Field
}
+
+// createtypes creates the asanGlobal and defString struct type.
+// Go compiler does not refer to the C types, we represent the struct field
+// by a uintptr, then use type conversion to make copies of the data.
+// E.g., (*defString)(asanGlobal.name).data to C string.
+// Keep in sync with src/runtime/asan/asan.go.
+//
+// type asanGlobal struct {
+// beg uintptr
+// size uintptr
+// size_with_redzone uintptr
+// name uintptr
+// moduleName uintptr
+// hasDynamicInit uintptr
+// sourceLocation uintptr
+// odrIndicator uintptr
+// }
+//
+// defString is synthesized struct type meant to capture the underlying
+// implementations of string.
+// type defString struct {
+// data uintptr
+// len uintptr
+// }
+func createtypes() (*types.Type, *types.Type) {
+ up := types.Types[types.TUINTPTR]
+ fname := Lookup
+ nxp := src.NoXPos
+ asanGlobal := types.NewStruct(types.NoPkg, []*types.Field{
+ types.NewField(nxp, fname("beg"), up),
+ types.NewField(nxp, fname("size"), up),
+ types.NewField(nxp, fname("sizeWithRedzone"), up),
+ types.NewField(nxp, fname("name"), up),
+ types.NewField(nxp, fname("moduleName"), up),
+ types.NewField(nxp, fname("hasDynamicInit"), up),
+ types.NewField(nxp, fname("sourceLocation"), up),
+ types.NewField(nxp, fname("odrIndicator"), up),
+ })
+ types.CalcSize(asanGlobal)
+
+ defString := types.NewStruct(types.NoPkg, []*types.Field{
+ types.NewField(nxp, fname("data"), up),
+ types.NewField(nxp, fname("len"), up),
+ })
+ types.CalcSize(defString)
+
+ return asanGlobal, defString
+}
+
+// InstrumentGlobals declares a global array of _asan_global structures and initializes it.
+func InstrumentGlobals(fn *ir.Func) *ir.Name {
+ asanGlobalStruct, defStringstruct := createtypes()
+ // Make a global array of asanGlobal structure type.
+ // var asanglobals []asanGlobal
+ arraytype := types.NewArray(asanGlobalStruct, int64(len(ShouldInstrumentGlobals)))
+ sym := Lookup(".asanglobals")
+ globals := NewName(sym)
+ globals.SetType(arraytype)
+ globals.Class = ir.PEXTERN
+ sym.Def = globals
+ Target.Externs = append(Target.Externs, globals)
+ // Declare a local string variable.
+ // var asanSym string
+ nn := NewName(Lookup("asanSym"))
+ nn.SetType(types.Types[types.TSTRING])
+ nn.Class = ir.PAUTO
+ nn.SetUsed(true)
+ nn.Curfn = fn
+ fn.Dcl = append(fn.Dcl, nn)
+
+ var init ir.Nodes
+ var c ir.Node
+ i := int64(0)
+ for symName, n := range ShouldInstrumentGlobals {
+ setField := func(f string, val ir.Node, i int64) {
+ r := ir.NewAssignStmt(base.Pos, ir.NewSelectorExpr(base.Pos, ir.ODOT,
+ ir.NewIndexExpr(base.Pos, globals, ir.NewInt(i)), Lookup(f)), val)
+ init.Append(Stmt(r))
+ }
+ // globals[i].beg = uintptr(unsafe.Pointer(&n))
+ c = ConvNop(NodAddr(n), types.Types[types.TUNSAFEPTR])
+ c = ConvNop(c, types.Types[types.TUINTPTR])
+ setField("beg", c, i)
+ // Assign globals[i].size.
+ g := n.(*ir.Name)
+ types.CalcSize(g.Type())
+ size := g.Type().Width
+ c = ConvNop(ir.NewInt(size), types.Types[types.TUINTPTR])
+ setField("size", c, i)
+ // Assign globals[i].sizeWithRedzone.
+ rzSize := GetRedzoneSizeForGlobal(size)
+ sizeWithRz := rzSize + size
+ c = ConvNop(ir.NewInt(sizeWithRz), types.Types[types.TUINTPTR])
+ setField("sizeWithRedzone", c, i)
+ // The C string type is terminated by a null charactor "\0", Go should use three-digit
+ // octal "\000" or two-digit hexadecimal "\x00" to create null terminated string.
+ // asanSym = symName + "\000"
+ // globals[i].name = (*defString)(unsafe.Pointer(&asanSym)).data
+ init.Append(Stmt(ir.NewAssignStmt(base.Pos, nn, ir.NewString(symName+"\000"))))
+ c = ConvNop(NodAddr(nn), types.Types[types.TUNSAFEPTR])
+ c = ConvNop(c, types.NewPtr(defStringstruct))
+ c = ir.NewSelectorExpr(base.Pos, ir.ODOT, c, Lookup("data"))
+ setField("name", c, i)
+ // Set the name of package being compiled as a unique identifier of a module.
+ // asanSym = pkgName + "\000"
+ init.Append(Stmt(ir.NewAssignStmt(base.Pos, nn, ir.NewString(types.LocalPkg.Name+"\000"))))
+ c = ConvNop(NodAddr(nn), types.Types[types.TUNSAFEPTR])
+ c = ConvNop(c, types.NewPtr(defStringstruct))
+ c = ir.NewSelectorExpr(base.Pos, ir.ODOT, c, Lookup("data"))
+ setField("moduleName", c, i)
+ i++
+ }
+ fn.Body.Append(init...)
+ return globals
+}
+
+// Calculate redzone for globals.
+func GetRedzoneSizeForGlobal(size int64) int64 {
+ maxRZ := int64(1 << 18)
+ minRZ := int64(32)
+ redZone := (size / minRZ / 4) * minRZ
+ switch {
+ case redZone > maxRZ:
+ redZone = maxRZ
+ case redZone < minRZ:
+ redZone = minRZ
+ }
+ // Round up to multiple of minRZ.
+ if size%minRZ != 0 {
+ redZone += minRZ - (size % minRZ)
+ }
+ return redZone
+}
+
+var ShouldInstrumentGlobals = make(map[string]ir.Node)
+
+func CanInstrumentGlobal(g ir.Node) bool {
+ if g.Op() != ir.ONAME {
+ return false
+ }
+ n := g.(*ir.Name)
+ if n.Class == ir.PFUNC {
+ return false
+ }
+ if n.Sym().Pkg != types.LocalPkg {
+ return false
+ }
+ // Do not instrument function pointers.
+ if strings.HasPrefix(n.Sym().Name, "x_cgo_") || strings.HasPrefix(n.Sym().Name, "_cgo_") ||
+ strings.HasPrefix(n.Sym().Name, "__cgofn_") {
+ return false
+ }
+ // Do not instrument globals that do not declared in the package being compiled.
+ if !strings.HasPrefix(n.Linksym().Name, `"".`) {
+ return false
+ }
+ return true
+}
diff --git a/src/runtime/asan.go b/src/runtime/asan.go
index 7ff5f26..ad68cf2 100644
--- a/src/runtime/asan.go
+++ b/src/runtime/asan.go
@@ -36,8 +36,12 @@
//go:noescape
func asanpoison(addr unsafe.Pointer, sz uintptr)
+//go:noescape
+func asanregisterglobals(addr unsafe.Pointer, n uintptr)
+
// These are called from asan_GOARCH.s
//go:cgo_import_static __asan_read_go
//go:cgo_import_static __asan_write_go
//go:cgo_import_static __asan_unpoison_go
//go:cgo_import_static __asan_poison_go
+//go:cgo_import_static __asan_register_globals_go
diff --git a/src/runtime/asan/asan.go b/src/runtime/asan/asan.go
index 40ebf96..f0d8802 100644
--- a/src/runtime/asan/asan.go
+++ b/src/runtime/asan/asan.go
@@ -48,5 +48,35 @@
__asan_poison_memory_region(addr, sz);
}
+// Keep in sync with the defination in compiler-rt
+// https://github.com/llvm-mirror/compiler-rt/blob/69445f095c22aac2388f939bedebf224a6efcdaf/lib/asan/asan_interface_internal.h#L41
+// This structure is used to describe the source location of
+// a place where global was defined.
+struct _asan_global_source_location {
+ const char *filename;
+ int line_no;
+ int column_no;
+};
+
+// Keep in sync with the defination in compiler-rt
+// https://github.com/llvm-mirror/compiler-rt/blob/69445f095c22aac2388f939bedebf224a6efcdaf/lib/asan/asan_interface_internal.h#L48
+// This structure describes an instrumented global variable.
+struct _asan_global {
+ uintptr_t beg;
+ uintptr_t size;
+ uintptr_t size_with_redzone;
+ const char *name;
+ const char *module_name;
+ uintptr_t has_dynamic_init;
+ struct _asan_global_source_location *location;
+ uintptr_t odr_indicator;
+};
+
+// Register global variables.
+// The 'globals' is an array of structures describing 'n' globals.
+void __asan_register_globals_go(void *addr, uintptr_t n) {
+ struct _asan_global *globals = (struct _asan_global *)(addr);
+ __asan_register_globals(globals, n);
+}
*/
import "C"
diff --git a/src/runtime/asan0.go b/src/runtime/asan0.go
index dad069b..18e3102 100644
--- a/src/runtime/asan0.go
+++ b/src/runtime/asan0.go
@@ -17,7 +17,8 @@
// Because asanenabled is false, none of these functions should be called.
-func asanread(addr unsafe.Pointer, sz uintptr) { throw("asan") }
-func asanwrite(addr unsafe.Pointer, sz uintptr) { throw("asan") }
-func asanunpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
-func asanpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanread(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanwrite(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanunpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanregisterglobals(addr unsafe.Pointer, sz uintptr) { throw("asan") }
diff --git a/src/runtime/asan_amd64.s b/src/runtime/asan_amd64.s
index 01bd612..d2e012b 100644
--- a/src/runtime/asan_amd64.s
+++ b/src/runtime/asan_amd64.s
@@ -54,6 +54,14 @@
MOVQ $__asan_poison_go(SB), AX
JMP asancall<>(SB)
+// func runtime·asanregisterglobals(addr unsafe.Pointer, n uintptr)
+TEXT runtime·asanregisterglobals(SB), NOSPLIT, $0-16
+ MOVD addr+0(FP), RARG0
+ MOVD size+8(FP), RARG1
+ // void __asan_register_globals_go(void *addr, uintptr_t n);
+ MOVD $__asan_register_globals_go(SB), AX
+ JMP asancall<>(SB)
+
// Switches SP to g0 stack and calls (AX). Arguments already set.
TEXT asancall<>(SB), NOSPLIT, $0-0
get_tls(R12)
diff --git a/src/runtime/asan_arm64.s b/src/runtime/asan_arm64.s
index f1c9ec1..e72f29a 100644
--- a/src/runtime/asan_arm64.s
+++ b/src/runtime/asan_arm64.s
@@ -46,6 +46,14 @@
MOVD $__asan_poison_go(SB), FARG
JMP asancall<>(SB)
+// func runtime·asanregisterglobals(addr unsafe.Pointer, n uintptr)
+TEXT runtime·asanregisterglobals(SB), NOSPLIT, $0-16
+ MOVD addr+0(FP), RARG0
+ MOVD size+8(FP), RARG1
+ // void __asan_register_globals_go(void *addr, uintptr_t n);
+ MOVD $__asan_register_globals_go(SB), FARG
+ JMP asancall<>(SB)
+
// Switches SP to g0 stack and calls (FARG). Arguments already set.
TEXT asancall<>(SB), NOSPLIT, $0-0
MOVD RSP, R19 // callee-saved
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
fannie zhang uploaded patch set #2 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
Updates #44853.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/pkginit/init.go
M src/cmd/compile/internal/typecheck/subr.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
10 files changed, 279 insertions(+), 8 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Patch set 2:Run-TryBot +1
fannie zhang (Gerrit)
Attention is currently required from: fannie zhang.
fannie zhang uploaded patch set #3 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
Updates #44853.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
10 files changed, 334 insertions(+), 8 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Patch set 3:Run-TryBot +1
fannie zhang (Gerrit)
Attention is currently required from: fannie zhang.
fannie zhang uploaded patch set #4 to this change.
10 files changed, 341 insertions(+), 8 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Patch set 4:Run-TryBot +1
fannie zhang (Gerrit)
fannie zhang uploaded patch set #7 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
So far, the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, to use
the -asan option, a compatible version of ASan library must
be used, otherwise a segmentation fault will occur.
Updates #44853.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
12 files changed, 352 insertions(+), 8 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Patch set 7:Run-TryBot +1
fannie zhang (Gerrit)
Attention is currently required from: fannie zhang.
fannie zhang uploaded patch set #8 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
So far, the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, to use
the -asan option, a compatible version of ASan library must
be used, otherwise a segmentation fault will occur.
Updates #44853.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M misc/cgo/testsanitizers/asan_test.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
13 files changed, 389 insertions(+), 8 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Patch set 8:Run-TryBot +1
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 8:Trust +1
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
fannie zhang uploaded patch set #11 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
So far, the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, to use
the -asan option, a compatible version of ASan library must
be used, otherwise a segmentation fault will occur.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M misc/cgo/testsanitizers/asan_test.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
14 files changed, 390 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 11:Run-TryBot +1Trust +1
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
fannie zhang uploaded patch set #12 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
So far, the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, to use
the -asan option, a compatible version of ASan library must
be used, otherwise a segmentation fault will occur.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M src/runtime/asan0.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/go/internal/work/build.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/runtime/asan_arm64.s
M src/runtime/asan.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/pkginit/init.go
M src/runtime/asan/asan.go
M src/runtime/asan_amd64.s
M src/cmd/go/alldocs.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/reader.go
M misc/cgo/testsanitizers/asan_test.go
14 files changed, 421 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 12:Run-TryBot +1Trust +1
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, fannie zhang, Cherry Mui.
fannie zhang uploaded patch set #14 to this change.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 14:Run-TryBot +1Trust +1
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, fannie zhang, Cherry Mui.
2 comments:
Patchset:
It seems to me that if we are going to do this then cmd/go should verify that the compiler being used (go env CC) is a supported version. Otherwise I think the result will be an inexplicable crash, and the documentation mentioning the crash will be very obscure. It's easy to imagine some testing system running "go test -asan"; it will be weird for that to crash when updating to a new version of clang, and it won't be obvious what is wrong or how to fix it. If cmd/go can print an error, that would be much better.
File src/cmd/go/internal/work/build.go:
Patch Set #14, Line 81:
So far, the current implementation is only compatible with the ASan libraryOmit "So far, ".
I don't know how people can know what version of the ASan library they have installed. In practice people will be running "clang -fsanitize=address" or possibly "gcc -fsanitize=address". People will know the version of clang or GCC that they are using. But how will do know the version of the ASan library?
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, fannie zhang, Cherry Mui.
fannie zhang uploaded patch set #15 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M src/runtime/asan0.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/go/internal/work/init.go
M src/cmd/go/internal/work/build.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/runtime/asan_arm64.s
M src/runtime/asan.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/pkginit/init.go
M src/runtime/asan/asan.go
M src/runtime/asan_amd64.s
M src/cmd/go/alldocs.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/reader.go
M misc/cgo/testsanitizers/asan_test.go
15 files changed, 478 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 15:Trust +1
2 comments:
Patchset:
It seems to me that if we are going to do this then cmd/go should verify that the compiler being use […]
It makes sense. I will add a check for gcc/clang version in cmd/go. Done. Thank you.
File src/cmd/go/internal/work/build.go:
Patch Set #14, Line 81:
So far, the current implementation is only compatible with the ASan libraryOmit "So far, ". […]
It makes sense. I will add the required gcc/clang version here. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 16:Run-TryBot +1Trust +1
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
fannie zhang uploaded patch set #17 to this change.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, fannie zhang, Cherry Mui.
1 comment:
Patchset:
Just for the record, this will need to be reviewed by someone more familiar with cmd/compile than I am. And it will probably have to wait for the 1.19 release.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
fannie zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 17:Run-TryBot +1Trust +1
1 comment:
Patchset:
Just for the record, this will need to be reviewed by someone more familiar with cmd/compile than I […]
I got it. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 18:Trust +1
1 comment:
Patchset:
I got it. Thank you.
Hi Ian, can you help add the cmd/compile experts as reviewers? Thank you. 🙂
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Fannie Zhang, Cherry Mui.
1 comment:
Patchset:
This is for Keith, who is already a reviewer.
But this is probably for 1.19.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
Patchset:
This is for Keith, who is already a reviewer. […]
OK, Got it. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Fannie Zhang uploaded patch set #20 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M misc/cgo/testsanitizers/asan_test.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/cmd/go/internal/work/init.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
15 files changed, 481 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 20:Run-TryBot +1Trust +1
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 21:Run-TryBot +1Trust +1
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Fannie Zhang uploaded patch set #22 to this change.
15 files changed, 505 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 22:Run-TryBot +1Trust +1
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 23:Run-TryBot +1Trust +1
1 comment:
Patchset:
Could you please help to review this patch? Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
7 comments:
File src/cmd/compile/internal/gc/obj.go:
Patch Set #22, Line 294:
name = nam.Sym().Pkg.Name + strings.TrimPrefix(s.Name, `""`)This trickiness may go away soon as we're going to require base.Ctxt.Pkgpath be set for 1.19.
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #22, Line 95:
[]_asan_global= []_asan_global{...}
Patch Set #22, Line 96:
globals&globals[0]
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
put an "i" here instead of "_".
It will be int instead of int64, but you can do a i2 := int64(i) at the start of the loop if you need to.Patch Set #22, Line 99:
// TODO: How do we get the file name here?Probably something like base.Ctxt.PosTable.Pos(n.Pos()).Filename (or AbsFilename? RelFilename?).
I notice you're not setting odrIndicator. Is that written by asan itself?
Part of why I ask is that this data will be in the data section (writeable). It might be interesting to figure out how to put this data in the read-only section. Of course that wouldn't work if asan itself writes it.
Patch Set #22, Line 224:
// Do not instrument buildcfg.defaultGOROOT, because it will be set to a final value by linker.Maybe just add buildcfg to the NoInstrementPkgs list?
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #24 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
CustomizedGitHooks: yes
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M misc/cgo/testsanitizers/asan_test.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/cmd/go/internal/work/init.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
15 files changed, 518 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 24:Run-TryBot +1Trust +1
8 comments:
Patchset:
Thank you for the comments.
File src/cmd/compile/internal/gc/obj.go:
Patch Set #22, Line 294:
name = nam.Sym().Pkg.Name + strings.TrimPrefix(s.Name, `""`)This trickiness may go away soon as we're going to require base.Ctxt.Pkgpath be set for 1.19.
Got it. Do we need to change to use base.Ctxt.Pkgpath here? Thank you.
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #22, Line 95:
[]_asan_global
= []_asan_global{... […]
Done
Patch Set #22, Line 96:
globals&globals[0]
Done
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
put an "i" here instead of "_". […]
Sorry, "i" here is string type. Because ShouldInstrumentGlobals is map[string]ir.Node. Thank you.
Patch Set #22, Line 99:
// TODO: How do we get the file name here?Probably something like base.Ctxt.PosTable.Pos(n.Pos()).Filename (or AbsFilename? RelFilename?).
Wonderful! I will try it. Thank you. 👍
I notice you're not setting odrIndicator. Is that written by asan itself? […]
Yes, I didn't set its value and it's not written by asan either. Asan will only check if its value is non-zero.
You mean we should write it even if its value is 0? (go does not follow ODR, right?). Thank you.
Patch Set #22, Line 224:
// Do not instrument buildcfg.defaultGOROOT, because it will be set to a final value by linker.Maybe just add buildcfg to the NoInstrementPkgs list?
If we add it to the NoInstrumentPkgs list, then -race and -msan options will also not work on it. Is this behavior acceptable? Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
3 comments:
File src/cmd/compile/internal/gc/obj.go:
Patch Set #22, Line 294:
name = nam.Sym().Pkg.Name + strings.TrimPrefix(s.Name, `""`)
Got it. Do we need to change to use base.Ctxt.Pkgpath here? Thank you.
Sorry, I'm not entirely sure what I meant here.
I think you could do name = nam.Sym().Pkg.Prefix + s.Name. I don't think s.Name ever has "" in front of it.
Using Pkg.Prefix instead of Pkg.Name avoids possible package conflicts.(here and where ShouldInstrumentGlobals is initialized)
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Yes, I didn't set its value and it's not written by asan either. […]
Maybe just a comment saying // odrIndicator = 0 is the default, no need to set it explicitly here
So it sounds like this data could live in a read-only section. Probably not worth making that happen at the moment, I was just curious.
Patch Set #22, Line 224:
// Do not instrument buildcfg.defaultGOROOT, because it will be set to a final value by linker.
If we add it to the NoInstrumentPkgs list, then -race and -msan options will also not work on it. […]
Yes, that's fine. buildcfg is only used by tools, not end-user programs.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #29 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M misc/cgo/testsanitizers/asan_test.go
M src/cmd/compile/internal/base/base.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/cmd/go/internal/work/init.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
16 files changed, 510 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 29:Trust +1
4 comments:
Patchset:
All done. Thank you.
File src/cmd/compile/internal/gc/obj.go:
Patch Set #22, Line 294:
name = nam.Sym().Pkg.Name + strings.TrimPrefix(s.Name, `""`)
Sorry, I'm not entirely sure what I meant here. […]
1. The `nam.Sym().Name` does have `""` in front of it. The `S.Name` here is `nam.Linksym().Name`, it has `"".` in front of it. So change to `name.Sym().Name` here.
2. Why use `Pkg.Name` instead of `Pkg.Prefix`, please refer to the comments on line 99 of init.go. Thank you.
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Maybe just a comment saying // odrIndicator = 0 is the default, no need to set it explicitly here […]
Done
Patch Set #22, Line 224:
// Do not instrument buildcfg.defaultGOROOT, because it will be set to a final value by linker.
Yes, that's fine. buildcfg is only used by tools, not end-user programs.
Got it. Done. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 29:Run-TryBot +1
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
1 comment:
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #29, Line 104:
name := n.Sym().LinknameSorry, I'm still a bit confused as to what this is trying to fix.
canInstrumentGlobal(n) can't return true if n is not from the local package. So I don't really understand why it matters that the names match.
For normal, non-linkname'd globals, the package they are declared in will emit their global declaration and add an entry for them in a asanregisterglobals call.
For linkname'd globals, who does what? Which package is responsible? For example, we have:
in reflect:
//go:linkname zeroVal runtime.zeroVal
var zeroVal [maxZero]bytein runtime:
var zeroVal [maxZero]byte
Presumably runtime must be the one responsible, because when we're compiling runtime we don't know that someone has a linkname pointing to it. The code in this CL makes me think that the reflect package is also trying to register this variable. But do we need it registered twice? Why don't we just skip globals that are linknamed, because their home package will do the work?
But there are other linkname scenarios, so-called push and pull ones, not sure if other ways of using it might introduce different complications. Can you point to a linkname'd global that demonstrates why this code is needed?
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #30 to this change.
16 files changed, 519 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #31 to this change.
16 files changed, 513 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 31:Run-TryBot +1Trust +1
1 comment:
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #29, Line 104:
name := n.Sym().Linkname
Sorry, I'm still a bit confused as to what this is trying to fix. […]
Why I added this code because I used -asan option to test some go-related projects, but it will report negative false.
Take the above case as an example, the reason is that we instrument `zeroVal` variable whem compiling `runtime` package and set its symbol name `"".zerVal` as the `ShouldInstrumentGlobals` map's key. But when we write the linkname'd variable `zeroVal` into object file when compiling `reflect` package, because its symbol name is `runtime.zeroVal`, we get the false when look into the `ShouldInstrumentGlobals` map, so we write the incorrect size that does not have the trailing red area into object file. Incorrect sizes cause some global variables to be allocated to poisoned memory segments.
So use the `pkg.name` instead of `n.Sym().Name` as the `ShouldInstrumentGlobals's` key to avoid the above issues.
`But do we need it registered twice?`
Yes, you are right. We can skip instrument for globals that are linknamed. And set the `pkg.name` as the key. In the process of writing object file, for linkname'd symbols, we can use their link name as the key to check if they are intrumented in their home package.
`But there are other linkname scenarios, so-called push and pull ones, not sure if other ways of using it might introduce different complications.`
I haven't seen any other link name scenarios so far.
Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 34:Trust +1
1 comment:
Patchset:
Hi Keith, could you help to review it again? Thank you. 🙂
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Patch set 34:Run-TryBot +1
3 comments:
File src/cmd/compile/internal/gc/obj.go:
Patch Set #31, Line 288:
// For linkname'd global variables, if we instrument them in the package in whichWe're now just skipping linkname'd globals, so we can just skip those.
if base.Flag.ASan && name.Sym().Linkname == "" && pkginit.ShouldInstrumentGlobals[name] != nil { ... }
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #31, Line 109:
ShouldInstrumentGlobals[name] = nThis map should only contain globals from the local package now, right? I think that means we can use just the package-local name (n.Sym().Name) as the key.
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #31, Line 212:
var ShouldInstrumentGlobals = make(map[string]ir.Node)Comment that this contains only package-local (and unlinknamed from somewhere else) globals.
And mention what the key looks like. A simple example would probably be clearest."In package p, a global foo would be in this map as "p.foo"" or "foo" if we can just use the bare name.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #35 to this change.
16 files changed, 1,608 insertions(+), 1,070 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 35:Run-TryBot +1
3 comments:
File src/cmd/compile/internal/gc/obj.go:
Patch Set #31, Line 288:
// For linkname'd global variables, if we instrument them in the package in which
We're now just skipping linkname'd globals, so we can just skip those. […]
Done
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #31, Line 109:
ShouldInstrumentGlobals[name] = n
This map should only contain globals from the local package now, right? I think that means we can us […]
Done
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #31, Line 212:
var ShouldInstrumentGlobals = make(map[string]ir.Node)
Comment that this contains only package-local (and unlinknamed from somewhere else) globals. […]
Done
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #36 to this change.
16 files changed, 504 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 36:Run-TryBot +1
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
1 comment:
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #36, Line 70:
ShouldInstrumentGlobalsI think this will lead to nondeterministic compiler output, because range over maps is nondeterministic. You could read, sort, then build here, or have ShouldInstrumentGlobals be both a map and a slice. You can use the map for membership and the slice for iterating.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #37 to this change.
16 files changed, 511 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #36, Line 70:
ShouldInstrumentGlobals
I think this will lead to nondeterministic compiler output, because range over maps is nondeterminis […]
Done. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 37:Run-TryBot +1
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Patch set 37:Code-Review +2
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Patch set 37:Auto-Submit +1
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Patch set 37:Code-Review +1
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Fannie Zhang, Cherry Mui.
6 comments:
File misc/cgo/testsanitizers/asan_test.go:
Patch Set #37, Line 34:
cc, err := goEnv("CC")This package already has a compilerVersion function that returns the compiler version number. Use that.
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #22, Line 99:
// TODO: How do we get the file name here?Wonderful! I will try it. Thank you. […]
Done
Patch Set #22, Line 224:
// Do not instrument buildcfg.defaultGOROOT, because it will be set to a final value by linker.
Got it. Done. Thank you.
Done
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #36, Line 70:
ShouldInstrumentGlobals
Done. Thank you.
Done
File src/cmd/go/internal/work/build.go:
Patch Set #14, Line 81:
So far, the current implementation is only compatible with the ASan libraryIt makes sense. I will add the required gcc/clang version here. Thank you.
Done
File src/cmd/go/internal/work/build.go:
Patch Set #37, Line 82:
Notice that the current implementation is only compatible with the ASan library fromAt this documentation level we can shorten this.
Supported only on linux/amd64 or linux/arm64 and only with GCC 7 and higher or Clang/LLVM 4 and higher.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #38 to this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
---
M misc/cgo/testsanitizers/asan_test.go
M misc/cgo/testsanitizers/cc_test.go
M src/cmd/compile/internal/base/base.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/cmd/go/internal/work/init.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
17 files changed, 491 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 38:Run-TryBot +1
3 comments:
File misc/cgo/testsanitizers/asan_test.go:
Patch Set #37, Line 34:
cc, err := goEnv("CC")
This package already has a compilerVersion function that returns the compiler version number. […]
Done. Thank you.
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #37, Line 70:
for _, n := range ShouldInstrumentGlobals {@Keith, I found a to avoid range over maps here. That is, create two variables InstrumentGlobalsMap and InstrumentGlobalsSlice, the map is used for membership and the slice is used for iteration. But I do not know which implemetation is better. Could you help to review the patchset 38 again? Thank you.
File src/cmd/go/internal/work/build.go:
Patch Set #37, Line 82:
Notice that the current implementation is only compatible with the ASan library from
At this documentation level we can shorten this. […]
Done. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #39 to this change.
17 files changed, 490 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 39:Run-TryBot +1
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
Patchset:
2 of 30 TryBots failed. […]
I am not familiar with this error. Should we change `extern void __asan_register_globals(struct _asan_global*, uintptr_t)` to `extern void __asan_register_globals(void*, long int)` ? Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Fannie Zhang uploaded patch set #40 to this change.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 40:Run-TryBot +1
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
Patchset:
@Keith, could you please help to review this patch again? I did some changes to avoid range over maps. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Patch set 40:Auto-Submit +1Code-Review +2
1 comment:
Patchset:
Looks good.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
Patchset:
@Ian, could you please help to review this patch again? Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Fannie Zhang, Cherry Mui.
6 comments:
File misc/cgo/testsanitizers/cc_test.go:
Patch Set #40, Line 239:
func compilerRequiredVersion() bool {This is the version required for the asan tests, but this test is used for tsan and msan as well. So this function should be something like compilerIsRequiredAsanVersion.
File src/cmd/compile/internal/gc/obj.go:
Patch Set #22, Line 294:
name = nam.Sym().Pkg.Name + strings.TrimPrefix(s.Name, `""`)1. The `nam.Sym().Name` does have `""` in front of it. The `S.Name` here is `nam.Linksym(). […]
Ack
File src/cmd/compile/internal/pkginit/init.go:
Patch Set #29, Line 104:
name := n.Sym().Linkname
Why I added this code because I used -asan option to test some go-related projects, but it will repo […]
Ack
File src/cmd/compile/internal/pkginit/initAsanGlobals.go:
Patch Set #37, Line 70:
for _, n := range ShouldInstrumentGlobals {
@Keith, I found a to avoid range over maps here. […]
Ack
File src/cmd/go/internal/work/init.go:
Patch Set #40, Line 149:
fmt.Fprintf(os.Stderr, "-asan: too old version of gcc %d.%d uses v6 or lower version of ASan library", major, minor)Our users have no control over the version of the ASan library, Better would be
-asan is not supported with gcc %d.%d; requires gcc 7 or later
Patch Set #40, Line 155:
fmt.Fprintf(os.Stderr, "-asan: too old version of clang %d.%d uses v6 or lower version of ASan library", major, minor)-asan is not supported with clang %d.%d; requires clang 4 or later
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Fannie Zhang, Cherry Mui.
Fannie Zhang uploaded patch set #41 to this change.
17 files changed, 495 insertions(+), 9 deletions(-)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 42:Run-TryBot +1
4 comments:
Patchset:
All done. Thank you.
File misc/cgo/testsanitizers/cc_test.go:
Patch Set #40, Line 239:
func compilerRequiredVersion() bool {
This is the version required for the asan tests, but this test is used for tsan and msan as well. […]
Make sense. Done. Thank you.
File src/cmd/go/internal/work/init.go:
Patch Set #40, Line 149:
fmt.Fprintf(os.Stderr, "-asan: too old version of gcc %d.%d uses v6 or lower version of ASan library", major, minor)
Our users have no control over the version of the ASan library, Better would be […]
Done
Patch Set #40, Line 155:
fmt.Fprintf(os.Stderr, "-asan: too old version of clang %d.%d uses v6 or lower version of ASan library", major, minor)
-asan is not supported with clang %d. […]
Done
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
Patchset:
I am not familiar with this error. […]
Done
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Fannie Zhang, Cherry Mui.
1 comment:
File misc/cgo/testsanitizers/asan_test.go:
Patch Set #42, Line 30:
if compilerRequiredAsanVersion() {Shouldn't this be
if !compilerRequiredAsanVersion() {? That is, aren't you missing a '!'?
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
Patch set 43:Run-TryBot +1
1 comment:
File misc/cgo/testsanitizers/asan_test.go:
Patch Set #42, Line 30:
if compilerRequiredAsanVersion() {
Shouldn't this be […]
Sorry, "!" is missed here. Done. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
File misc/cgo/testsanitizers/asan_test.go:
Patch Set #42, Line 30:
if compilerRequiredAsanVersion() {
Sorry, "!" is missed here. Done. Thank you.
Done
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Cherry Mui.
1 comment:
Patchset:
1 of 30 TryBots failed. […]
This issue does not seem to be related to this patch. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Keith Randall, Ian Lance Taylor, Fannie Zhang, Cherry Mui.
Patch set 43:Code-Review +2
Ian Lance Taylor (Gerrit)
Ian Lance Taylor submitted this change.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Change-Id: I664e74dcabf5dc7ed46802859174606454e8f1d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/321715
Reviewed-by: Keith Randall <k...@google.com>
Reviewed-by: Keith Randall <k...@golang.org>
Run-TryBot: Fannie Zhang <Fannie...@arm.com>
Reviewed-by: Ian Lance Taylor <ia...@google.com>
---
M misc/cgo/testsanitizers/asan_test.go
M misc/cgo/testsanitizers/cc_test.go
M src/cmd/compile/internal/base/base.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/cmd/go/internal/work/init.go
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
17 files changed, 500 insertions(+), 9 deletions(-)
diff --git a/misc/cgo/testsanitizers/asan_test.go b/misc/cgo/testsanitizers/asan_test.go
index ff578ac..23392c0 100644
--- a/misc/cgo/testsanitizers/asan_test.go
+++ b/misc/cgo/testsanitizers/asan_test.go
@@ -22,6 +22,14 @@
if !aSanSupported(goos, goarch) {
t.Skipf("skipping on %s/%s; -asan option is not supported.", goos, goarch)
}
+ // The current implementation is only compatible with the ASan library from version
+ // v7 to v9 (See the description in src/runtime/asan/asan.go). Therefore, using the
+ // -asan option must use a compatible version of ASan library, which requires that
+ // the gcc version is not less than 7 and the clang version is not less than 4,
+ // otherwise a segmentation fault will occur.
+ if !compilerRequiredAsanVersion() {
+ t.Skipf("skipping: too old version of compiler")
+ }
t.Parallel()
requireOvercommit(t)
diff --git a/misc/cgo/testsanitizers/cc_test.go b/misc/cgo/testsanitizers/cc_test.go
index 05b7793..ee3c3bf 100644
--- a/misc/cgo/testsanitizers/cc_test.go
+++ b/misc/cgo/testsanitizers/cc_test.go
@@ -235,6 +235,22 @@
}
}
+// compilerRequiredAsanVersion reports whether the compiler is the version required by Asan.
+func compilerRequiredAsanVersion() bool {
+ compiler, err := compilerVersion()
+ if err != nil {
+ return false
+ }
+ switch compiler.name {
+ case "gcc":
+ return compiler.major >= 7
+ case "clang":
+ return true
+ default:
+ return false
+ }
+}
+
type compilerCheck struct {
once sync.Once
err error
diff --git a/src/cmd/compile/internal/base/base.go b/src/cmd/compile/internal/base/base.go
index 39ce8e6..5e1493e 100644
--- a/src/cmd/compile/internal/base/base.go
+++ b/src/cmd/compile/internal/base/base.go
@@ -70,6 +70,7 @@
"runtime/msan",
"runtime/asan",
"internal/cpu",
+ "buildcfg",
}
// Don't insert racefuncenter/racefuncexit into the following packages.
diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go
index fe8b6e9..fea2df8 100644
--- a/src/cmd/compile/internal/gc/obj.go
+++ b/src/cmd/compile/internal/gc/obj.go
@@ -9,6 +9,7 @@
"cmd/compile/internal/ir"
"cmd/compile/internal/noder"
"cmd/compile/internal/objw"
+ "cmd/compile/internal/pkginit"
"cmd/compile/internal/reflectdata"
"cmd/compile/internal/staticdata"
"cmd/compile/internal/typecheck"
@@ -110,7 +111,6 @@
func dumpdata() {
numExterns := len(typecheck.Target.Externs)
numDecls := len(typecheck.Target.Decls)
-
dumpglobls(typecheck.Target.Externs)
reflectdata.CollectPTabs()
numExports := len(typecheck.Target.Exports)
@@ -287,7 +287,20 @@
if nam.Type() != nil && !nam.Type().HasPointers() {
flags |= obj.NOPTR
}
- base.Ctxt.Globl(s, nam.Type().Size(), flags)
+ size := nam.Type().Size()
+ linkname := nam.Sym().Linkname
+ name := nam.Sym().Name
+
+ // We've skipped linkname'd globals's instrument, so we can skip them here as well.
+ if base.Flag.ASan && linkname == "" && pkginit.InstrumentGlobalsMap[name] != nil {
+ // Write the new size of instrumented global variables that have
+ // trailing redzones into object file.
+ rzSize := pkginit.GetRedzoneSizeForGlobal(size)
+ sizeWithRZ := rzSize + size
+ base.Ctxt.Globl(s, sizeWithRZ, flags)
+ } else {
+ base.Ctxt.Globl(s, size, flags)
+ }
if nam.LibfuzzerExtraCounter() {
s.Type = objabi.SLIBFUZZER_EXTRA_COUNTER
}
diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go
index 9a42b5a..c4c2db5 100644
--- a/src/cmd/compile/internal/noder/noder.go
+++ b/src/cmd/compile/internal/noder/noder.go
@@ -442,7 +442,7 @@
// the name, normally "pkg.init", is altered to "pkg.init.0".
var renameinitgen int
-func renameinit() *types.Sym {
+func Renameinit() *types.Sym {
s := typecheck.LookupNum("init.", renameinitgen)
renameinitgen++
return s
diff --git a/src/cmd/compile/internal/noder/object.go b/src/cmd/compile/internal/noder/object.go
index e8dbaac..ee9e0e2 100644
--- a/src/cmd/compile/internal/noder/object.go
+++ b/src/cmd/compile/internal/noder/object.go
@@ -104,7 +104,7 @@
var typ *types.Type
if recv := sig.Recv(); recv == nil {
if obj.Name() == "init" {
- sym = renameinit()
+ sym = Renameinit()
} else {
sym = g.sym(obj)
}
diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go
index 1350c22..1086171 100644
--- a/src/cmd/compile/internal/noder/reader.go
+++ b/src/cmd/compile/internal/noder/reader.go
@@ -643,7 +643,7 @@
case pkgbits.ObjFunc:
if sym.Name == "init" {
- sym = renameinit()
+ sym = Renameinit()
}
name := do(ir.ONAME, true)
setType(name, r.signature(sym.Pkg, nil))
diff --git a/src/cmd/compile/internal/pkginit/init.go b/src/cmd/compile/internal/pkginit/init.go
index 32e95be..d94482a 100644
--- a/src/cmd/compile/internal/pkginit/init.go
+++ b/src/cmd/compile/internal/pkginit/init.go
@@ -7,6 +7,7 @@
import (
"cmd/compile/internal/base"
"cmd/compile/internal/ir"
+ "cmd/compile/internal/noder"
"cmd/compile/internal/objw"
"cmd/compile/internal/staticinit"
"cmd/compile/internal/typecheck"
@@ -83,6 +84,58 @@
}
deps = append(deps, n.(*ir.Name).Linksym())
}
+ if base.Flag.ASan {
+ // Make an initialization function to call runtime.asanregisterglobals to register an
+ // array of instrumented global variables when -asan is enabled. An instrumented global
+ // variable is described by a structure.
+ // See the _asan_global structure declared in src/runtime/asan/asan.go.
+ //
+ // func init {
+ // var globals []_asan_global {...}
+ // asanregisterglobals(&globals[0], len(globals))
+ // }
+ for _, n := range typecheck.Target.Externs {
+ if canInstrumentGlobal(n) {
+ name := n.Sym().Name
+ InstrumentGlobalsMap[name] = n
+ InstrumentGlobalsSlice = append(InstrumentGlobalsSlice, n)
+ }
+ }
+ ni := len(InstrumentGlobalsMap)
+ if ni != 0 {
+ // Make an init._ function.
+ base.Pos = base.AutogeneratedPos
+ typecheck.DeclContext = ir.PEXTERN
+ name := noder.Renameinit()
+ fnInit := typecheck.DeclFunc(name, ir.NewFuncType(base.Pos, nil, nil, nil))
+
+ // Get an array of intrumented global variables.
+ globals := instrumentGlobals(fnInit)
+
+ // Call runtime.asanregisterglobals function to poison redzones.
+ // runtime.asanregisterglobals(unsafe.Pointer(&globals[0]), ni)
+ asanf := typecheck.NewName(ir.Pkgs.Runtime.Lookup("asanregisterglobals"))
+ ir.MarkFunc(asanf)
+ asanf.SetType(types.NewSignature(types.NoPkg, nil, nil, []*types.Field{
+ types.NewField(base.Pos, nil, types.Types[types.TUNSAFEPTR]),
+ types.NewField(base.Pos, nil, types.Types[types.TUINTPTR]),
+ }, nil))
+ asancall := ir.NewCallExpr(base.Pos, ir.OCALL, asanf, nil)
+ asancall.Args.Append(typecheck.ConvNop(typecheck.NodAddr(
+ ir.NewIndexExpr(base.Pos, globals, ir.NewInt(0))), types.Types[types.TUNSAFEPTR]))
+ asancall.Args.Append(typecheck.ConvNop(ir.NewInt(int64(ni)), types.Types[types.TUINTPTR]))
+
+ fnInit.Body.Append(asancall)
+ typecheck.FinishFuncBody()
+ typecheck.Func(fnInit)
+ ir.CurFunc = fnInit
+ typecheck.Stmts(fnInit.Body)
+ ir.CurFunc = nil
+
+ typecheck.Target.Decls = append(typecheck.Target.Decls, fnInit)
+ typecheck.Target.Inits = append(typecheck.Target.Inits, fnInit)
+ }
+ }
// Record user init functions.
for _, fn := range typecheck.Target.Inits {
diff --git a/src/cmd/compile/internal/pkginit/initAsanGlobals.go b/src/cmd/compile/internal/pkginit/initAsanGlobals.go
new file mode 100644
index 0000000..7276791
--- /dev/null
+++ b/src/cmd/compile/internal/pkginit/initAsanGlobals.go
@@ -0,0 +1,241 @@
+// Copyright 2022 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 pkginit
+
+import (
+ "strings"
+
+ "cmd/compile/internal/base"
+ "cmd/compile/internal/ir"
+ "cmd/compile/internal/typecheck"
+ "cmd/compile/internal/types"
+ "cmd/internal/src"
+)
+
+// instrumentGlobals declares a global array of _asan_global structures and initializes it.
+func instrumentGlobals(fn *ir.Func) *ir.Name {
+ asanGlobalStruct, asanLocationStruct, defStringstruct := createtypes()
+ lname := typecheck.Lookup
+ tconv := typecheck.ConvNop
+ // Make a global array of asanGlobalStruct type.
+ // var asanglobals []asanGlobalStruct
+ arraytype := types.NewArray(asanGlobalStruct, int64(len(InstrumentGlobalsMap)))
+ symG := lname(".asanglobals")
+ globals := typecheck.NewName(symG)
+ globals.SetType(arraytype)
+ globals.Class = ir.PEXTERN
+ symG.Def = globals
+ typecheck.Target.Externs = append(typecheck.Target.Externs, globals)
+ // Make a global array of asanLocationStruct type.
+ // var asanL []asanLocationStruct
+ arraytype = types.NewArray(asanLocationStruct, int64(len(InstrumentGlobalsMap)))
+ symL := lname(".asanL")
+ asanlocation := typecheck.NewName(symL)
+ asanlocation.SetType(arraytype)
+ asanlocation.Class = ir.PEXTERN
+ symL.Def = asanlocation
+ typecheck.Target.Externs = append(typecheck.Target.Externs, asanlocation)
+ // Make three global string variables to pass the global name and module name
+ // and the name of the source file that defines it.
+ // var asanName string
+ // var asanModulename string
+ // var asanFilename string
+ symL = lname(".asanName")
+ asanName := typecheck.NewName(symL)
+ asanName.SetType(types.Types[types.TSTRING])
+ asanName.Class = ir.PEXTERN
+ symL.Def = asanName
+ typecheck.Target.Externs = append(typecheck.Target.Externs, asanName)
+
+ symL = lname(".asanModulename")
+ asanModulename := typecheck.NewName(symL)
+ asanModulename.SetType(types.Types[types.TSTRING])
+ asanModulename.Class = ir.PEXTERN
+ symL.Def = asanModulename
+ typecheck.Target.Externs = append(typecheck.Target.Externs, asanModulename)
+
+ symL = lname(".asanFilename")
+ asanFilename := typecheck.NewName(symL)
+ asanFilename.SetType(types.Types[types.TSTRING])
+ asanFilename.Class = ir.PEXTERN
+ symL.Def = asanFilename
+ typecheck.Target.Externs = append(typecheck.Target.Externs, asanFilename)
+
+ var init ir.Nodes
+ var c ir.Node
+ // globals[i].odrIndicator = 0 is the default, no need to set it explicitly here.
+ for i, n := range InstrumentGlobalsSlice {
+ setField := func(f string, val ir.Node, i int) {
+ r := ir.NewAssignStmt(base.Pos, ir.NewSelectorExpr(base.Pos, ir.ODOT,
+ ir.NewIndexExpr(base.Pos, globals, ir.NewInt(int64(i))), lname(f)), val)
+ init.Append(typecheck.Stmt(r))
+ }
+ // globals[i].beg = uintptr(unsafe.Pointer(&n))
+ c = tconv(typecheck.NodAddr(n), types.Types[types.TUNSAFEPTR])
+ c = tconv(c, types.Types[types.TUINTPTR])
+ setField("beg", c, i)
+ // Assign globals[i].size.
+ g := n.(*ir.Name)
+ size := g.Type().Size()
+ c = tconv(ir.NewInt(size), types.Types[types.TUINTPTR])
+ setField("size", c, i)
+ // Assign globals[i].sizeWithRedzone.
+ rzSize := GetRedzoneSizeForGlobal(size)
+ sizeWithRz := rzSize + size
+ c = tconv(ir.NewInt(sizeWithRz), types.Types[types.TUINTPTR])
+ setField("sizeWithRedzone", c, i)
+ // The C string type is terminated by a null charactor "\0", Go should use three-digit
+ // octal "\000" or two-digit hexadecimal "\x00" to create null terminated string.
+ // asanName = symbol's linkname + "\000"
+ // globals[i].name = (*defString)(unsafe.Pointer(&asanName)).data
+ name := g.Linksym().Name
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(base.Pos, asanName, ir.NewString(name+"\000"))))
+ c = tconv(typecheck.NodAddr(asanName), types.Types[types.TUNSAFEPTR])
+ c = tconv(c, types.NewPtr(defStringstruct))
+ c = ir.NewSelectorExpr(base.Pos, ir.ODOT, c, lname("data"))
+ setField("name", c, i)
+
+ // Set the name of package being compiled as a unique identifier of a module.
+ // asanModulename = pkgName + "\000"
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(base.Pos, asanModulename, ir.NewString(types.LocalPkg.Name+"\000"))))
+ c = tconv(typecheck.NodAddr(asanModulename), types.Types[types.TUNSAFEPTR])
+ c = tconv(c, types.NewPtr(defStringstruct))
+ c = ir.NewSelectorExpr(base.Pos, ir.ODOT, c, lname("data"))
+ setField("moduleName", c, i)
+ // Assign asanL[i].filename, asanL[i].line, asanL[i].column
+ // and assign globals[i].location = uintptr(unsafe.Pointer(&asanL[i]))
+ asanLi := ir.NewIndexExpr(base.Pos, asanlocation, ir.NewInt(int64(i)))
+ filename := ir.NewString(base.Ctxt.PosTable.Pos(n.Pos()).Filename() + "\000")
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(base.Pos, asanFilename, filename)))
+ c = tconv(typecheck.NodAddr(asanFilename), types.Types[types.TUNSAFEPTR])
+ c = tconv(c, types.NewPtr(defStringstruct))
+ c = ir.NewSelectorExpr(base.Pos, ir.ODOT, c, lname("data"))
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(base.Pos, ir.NewSelectorExpr(base.Pos, ir.ODOT, asanLi, lname("filename")), c)))
+ line := ir.NewInt(int64(n.Pos().Line()))
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(base.Pos, ir.NewSelectorExpr(base.Pos, ir.ODOT, asanLi, lname("line")), line)))
+ col := ir.NewInt(int64(n.Pos().Col()))
+ init.Append(typecheck.Stmt(ir.NewAssignStmt(base.Pos, ir.NewSelectorExpr(base.Pos, ir.ODOT, asanLi, lname("column")), col)))
+ c = tconv(typecheck.NodAddr(asanLi), types.Types[types.TUNSAFEPTR])
+ c = tconv(c, types.Types[types.TUINTPTR])
+ setField("sourceLocation", c, i)
+ }
+ fn.Body.Append(init...)
+ return globals
+}
+
+// createtypes creates the asanGlobal, asanLocation and defString struct type.
+// Go compiler does not refer to the C types, we represent the struct field
+// by a uintptr, then use type conversion to make copies of the data.
+// E.g., (*defString)(asanGlobal.name).data to C string.
+//
+// Keep in sync with src/runtime/asan/asan.go.
+// type asanGlobal struct {
+// beg uintptr
+// size uintptr
+// size_with_redzone uintptr
+// name uintptr
+// moduleName uintptr
+// hasDynamicInit uintptr
+// sourceLocation uintptr
+// odrIndicator uintptr
+// }
+//
+// type asanLocation struct {
+// filename uintptr
+// line int32
+// column int32
+// }
+//
+// defString is synthesized struct type meant to capture the underlying
+// implementations of string.
+// type defString struct {
+// data uintptr
+// len uintptr
+// }
+
+func createtypes() (*types.Type, *types.Type, *types.Type) {
+ up := types.Types[types.TUINTPTR]
+ i32 := types.Types[types.TINT32]
+ fname := typecheck.Lookup
+ nxp := src.NoXPos
+ nfield := types.NewField
+ asanGlobal := types.NewStruct(types.NoPkg, []*types.Field{
+ nfield(nxp, fname("beg"), up),
+ nfield(nxp, fname("size"), up),
+ nfield(nxp, fname("sizeWithRedzone"), up),
+ nfield(nxp, fname("name"), up),
+ nfield(nxp, fname("moduleName"), up),
+ nfield(nxp, fname("hasDynamicInit"), up),
+ nfield(nxp, fname("sourceLocation"), up),
+ nfield(nxp, fname("odrIndicator"), up),
+ })
+ types.CalcSize(asanGlobal)
+
+ asanLocation := types.NewStruct(types.NoPkg, []*types.Field{
+ nfield(nxp, fname("filename"), up),
+ nfield(nxp, fname("line"), i32),
+ nfield(nxp, fname("column"), i32),
+ })
+ types.CalcSize(asanLocation)
+
+ defString := types.NewStruct(types.NoPkg, []*types.Field{
+ types.NewField(nxp, fname("data"), up),
+ types.NewField(nxp, fname("len"), up),
+ })
+ types.CalcSize(defString)
+
+ return asanGlobal, asanLocation, defString
+}
+
+// Calculate redzone for globals.
+func GetRedzoneSizeForGlobal(size int64) int64 {
+ maxRZ := int64(1 << 18)
+ minRZ := int64(32)
+ redZone := (size / minRZ / 4) * minRZ
+ switch {
+ case redZone > maxRZ:
+ redZone = maxRZ
+ case redZone < minRZ:
+ redZone = minRZ
+ }
+ // Round up to multiple of minRZ.
+ if size%minRZ != 0 {
+ redZone += minRZ - (size % minRZ)
+ }
+ return redZone
+}
+
+// InstrumentGlobalsMap contains only package-local (and unlinknamed from somewhere else)
+// globals.
+// And the key is the object name. For example, in package p, a global foo would be in this
+// map as "foo".
+// Consider range over maps is nondeterministic, make a slice to hold all the values in the
+// InstrumentGlobalsMap and iterate over the InstrumentGlobalsSlice.
+var InstrumentGlobalsMap = make(map[string]ir.Node)
+var InstrumentGlobalsSlice = make([]ir.Node, 0, 0)
+
+func canInstrumentGlobal(g ir.Node) bool {
+ if g.Op() != ir.ONAME {
+ return false
+ }
+ n := g.(*ir.Name)
+ if n.Class == ir.PFUNC {
+ return false
+ }
+ if n.Sym().Pkg != types.LocalPkg {
+ return false
+ }
+ // Do not instrument any _cgo_ related global variables, because they are declared in C code.
+ if strings.Contains(n.Sym().Name, "cgo") {
+ return false
+ }
+
+ // Do not instrument globals that are linknamed, because their home package will do the work.
+ if n.Sym().Linkname != "" {
+ return false
+ }
+
+ return true
+}
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 6fdb4f9..7193ab6 100644
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -123,6 +123,8 @@
// -asan
// enable interoperation with address sanitizer.
// Supported only on linux/arm64, linux/amd64.
+// Supported only on linux/amd64 or linux/arm64 and only with GCC 7 and higher
+// or Clang/LLVM 4 and higher.
// -v
// print the names of packages as they are compiled.
// -work
diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index e9a8ee6..d69eb7a 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -79,6 +79,8 @@
-asan
enable interoperation with address sanitizer.
Supported only on linux/arm64, linux/amd64.
+ Supported only on linux/amd64 or linux/arm64 and only with GCC 7 and higher
+ or Clang/LLVM 4 and higher.
-v
print the names of packages as they are compiled.
-work
diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index 26192ec..22e29e8 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -14,9 +14,13 @@
"cmd/internal/quoted"
"cmd/internal/sys"
"fmt"
+ exec "internal/execabs"
"os"
"path/filepath"
+ "regexp"
"runtime"
+ "strconv"
+ "strings"
)
func BuildInit() {
@@ -107,6 +111,15 @@
base.SetExitStatus(2)
base.Exit()
}
+ // The current implementation is only compatible with the ASan library from version
+ // v7 to v9 (See the description in src/runtime/asan/asan.go). Therefore, using the
+ // -asan option must use a compatible version of ASan library, which requires that
+ // the gcc version is not less than 7 and the clang version is not less than 4,
+ // otherwise a segmentation fault will occur.
+ if cfg.BuildASan {
+ compilerRequiredAsanVersion()
+ }
+
mode := "race"
if cfg.BuildMSan {
mode = "msan"
@@ -310,3 +323,47 @@
}
}
}
+
+// compilerRequiredAsanVersion checks whether the compiler is the version required by Asan.
+func compilerRequiredAsanVersion() {
+ cc := os.Getenv("CC")
+ isgcc := false
+ if strings.HasPrefix(cc, "gcc") {
+ isgcc = true
+ } else if !strings.HasPrefix(cc, "clang") {
+ fmt.Fprintf(os.Stderr, "-asan requires C compiler is gcc or clang, not %s", cc)
+ base.SetExitStatus(2)
+ base.Exit()
+ }
+ out, err := exec.Command(cc, "-v").CombinedOutput()
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "-asan fails to check C compiler %s version: %v", cc, err)
+ base.SetExitStatus(2)
+ base.Exit()
+ }
+ re := regexp.MustCompile(`version ([0-9]+)\.([0-9]+)\.([0-9]+)`)
+ matches := re.FindSubmatch(out)
+ if len(matches) < 3 {
+ fmt.Fprintf(os.Stderr, "-asan fails to check C compiler %s version: %s", cc, out)
+ }
+ major, err1 := strconv.Atoi(string(matches[1]))
+ minor, err2 := strconv.Atoi(string(matches[2]))
+ if err1 != nil || err2 != nil {
+ fmt.Fprintf(os.Stderr, "-asan fails to check C compiler %s version: %v, %v", cc, err1, err2)
+ base.SetExitStatus(2)
+ base.Exit()
+ }
+ if isgcc {
+ if major < 7 {
+ fmt.Fprintf(os.Stderr, "-asan is not supported with gcc %d.%d; requires gcc 7 or later", major, minor)
+ base.SetExitStatus(2)
+ base.Exit()
+ }
+ } else {
+ if major < 4 {
+ fmt.Fprintf(os.Stderr, "-asan is not supported with clang %d.%d; requires clang 4 or later", major, minor)
+ base.SetExitStatus(2)
+ base.Exit()
+ }
+ }
+}
diff --git a/src/runtime/asan.go b/src/runtime/asan.go
index 8c41e41..25b8327 100644
--- a/src/runtime/asan.go
+++ b/src/runtime/asan.go
@@ -55,9 +55,13 @@
//go:noescape
func asanpoison(addr unsafe.Pointer, sz uintptr)
+//go:noescape
+func asanregisterglobals(addr unsafe.Pointer, n uintptr)
+
// These are called from asan_GOARCH.s
//
//go:cgo_import_static __asan_read_go
//go:cgo_import_static __asan_write_go
//go:cgo_import_static __asan_unpoison_go
//go:cgo_import_static __asan_poison_go
+//go:cgo_import_static __asan_register_globals_go
diff --git a/src/runtime/asan/asan.go b/src/runtime/asan/asan.go
index bab2362..3e41d60 100644
--- a/src/runtime/asan/asan.go
+++ b/src/runtime/asan/asan.go
@@ -34,5 +34,43 @@
__asan_poison_memory_region(addr, sz);
}
+// Keep in sync with the defination in compiler-rt
+// https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_interface_internal.h#L41
+// This structure is used to describe the source location of
+// a place where global was defined.
+struct _asan_global_source_location {
+ const char *filename;
+ int line_no;
+ int column_no;
+};
+
+// Keep in sync with the defination in compiler-rt
+// https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_interface_internal.h#L48
+// So far, the current implementation is only compatible with the ASan library from version v7 to v9.
+// https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_init_version.h
+// This structure describes an instrumented global variable.
+//
+// TODO: If a later version of the ASan library changes __asan_global or __asan_global_source_location
+// structure, we need to make the same changes.
+struct _asan_global {
+ uintptr_t beg;
+ uintptr_t size;
+ uintptr_t size_with_redzone;
+ const char *name;
+ const char *module_name;
+ uintptr_t has_dynamic_init;
+ struct _asan_global_source_location *location;
+ uintptr_t odr_indicator;
+};
+
+
+extern void __asan_register_globals(void*, long int);
+
+// Register global variables.
+// The 'globals' is an array of structures describing 'n' globals.
+void __asan_register_globals_go(void *addr, uintptr_t n) {
+ struct _asan_global *globals = (struct _asan_global *)(addr);
+ __asan_register_globals(globals, n);
+}
*/
import "C"
diff --git a/src/runtime/asan0.go b/src/runtime/asan0.go
index d5478d6..0948786 100644
--- a/src/runtime/asan0.go
+++ b/src/runtime/asan0.go
@@ -16,7 +16,8 @@
// Because asanenabled is false, none of these functions should be called.
-func asanread(addr unsafe.Pointer, sz uintptr) { throw("asan") }
-func asanwrite(addr unsafe.Pointer, sz uintptr) { throw("asan") }
-func asanunpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
-func asanpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanread(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanwrite(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanunpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanpoison(addr unsafe.Pointer, sz uintptr) { throw("asan") }
+func asanregisterglobals(addr unsafe.Pointer, sz uintptr) { throw("asan") }
diff --git a/src/runtime/asan_amd64.s b/src/runtime/asan_amd64.s
index 3857350..0489aa8 100644
--- a/src/runtime/asan_amd64.s
+++ b/src/runtime/asan_amd64.s
@@ -61,6 +61,14 @@
MOVQ $__asan_poison_go(SB), AX
JMP asancall<>(SB)
+// func runtime·asanregisterglobals(addr unsafe.Pointer, n uintptr)
+TEXT runtime·asanregisterglobals(SB), NOSPLIT, $0-16
+ MOVD addr+0(FP), RARG0
+ MOVD size+8(FP), RARG1
+ // void __asan_register_globals_go(void *addr, uintptr_t n);
+ MOVD $__asan_register_globals_go(SB), AX
+ JMP asancall<>(SB)
+
// Switches SP to g0 stack and calls (AX). Arguments already set.
TEXT asancall<>(SB), NOSPLIT, $0-0
get_tls(R12)
diff --git a/src/runtime/asan_arm64.s b/src/runtime/asan_arm64.s
index 5ed03c9..697c982 100644
--- a/src/runtime/asan_arm64.s
+++ b/src/runtime/asan_arm64.s
@@ -50,6 +50,14 @@
MOVD $__asan_poison_go(SB), FARG
JMP asancall<>(SB)
+// func runtime·asanregisterglobals(addr unsafe.Pointer, n uintptr)
+TEXT runtime·asanregisterglobals(SB), NOSPLIT, $0-16
+ MOVD addr+0(FP), RARG0
+ MOVD size+8(FP), RARG1
+ // void __asan_register_globals_go(void *addr, uintptr_t n);
+ MOVD $__asan_register_globals_go(SB), FARG
+ JMP asancall<>(SB)
+
// Switches SP to g0 stack and calls (FARG). Arguments already set.
TEXT asancall<>(SB), NOSPLIT, $0-0
MOVD RSP, R19 // callee-saved
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
1 comment:
Patchset:
This issue does not seem to be related to this patch. Thank you.
Ack
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
1 comment:
Patchset:
Thanks.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
mzh (Gerrit)
1 comment:
Patchset:
FYI This CL breaks some builders at TestScript/install_msan_and_race_require_cgo.
https://build.golang.org/log/4c63629b59f4ba8673d2193b936d32a0b371d812
https://build.golang.org/log/c85d3b42d0e8ea96a6db087195dafe0d2e0b7ac7
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
1 comment:
Patchset:
FYI This CL breaks some builders at TestScript/install_msan_and_race_require_cgo. […]
Sorry, it caused by this patch https://go-review.googlesource.com/c/go/+/298612/14. I will send a fix patch. Thank you for reporting this.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
1 comment:
Patchset:
Sorry, it caused by this patch https://go-review.googlesource.com/c/go/+/298612/14. […]
Sorry, I took some time to look at it and found that there is no problem with this patch https://go-review.googlesource.com/c/go/+/298612/14.
It caused by the version of CC, this patch adds some constraints to the CC version. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Bryan Mills has created a revert of this change.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
1 comment:
Patchset:
Sorry, I took some time to look at it and found that there is no problem with this patch https://go- […]
Even if the patch is otherwise correct, the test needs to be updated so that it stops failing. (If the CC version is not compatible, then the test ought to either check the CC version ahead of time, or accept either of the valid error messages for the condition under test.)
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
1 comment:
Patchset:
Even if the patch is otherwise correct, the test needs to be updated so that it stops failing. […]
Thank you for the solution. I will submit a fix ASAP. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Fannie Zhang has uploaded this change for review.
cmd/compile: enable Asan check for global variables
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Updates #44853.
Change-Id: Ib877a817209ab2be68a8e22c418fe4a4a20880fc
---
M misc/cgo/testsanitizers/asan_test.go
M misc/cgo/testsanitizers/cc_test.go
M src/cmd/compile/internal/base/base.go
M src/cmd/compile/internal/gc/obj.go
M src/cmd/compile/internal/noder/noder.go
M src/cmd/compile/internal/noder/object.go
M src/cmd/compile/internal/noder/reader.go
M src/cmd/compile/internal/pkginit/init.go
A src/cmd/compile/internal/pkginit/initAsanGlobals.go
M src/cmd/go/alldocs.go
M src/cmd/go/internal/work/build.go
M src/cmd/go/internal/work/init.go
R src/cmd/go/testdata/script/install_msan_and_race_and_asan_require_cgo.txt
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
18 files changed, 536 insertions(+), 15 deletions(-)
diff --git a/misc/cgo/testsanitizers/asan_test.go b/misc/cgo/testsanitizers/asan_test.go
index ff578ac..23392c0 100644
--- a/misc/cgo/testsanitizers/asan_test.go
+++ b/misc/cgo/testsanitizers/asan_test.go
@@ -22,6 +22,14 @@
if !aSanSupported(goos, goarch) {
t.Skipf("skipping on %s/%s; -asan option is not supported.", goos, goarch)
}
+ // The current implementation is only compatible with the ASan library from version
+ // v7 to v9 (See the description in src/runtime/asan/asan.go). Therefore, using the
+ // -asan option must use a compatible version of ASan library, which requires that
+ // the gcc version is not less than 7 and the clang version is not less than 4,
+ // otherwise a segmentation fault will occur.
+ if !compilerRequiredAsanVersion() {
+ t.Skipf("skipping: too old version of compiler")
+ }
t.Parallel()
requireOvercommit(t)
diff --git a/misc/cgo/testsanitizers/cc_test.go b/misc/cgo/testsanitizers/cc_test.go
index 05b7793..f733903 100644
--- a/misc/cgo/testsanitizers/cc_test.go
+++ b/misc/cgo/testsanitizers/cc_test.go
@@ -184,17 +184,16 @@
var match [][]byte
if bytes.HasPrefix(out, []byte("gcc")) {
compiler.name = "gcc"
-
- cmd, err := cc("-dumpversion")
+ cmd, err := cc("-v")
if err != nil {
return err
}
- out, err := cmd.Output()
+ out, err := cmd.CombinedOutput()
if err != nil {
- // gcc, but does not support gcc's "-dumpversion" flag?!
+ // gcc, but does not support gcc's "-v" flag?!
return err
}
- gccRE := regexp.MustCompile(`(\d+)\.(\d+)`)
+ gccRE := regexp.MustCompile(`gcc version (\d+)\.(\d+)`)
match = gccRE.FindSubmatch(out)
} else {
clangRE := regexp.MustCompile(`clang version (\d+)\.(\d+)`)
@@ -235,6 +234,22 @@
}
}
+// compilerRequiredAsanVersion reports whether the compiler is the version required by Asan.
+func compilerRequiredAsanVersion() bool {
+ compiler, err := compilerVersion()
+ if err != nil {
+ return false
+ }
+ switch compiler.name {
+ case "gcc":
+ return compiler.major >= 7
+ case "clang":
+ return compiler.major >= 4index 26192ec..6cf8b14 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -7,6 +7,7 @@
package work
import (
+ "bytes"
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/fsys"
@@ -14,9 +15,13 @@
"cmd/internal/quoted"
"cmd/internal/sys"
"fmt"
+ exec "internal/execabs"
"os"
"path/filepath"
+ "regexp"
"runtime"
+ "strconv"
+ "sync"
)
func BuildInit() {
@@ -107,6 +112,19 @@
base.SetExitStatus(2)
base.Exit()
}
+ // The current implementation is only compatible with the ASan library from version
+ // v7 to v9 (See the description in src/runtime/asan/asan.go). Therefore, using the
+ // -asan option must use a compatible version of ASan library, which requires that
+ // the gcc version is not less than 7 and the clang version is not less than 4,
+ // otherwise a segmentation fault will occur.
+ if cfg.BuildASan {
+ if !compilerRequiredAsanVersion() {
+ fmt.Fprintf(os.Stderr, "-asan: the version of the compiler is not required\n")
+ base.SetExitStatus(2)
+ base.Exit()
+ }
+ }
+
mode := "race"
if cfg.BuildMSan {
mode = "msan"
@@ -310,3 +328,76 @@
}
}
}
+
+type version struct {
+ name string
+ major, minor int
+}
+
+var compiler struct {
+ sync.Once
+ version
+ err error
+}
+
+// compilerVersion detects the version of $(go env CC).
+// It returns a non-nil error if the compiler matches a known version schema but
+// the version could not be parsed, or if $(go env CC) could not be determined.
+func compilerVersion() (version, error) {
+ compiler.Once.Do(func() {
+ compiler.err = func() error {
+ compiler.name = "unknown"
+ cc := os.Getenv("CC")
+ out, err := exec.Command(cc, "--version").Output()
+ if err != nil {
+ // Compiler does not support "--version" flag: not Clang or GCC.
+ return err
+ }
+
+ var match [][]byte
+ if bytes.HasPrefix(out, []byte("gcc")) {
+ compiler.name = "gcc"
+ out, err := exec.Command(cc, "-v").CombinedOutput()
+ if err != nil {
+ // gcc, but does not support gcc's "-v" flag?!
+ return err
+ }
+ gccRE := regexp.MustCompile(`gcc version (\d+)\.(\d+)`)
+ match = gccRE.FindSubmatch(out)
+ } else {
+ clangRE := regexp.MustCompile(`clang version (\d+)\.(\d+)`)
+ if match = clangRE.FindSubmatch(out); len(match) > 0 {
+ compiler.name = "clang"
+ }
+ }
+
+ if len(match) < 3 {
+ return nil // "unknown"
+ }
+ if compiler.major, err = strconv.Atoi(string(match[1])); err != nil {
+ return err
+ }
+ if compiler.minor, err = strconv.Atoi(string(match[2])); err != nil {
+ return err
+ }
+ return nil
+ }()
+ })
+ return compiler.version, compiler.err
+}
+
+// compilerRequiredAsanVersion is a copy of the function defined in misc/cgo/testsanitizers/cc_test.go
+// compilerRequiredAsanVersion reports whether the compiler is the version required by Asan.
+func compilerRequiredAsanVersion() bool {
+ compiler, err := compilerVersion()
+ if err != nil {
+ return false
+ }
+ switch compiler.name {
+ case "gcc":
+ return compiler.major >= 7
+ case "clang":
+ return compiler.major >= 4
+ }
+ return false
+}
diff --git a/src/cmd/go/testdata/script/install_msan_and_race_require_cgo.txt b/src/cmd/go/testdata/script/install_msan_and_race_and_asan_require_cgo.txt
similarity index 78%
rename from src/cmd/go/testdata/script/install_msan_and_race_require_cgo.txt
rename to src/cmd/go/testdata/script/install_msan_and_race_and_asan_require_cgo.txt
index 5e88f7b..5c3b9fd 100644
--- a/src/cmd/go/testdata/script/install_msan_and_race_require_cgo.txt
+++ b/src/cmd/go/testdata/script/install_msan_and_race_and_asan_require_cgo.txt
@@ -11,7 +11,7 @@
[msan] ! stderr '-race'
[asan] ! go install -asan triv.go
-[asan] stderr '-asan requires cgo'
+[asan] stderr '(-asan: the version of the compiler is not required)|(-asan requires cgo)'
[asan] ! stderr '-msan'
-- triv.go --
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Patch set 1:Run-TryBot +1
1 comment:
Patchset:
TRY=linux-amd64-staticlockranking,linux-amd64-clang
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
2 comments:
Patchset:
1 of 32 SlowBots failed. […]
`fatal error: error in backend: Sanitizer interface function redefined: void (i64, i64)* bitcast (void (i8*, i64)* @__asan_register_globals to void (i64, i64)*)`
I tested TestASAN test locally with different versions of clang (from version 4.0 to version 10.0), only version 9.0 and version 10.0 do not report the above error, other versions will have the same error.
I do not fimilar with this error, so do we skip test for TestASAN when the version is not 9.0 or higher? Thank you.
TRY=linux-arm64,linux-amd64
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
1 comment:
File misc/cgo/testsanitizers/cc_test.go:
Patch Set #1, Line 187:
cc("-v")Here i changed to use `-v` to obtain the version of the compiler, because the version number obtained when using `-dumpversion` is incomplete.
For example:
The output of `gcc-8 -dumpversion` is `8`. The output of `gcc-8 -v` is `gcc version 8.4.0 (Ubuntu/Linaro 8.4.0-1ubuntu1~18.04)`. Then the output of `gcc-8 -dumpversion` will not meet the conditions on line 205, but it is actually the version required by asan. Thank you.
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
1 comment:
Patchset:
Thank you for the solution. I will submit a fix ASAP. Thank you.
Please help to review the this patch https://go-review.googlesource.com/c/go/+/401775 with the fix codes. Thank you.
To view, visit change 321715. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor.
1 comment:
Patchset:
This patch is a new patch of the reverted patch CL 321715, which fixes the error in TestScript/install_msan_and_race_require_cgo.
Could you please help to review it. Thank you.
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor.
Fannie Zhang uploaded patch set #2 to this change.
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
17 files changed, 497 insertions(+), 9 deletions(-)
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor.
Fannie Zhang uploaded patch set #3 to this change.
A src/cmd/go/testdata/script/install_msan_and_race_and_asan_require_cgo.txt
D src/cmd/go/testdata/script/install_msan_and_race_require_cgo.txt
M src/runtime/asan.go
M src/runtime/asan/asan.go
M src/runtime/asan0.go
M src/runtime/asan_amd64.s
M src/runtime/asan_arm64.s
19 files changed, 566 insertions(+), 34 deletions(-)
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor.
1 comment:
Patchset:
Hi all, in order to better review what the new patch has changed, the patch set 2 is the old version, the patch set 3 is the new version. So comparing the patch set 2 (https://go-review.googlesource.com/c/go/+/401775/2..3), we can see the changes. Thank you.
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor.
Patch set 3:Run-TryBot +1
1 comment:
Patchset:
TRY=linux-arm64,linux-amd64
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor, Fannie Zhang.
Patch set 3:Run-TryBot +1Auto-Submit +1Code-Review +2
1 comment:
Patchset:
Thanks.
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Fannie Zhang (Gerrit)
Attention is currently required from: Bryan Mills, mzh, Keith Randall, Ian Lance Taylor.
1 comment:
Patchset:
Thanks.
@Ian Thank you for the review.
You may not have noticed the error when it runs on linux-amd64-clang. Please see https://storage.googleapis.com/go-build-log/78dec488/linux-amd64-clang_1564c3c0.log.
`fatal error: error in backend: Sanitizer interface function redefined: void (i64, i64)* bitcast (void (i8*, i64)* @__asan_register_globals to void (i64, i64)*)`
I tested TestASAN test locally with different versions of clang (from version 4.0 to version 10.0), only `version 9.0` and `version 10.0` do not report the above error, other versions will have the same error.
I am not fimilar with this error, so do we need to skip test for TestASAN when the version of clang is not 9.0 or higher? Thank you.
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: mzh, Keith Randall, Ian Lance Taylor, Fannie Zhang.
4 comments:
File src/cmd/go/internal/work/init.go:
Patch Set #3, Line 121:
compilerRequiredAsanVersion()Please follow the same pattern as the previous checks in this function — return the error up to `instrumentInit` and use the same sequence of calls for logging and exiting.
Patch Set #3, Line 342:
func compilerVersion() (version, error) {In #50982 we found that merely invoking `gcc` can be quite slow on some platforms.
(We're planning to mitigate that by caching; see CL 392454.)Is it really necessary to check the exact compiler version explicitly in `cmd/go`?
(What would happen if we skipped the check entirely? Or, is there some call to `gccSupportsFlag` that we could replace it with?)
Patch Set #3, Line 392:
fmt.Fprintf(os.Stderr, "-asan fails to get C compiler version\n")This failure message is hard for me to parse. Could we use the error returned by compilerVersion directly?
Patch Set #3, Line 410:
fmt.Fprintf(os.Stderr, "-asan requires C compiler is gcc or clang\n")This seems problematic — what stops a third compiler vendor from implementing ASAN support?
(At the very least, a comment in the code explaining the reasoning would be helpful.)
To view, visit change 401775. To unsubscribe, or for help writing mail filters, visit settings.