Patchwork [go] : Do not panic in test/nilptr.go

login
register
mail settings
Submitter Uros Bizjak
Date Jan. 30, 2012, 7:19 p.m.
Message ID <CAFULd4Y2HL373SCnob9Ppd0A7ezPzQ1Xku=TgjNjSPw==j-GWA@mail.gmail.com>
Download mbox | patch
Permalink /patch/138623/
State New
Headers show

Comments

Uros Bizjak - Jan. 30, 2012, 7:19 p.m.
Hello!

There is no need for a panic in test/nilptr.go if array doesn't get
allocated in first 256 meg of memory. The compiler has nothing to do
with this.


Tested on alphaev68-pc-linux-gnu where it "fixes" the failure.

After proposed patch(es), the only remaining failure on
alpha-linux-gnu is in test/chan/select2.go:

FAIL: go.test/test/chan/select2.go execution,  -O2 -g

with

Executing on host: /space/uros/gcc-build/gcc/testsuite/go2/../../gccgo
-B/space/uros/gcc-build/gcc/testsuite/go2/../../
/home/uros/gcc-svn/trunk/gcc/testsuite/go.test/test/chan/select2.go
-I/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo  -w  -O2
-g   -L/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo
-L/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo/.libs  -lm
  -o /space/uros/gcc-build/gcc/testsuite/go2/select2.x    (timeout =
300)
spawn /space/uros/gcc-build/gcc/testsuite/go2/../../gccgo
-B/space/uros/gcc-build/gcc/testsuite/go2/../../
/home/uros/gcc-svn/trunk/gcc/testsuite/go.test/test/chan/select2.go
-I/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo -w -O2 -g
-L/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo
-L/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo/.libs -lm
-o /space/uros/gcc-build/gcc/testsuite/go2/select2.x^M
PASS: go.test/test/chan/select2.go compilation,  -O2 -g
Setting LD_LIBRARY_PATH to
.:/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo/.libs:/home/uros/gcc-build/gcc:.:/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo/.libs:/home/uros/gcc-build/gcc
spawn [open ...]^M
BUG: too much memory for 100,000 selects: 2098576
FAIL: go.test/test/chan/select2.go execution,  -O2 -g

The test works without problems otherwise. I suspect that memory
consumption threshold is set way too low...

Uros.
Ian Taylor - Jan. 31, 2012, 7:36 p.m.
Uros Bizjak <ubizjak@gmail.com> writes:

> There is no need for a panic in test/nilptr.go if array doesn't get
> allocated in first 256 meg of memory. The compiler has nothing to do
> with this.

This is true but this does not seem like the right patch.


> Index: test/nilptr.go
> ===================================================================
> --- test/nilptr.go      (revision 183732)
> +++ test/nilptr.go      (working copy)
> @@ -22,7 +22,8 @@
>         // at the address that might be accidentally
>         // dereferenced below.
>         if uintptr(unsafe.Pointer(&dummy)) > 256<<20 {
> -               panic("dummy too far out")
> +               println("dummy too far out")
> +               return
>         }
>
>         shouldPanic(p1)


This patch makes this test pretty much useless: if the linker changes,
the test will always pass, and nobody will ever notice that the test is
no longer being run.

I think I would prefer to just change go-test.exp to mark this test as
xfail on Alpha.  A patch to do that is preapproved.

Ian

Patch

Index: test/nilptr.go
===================================================================
--- test/nilptr.go      (revision 183732)
+++ test/nilptr.go      (working copy)
@@ -22,7 +22,8 @@ 
        // at the address that might be accidentally
        // dereferenced below.
        if uintptr(unsafe.Pointer(&dummy)) > 256<<20 {
-               panic("dummy too far out")
+               println("dummy too far out")
+               return
        }

        shouldPanic(p1)