Skip to content

Commit

Permalink
all: skip and fix various tests with -asan and -msan
Browse files Browse the repository at this point in the history
First, skip all the allocation count tests.

In some cases this aligns with existing skips for -race, but in others
we've got new issues. These are debug modes, so some performance loss is
expected, and this is clearly no worse than today where the tests fail.

Next, skip internal linking and static linking tests for msan and asan.

With asan we get an explicit failure that neither are supported by the C
and/or Go compilers. With msan, we only get the Go compiler telling us
internal linking is unavailable. With static linking, we segfault
instead. Filed #70080 to track that.

Next, skip some malloc tests with asan that don't quite work because of
the redzone.

This is because of some sizeclass assumptions that get broken with the
redzone and the fact that the tiny allocator is effectively disabled
(again, due to the redzone).

Next, skip some runtime/pprof tests with asan, because of extra
allocations.

Next, skip some malloc tests with asan that also fail because of extra
allocations.

Next, fix up memstats accounting for arenas when asan is enabled. There
is a bug where more is added to the stats than subtracted. This also
simplifies the accounting a little.

Next, skip race tests with msan or asan enabled; they're mutually
incompatible.

Fixes #70054.
Fixes #64256.
Fixes #64257.
For #70079.
For #70080.

Change-Id: I99c02a0b9d621e44f1f918b307aa4a4944c3ec60
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15,gotip-linux-amd64-msan-clang15
Reviewed-on: https://go-review.googlesource.com/c/go/+/622855
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
  • Loading branch information
mknyszek committed Oct 28, 2024
1 parent 808da68 commit 579eb79
Show file tree
Hide file tree
Showing 22 changed files with 151 additions and 26 deletions.
4 changes: 4 additions & 0 deletions src/bufio/bufio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"bytes"
"errors"
"fmt"
"internal/asan"
"io"
"math/rand"
"strconv"
Expand Down Expand Up @@ -585,6 +586,9 @@ func TestWriteInvalidRune(t *testing.T) {
}

func TestReadStringAllocs(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}
r := strings.NewReader(" foo foo 42 42 42 42 42 42 42 42 4.2 4.2 4.2 4.2\n")
buf := NewReader(r)
allocs := testing.AllocsPerRun(100, func() {
Expand Down
4 changes: 4 additions & 0 deletions src/cmd/cgo/internal/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,7 @@ import "C"
import (
"context"
"fmt"
"internal/asan"
"math"
"math/rand"
"os"
Expand Down Expand Up @@ -1773,6 +1774,9 @@ func issue8331a() C.issue8331 {
// issue 10303

func test10303(t *testing.T, n int) {
if asan.Enabled {
t.Skip("variable z is heap-allocated due to extra allocations with -asan; see #70079")
}
if runtime.Compiler == "gccgo" {
t.Skip("gccgo permits C pointers on the stack")
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/test/issue53888_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build !race
//go:build !race && !asan && !msan

package test

Expand Down
21 changes: 17 additions & 4 deletions src/cmd/dist/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,8 @@ func (t *tester) registerTests() {
}
}

if t.raceDetectorSupported() {
if t.raceDetectorSupported() && !t.msan && !t.asan {
// N.B. -race is incompatible with -msan and -asan.
t.registerRaceTests()
}

Expand Down Expand Up @@ -1090,10 +1091,18 @@ func (t *tester) internalLink() bool {
// linkmode=internal isn't supported.
return false
}
if t.msan || t.asan {
// linkmode=internal isn't supported by msan or asan.
return false
}
return true
}

func (t *tester) internalLinkPIE() bool {
if t.msan || t.asan {
// linkmode=internal isn't supported by msan or asan.
return false
}
switch goos + "-" + goarch {
case "darwin-amd64", "darwin-arm64",
"linux-amd64", "linux-arm64", "linux-ppc64le",
Expand Down Expand Up @@ -1232,18 +1241,22 @@ func (t *tester) registerCgoTests(heading string) {
}

// Static linking tests
if goos != "android" && p != "netbsd/arm" {
if goos != "android" && p != "netbsd/arm" && !t.msan && !t.asan {
// TODO(#56629): Why does this fail on netbsd-arm?
// TODO(#70080): Why does this fail with msan?
// asan doesn't support static linking (this is an explicit build error on the C side).
cgoTest("static", "testtls", "external", "static", staticCheck)
}
cgoTest("external", "testnocgo", "external", "", staticCheck)
if goos != "android" {
if goos != "android" && !t.msan && !t.asan {
// TODO(#70080): Why does this fail with msan?
// asan doesn't support static linking (this is an explicit build error on the C side).
cgoTest("static", "testnocgo", "external", "static", staticCheck)
cgoTest("static", "test", "external", "static", staticCheck)
// -static in CGO_LDFLAGS triggers a different code path
// than -static in -extldflags, so test both.
// See issue #16651.
if goarch != "loong64" {
if goarch != "loong64" && !t.msan && !t.asan {
// TODO(#56623): Why does this fail on loong64?
cgoTest("auto-static", "test", "auto", "static", staticCheck)
}
Expand Down
6 changes: 4 additions & 2 deletions src/crypto/rand/rand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"compress/flate"
"crypto/internal/boring"
"errors"
"internal/asan"
"internal/msan"
"internal/race"
"internal/testenv"
"io"
Expand Down Expand Up @@ -155,8 +157,8 @@ func TestAllocations(t *testing.T) {
// Might be fixable with https://go.dev/issue/56378.
t.Skip("boringcrypto allocates")
}
if race.Enabled {
t.Skip("urandomRead allocates under -race")
if race.Enabled || msan.Enabled || asan.Enabled {
t.Skip("urandomRead allocates under -race, -asan, and -msan")
}
testenv.SkipIfOptimizationOff(t)

Expand Down
4 changes: 4 additions & 0 deletions src/database/sql/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package sql
import (
"database/sql/driver"
"fmt"
"internal/asan"
"reflect"
"runtime"
"strings"
Expand Down Expand Up @@ -353,6 +354,9 @@ func TestRawBytesAllocs(t *testing.T) {
{"bool", false, "false"},
{"time", time.Unix(2, 5).UTC(), "1970-01-01T00:00:02.000000005Z"},
}
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}

var buf RawBytes
rows := &Rows{}
Expand Down
7 changes: 7 additions & 0 deletions src/encoding/binary/binary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package binary
import (
"bytes"
"fmt"
"internal/asan"
"io"
"math"
"reflect"
Expand Down Expand Up @@ -710,6 +711,9 @@ func TestNoFixedSize(t *testing.T) {
}

func TestAppendAllocs(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}
buf := make([]byte, 0, Size(&s))
var err error
allocs := testing.AllocsPerRun(1, func() {
Expand Down Expand Up @@ -745,6 +749,9 @@ var sizableTypes = []any{
}

func TestSizeAllocs(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}
for _, data := range sizableTypes {
t.Run(fmt.Sprintf("%T", data), func(t *testing.T) {
// Size uses a sync.Map behind the scenes. The slow lookup path of
Expand Down
4 changes: 4 additions & 0 deletions src/log/slog/attr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@
package slog

import (
"internal/asan"
"internal/testenv"
"testing"
"time"
)

func TestAttrNoAlloc(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates with -asan")
}
testenv.SkipIfOptimizationOff(t)
// Assign values just to make sure the compiler doesn't optimize away the statements.
var (
Expand Down
6 changes: 4 additions & 2 deletions src/log/slog/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package slog
import (
"bytes"
"context"
"internal/asan"
"internal/msan"
"internal/race"
"internal/testenv"
"io"
Expand Down Expand Up @@ -644,8 +646,8 @@ func callerPC(depth int) uintptr {
}

func wantAllocs(t *testing.T, want int, f func()) {
if race.Enabled {
t.Skip("skipping test in race mode")
if race.Enabled || asan.Enabled || msan.Enabled {
t.Skip("skipping test in race, asan, and msan modes")
}
testenv.SkipIfOptimizationOff(t)
t.Helper()
Expand Down
5 changes: 5 additions & 0 deletions src/log/slog/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package slog

import (
"fmt"
"internal/asan"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -86,6 +87,10 @@ func TestValueString(t *testing.T) {
}

func TestValueNoAlloc(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}

// Assign values just to make sure the compiler doesn't optimize away the statements.
var (
i int64
Expand Down
5 changes: 5 additions & 0 deletions src/net/netip/netip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/json"
"flag"
"fmt"
"internal/asan"
"internal/testenv"
"net"
. "net/netip"
Expand Down Expand Up @@ -2132,6 +2133,10 @@ var (
)

func TestNoAllocs(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}

// Wrappers that panic on error, to prove that our alloc-free
// methods are returning successfully.
panicIP := func(ip Addr, err error) Addr {
Expand Down
4 changes: 4 additions & 0 deletions src/net/udpsock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package net
import (
"errors"
"fmt"
"internal/asan"
"internal/testenv"
"net/netip"
"os"
Expand Down Expand Up @@ -493,6 +494,9 @@ func TestAllocs(t *testing.T) {
if !testableNetwork("udp4") {
t.Skipf("skipping: udp4 not available")
}
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}

// Optimizations are required to remove the allocs.
testenv.SkipIfOptimizationOff(t)
Expand Down
15 changes: 14 additions & 1 deletion src/reflect/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"flag"
"fmt"
"go/token"
"internal/asan"
"internal/goarch"
"internal/goexperiment"
"internal/testenv"
Expand Down Expand Up @@ -1278,6 +1279,9 @@ func TestDeepEqualAllocs(t *testing.T) {
if goexperiment.SwissMap {
t.Skipf("Maps on stack not yet implemented")
}
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}

for _, tt := range deepEqualPerfTests {
t.Run(ValueOf(tt.x).Type().String(), func(t *testing.T) {
Expand Down Expand Up @@ -7353,6 +7357,9 @@ func TestPtrToMethods(t *testing.T) {
}

func TestMapAlloc(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}
m := ValueOf(make(map[int]int, 10))
k := ValueOf(5)
v := ValueOf(7)
Expand Down Expand Up @@ -7383,6 +7390,9 @@ func TestMapAlloc(t *testing.T) {
}

func TestChanAlloc(t *testing.T) {
if asan.Enabled {
t.Skip("test allocates more with -asan; see #70079")
}
// Note: for a chan int, the return Value must be allocated, so we
// use a chan *int instead.
c := ValueOf(make(chan *int, 1))
Expand Down Expand Up @@ -7745,11 +7755,14 @@ func TestMapIterReset(t *testing.T) {
}

// Reset should not allocate.
//
// Except with -asan, where there are additional allocations.
// See #70079.
n := int(testing.AllocsPerRun(10, func() {
iter.Reset(ValueOf(m2))
iter.Reset(Value{})
}))
if n > 0 {
if !asan.Enabled && n > 0 {
t.Errorf("MapIter.Reset allocated %d times", n)
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/runtime/arena.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,11 +798,8 @@ func newUserArenaChunk() (unsafe.Pointer, *mspan) {

if asanenabled {
// TODO(mknyszek): Track individual objects.
rzSize := redZoneSize(span.elemsize)
span.elemsize -= rzSize
span.largeType.Size_ = span.elemsize
// N.B. span.elemsize includes a redzone already.
rzStart := span.base() + span.elemsize
span.userArenaChunkFree = makeAddrRange(span.base(), rzStart)
asanpoison(unsafe.Pointer(rzStart), span.limit-rzStart)
asanunpoison(unsafe.Pointer(span.base()), span.elemsize)
}
Expand Down Expand Up @@ -1067,6 +1064,11 @@ func (h *mheap) allocUserArenaChunk() *mspan {
s.freeindex = 1
s.allocCount = 1

// Adjust size to include redzone.
if asanenabled {
s.elemsize -= redZoneSize(s.elemsize)
}

// Account for this new arena chunk memory.
gcController.heapInUse.add(int64(userArenaChunkBytes))
gcController.heapReleased.add(-int64(userArenaChunkBytes))
Expand Down
10 changes: 10 additions & 0 deletions src/runtime/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package runtime_test
import (
"fmt"
"internal/abi"
"internal/asan"
"internal/msan"
"math"
"os"
"regexp"
Expand All @@ -32,6 +34,14 @@ func startDebugCallWorker(t *testing.T) (g *runtime.G, after func()) {
// a debugger.
skipUnderDebugger(t)

// asan/msan instrumentation interferes with tests since we might
// inject debugCallV2 while in the asan/msan runtime. This is a
// problem for doing things like running the GC or taking stack
// traces. Not sure why this is happening yet, but skip for now.
if msan.Enabled || asan.Enabled {
t.Skip("debugCallV2 is injected erroneously during asan/msan runtime calls; skipping")
}

// This can deadlock if there aren't enough threads or if a GC
// tries to interrupt an atomic loop (see issue #10958). Execute
// an extra GC to ensure even the sweep phase is done (out of
Expand Down
7 changes: 7 additions & 0 deletions src/runtime/gc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package runtime_test

import (
"fmt"
"internal/asan"
"math/bits"
"math/rand"
"os"
Expand Down Expand Up @@ -208,6 +209,9 @@ func TestGcZombieReporting(t *testing.T) {
}

func TestGCTestMoveStackOnNextCall(t *testing.T) {
if asan.Enabled {
t.Skip("extra allocations with -asan causes this to fail; see #70079")
}
t.Parallel()
var onStack int
// GCTestMoveStackOnNextCall can fail in rare cases if there's
Expand Down Expand Up @@ -298,6 +302,9 @@ var pointerClassBSS *int
var pointerClassData = 42

func TestGCTestPointerClass(t *testing.T) {
if asan.Enabled {
t.Skip("extra allocations cause this test to fail; see #70079")
}
t.Parallel()
check := func(p unsafe.Pointer, want string) {
t.Helper()
Expand Down
Loading

0 comments on commit 579eb79

Please sign in to comment.