log/slog: nil pointer dereference when printing nil regexp

[romaindoumenc]

What version of Go are you using (go version)?

$ go1.21rc3 linux/amd64

Does this issue reproduce with the latest release?

Yes – this is indeed a regression from using go1.20 + exp/slog

What did you do?

Log in a simple nil value using slog
https://go.dev/play/p/jAg4xDbs4sn?v=gotip

What did you expect to see?

A formatted nil line, as per https://go.dev/play/p/b7Vjf1L09sC

2009/11/10 23:00:00 INFO priting re=<nil>

What did you see instead?

Crash, with nil pointer deref

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x4afbee]

goroutine 1 [running]:
regexp.(*Regexp).String(...)
	/usr/local/go-faketime/src/regexp/regexp.go:110
regexp.(*Regexp).MarshalText(0x0?)
	/usr/local/go-faketime/src/regexp/regexp.go:1294 +0xe
log/slog.appendTextValue(0xc000051a60, {{}, 0x4cf000?, {0x4cf000?, 0x0?}})
	/usr/local/go-faketime/src/log/slog/text_handler.go:106 +0x28e
log/slog.(*handleState).appendValue(0xc000051a60, {{}, 0x4d2164?, {0x4cf000?, 0x0?}})
	/usr/local/go-faketime/src/log/slog/handler.go:519 +0x31
log/slog.(*handleState).appendAttr(0xc000051a60, {{0x4d2164, 0x2}, {{}, 0x0, {0x4cf000, 0x0}}})
	/usr/local/go-faketime/src/log/slog/handler.go:475 +0x7ea
log/slog.(*handleState).appendNonBuiltIns.func1({{0x4d2164, 0x2}, {{}, 0x0, {0x4cf000, 0x0}}})
	/usr/local/go-faketime/src/log/slog/handler.go:326 +0x49
log/slog.Record.Attrs({{0xbab699fc00000000, 0x1, 0x586660}, {0x4d2586, 0x7}, 0x0, 0x4afd1c, {{{0x4d2164, 0x2}, {{...}, ...}}, ...}, ...}, ...)
	/usr/local/go-faketime/src/log/slog/record.go:84 +0xc4
log/slog.(*handleState).appendNonBuiltIns(_, {{0xbab699fc00000000, 0x1, 0x586660}, {0x4d2586, 0x7}, 0x0, 0x4afd1c, {{{0x4d2164, 0x2}, ...}, ...}, ...})
	/usr/local/go-faketime/src/log/slog/handler.go:325 +0x465
log/slog.(*defaultHandler).Handle(_, {_, _}, {{0xbab699fc00000000, 0x1, 0x586660}, {0x4d2586, 0x7}, 0x0, 0x4afd1c, ...})
	/usr/local/go-faketime/src/log/slog/handler.go:115 +0x385
log/slog.(*Logger).log(0xc000014090, {0x4f75f0, 0x5b3f80}, 0x0, {0x4d2586, 0x7}, {0xc00007af10, 0x2, 0x2})
	/usr/local/go-faketime/src/log/slog/logger.go:225 +0x1e4
log/slog.Info({0x4d2586?, 0xc00003e730?}, {0xc00003e710?, 0x60?, 0x0?})
	/usr/local/go-faketime/src/log/slog/logger.go:260 +0x91
main.main()
	/tmp/sandbox2138239418/prog.go:10 +0x5c

Program exited.

[romaindoumenc]

I’m assuming the crash is linked to the new regexp implementation of MarshalText:

+
+// MarshalText implements [encoding.TextMarshaler]. The output
+// matches that of calling the [Regexp.String] method.
+//
+// Note that the output is lossy in some cases: This method does not indicate
+// POSIX regular expressions (i.e. those compiled by calling [CompilePOSIX]), or
+// those for which the [Regexp.Longest] method has been called.
+func (re *Regexp) MarshalText() ([]byte, error) {
+       return []byte(re.String()), nil
+}
+
+// UnmarshalText implements [encoding.TextUnmarshaler] by calling
+// [Compile] on the encoded value.
+func (re *Regexp) UnmarshalText(text []byte) error {
+       newRE, err := Compile(string(text))
+       if err != nil {
+               return err
+       }
+       *re = *newRE
+       return nil
+}

Logging is a place where we really want to have nil values printed (indeed, this can be valuable for program debugging), so my sense is that log/slog should be robust against nil values.

[seankhliao]

cc @jba

[jba]

Agreed, slog should catch panics when formatting, like the fmt functions do.

I changed the title of this issue to reflect that.

[gopherbot]

Change https://go.dev/cl/514135 mentions this issue: log/slog: print nil values instead of panic