rs / zerolog

Zero Allocation JSON Logger
MIT License
10.62k stars 572 forks source link

Panic with diode in 32-bit arch #228

Closed jsdidierlaurent closed 4 years ago

jsdidierlaurent commented 4 years ago

Hi!

Diode panic when using zerolog in 32-bit arch.

 GOARCH=386 go test ./...
# github.com/rs/zerolog/internal/cbor [github.com/rs/zerolog/internal/cbor.test]
internal/cbor/types_test.go:91:3: constant 4294967294 overflows int
internal/cbor/types_test.go:94:3: constant 3022355601293312 overflows int
internal/cbor/types_test.go:95:3: constant 1000000000000 overflows int
internal/cbor/types_test.go:119:3: constant -4294967294 overflows int
internal/cbor/types_test.go:122:3: constant -3022355601293313 overflows int
internal/cbor/types_test.go:123:3: constant -1000000000001 overflows int
ok      github.com/rs/zerolog   (cached)
?       github.com/rs/zerolog/cmd/lint  [no test files]
--- FAIL: TestNewWriter (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x804a05c]

goroutine 6 [running]:
testing.tRunner.func1.1(0x8162b60, 0x82751d8)
        /opt/golang/current/src/testing/testing.go:941 +0x331
testing.tRunner.func1(0x851a0a0)
        /opt/golang/current/src/testing/testing.go:944 +0x349
panic(0x8162b60, 0x82751d8)
        /opt/golang/current/src/runtime/panic.go:967 +0x122
runtime/internal/atomic.Xadd64(0x84640cc, 0x1, 0x0, 0x8520200, 0x0)
        /opt/golang/current/src/runtime/internal/atomic/asm_386.s:105 +0xc
github.com/rs/zerolog/diode/internal/diodes.(*ManyToOne).Set(0x84640c0, 0x840c070)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/diode/internal/diodes/many_to_one.go:43 +0x3a
github.com/rs/zerolog/diode/internal/diodes.(*Waiter).Set(0x8410820, 0x840c070)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/diode/internal/diodes/waiter.go:51 +0x2e
github.com/rs/zerolog/diode.Writer.Write(0x81a9160, 0x84107e0, 0x81a9ec0, 0x8410820, 0x840e3a8, 0x84222c0, 0x8520000, 0x23, 0x1f4, 0x84660b0, ...)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/diode/diode.go:78 +0xe2
github.com/rs/zerolog.levelWriterAdapter.WriteLevel(...)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/writer.go:20
github.com/rs/zerolog.(*Event).write(0x84640f0, 0x1c, 0x1f4)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/event.go:76 +0xec
github.com/rs/zerolog.(*Event).msg(0x84640f0, 0x8414320, 0x4)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/event.go:140 +0x17c
github.com/rs/zerolog.(*Event).Msg(...)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/event.go:106
github.com/rs/zerolog.(*Logger).Print(0x843c74c, 0x843c6fc, 0x1, 0x1)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/log.go:387 +0x7c
github.com/rs/zerolog/diode_test.TestNewWriter(0x851a0a0)
        /home/jsdidierlaurent/Workspace/github.com/jsdidierlaurent/zerolog/diode/diode_test.go:23 +0x16d
testing.tRunner(0x851a0a0, 0x8189e64)
        /opt/golang/current/src/testing/testing.go:992 +0xb4
created by testing.(*T).Run
        /opt/golang/current/src/testing/testing.go:1043 +0x2ad
FAIL    github.com/rs/zerolog/diode     0.006s
?       github.com/rs/zerolog/diode/internal/diodes     [no test files]
ok      github.com/rs/zerolog/hlog      (cached)
?       github.com/rs/zerolog/hlog/internal/mutil       [no test files]
FAIL    github.com/rs/zerolog/internal/cbor [build failed]
?       github.com/rs/zerolog/internal/cbor/examples    [no test files]
ok      github.com/rs/zerolog/internal/json     (cached)
ok      github.com/rs/zerolog/journald  (cached)

There is a note in golang docs about Atomic pkg in 32-bit arch: https://golang.org/pkg/sync/atomic/#pkg-note-BUG

I already saw someone fix this kind of issue like this: https://github.com/valyala/fasthttp/pull/451 But it's a bit strange ...

jsdidierlaurent commented 4 years ago

I try a fix in fork. Changing variables order fix this issue ...


type ManyToOne struct {
    writeIndex uint64
    readIndex  uint64
    buffer     []unsafe.Pointer
    alerter    Alerter
}```
jsdidierlaurent commented 4 years ago

Note: there is also overflow issue mentionned by #163 in my test logs