relab/hotstuff

--list-modules CLI flag produces segfault

Closed this issue · 1 comments

Lavode commented

The --list-modules CLI flag produces a segfault:

› make && ./hotstuff --list-modules
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xe8 pc=0x70ca1c]

goroutine 1 [running]:
github.com/relab/hotstuff/modules.ListModules()
	/home/michael/dev/go/hotstuff/modules/registry.go:89 +0x1dc
github.com/relab/hotstuff/internal/cli.glob..func1(0x11be760?, {0xc1f2a6?, 0x1?, 0x1?})
	/home/michael/dev/go/hotstuff/internal/cli/root.go:37 +0x34
github.com/spf13/cobra.(*Command).execute(0x11be760, {0xc0001a8010, 0x1, 0x1})
	/home/michael/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:856 +0x67c
github.com/spf13/cobra.(*Command).ExecuteC(0x11be760)
	/home/michael/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
	/home/michael/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902
github.com/relab/hotstuff/internal/cli.Execute()
	/home/michael/dev/go/hotstuff/internal/cli/root.go:52 +0x25
main.main()
	/home/michael/dev/go/hotstuff/cmd/hotstuff/main.go:7 +0x17

Following the stack trace shows it must be due to the t variable, a key of the byInterface map, being <nil>: https://github.com/relab/hotstuff/blob/master/modules/registry.go#L89

The queried datastructure (byInterface) is only filled within here, where the offending map key is called moduleType: https://github.com/relab/hotstuff/blob/master/modules/registry.go#L31-L34

moduleType is set in the same function as moduleType := reflect.TypeOf(t). Here, var t T is (the zero value of) a variable of type T. T is seemingly the (generic) return type of the constructor of the module which is being registered.

Which brings us to the likely cause. The return type of at least some of the registered modules' constructors is an interface, e.g here: https://github.com/relab/hotstuff/blob/master/consensus/byzantine/byzantine.go#L11

Thus var t T has the zero value of an interface, which is <nil>. moduleType := reflect.TypeOf(t) returns nil as t is a nil interface. This ends up in the byInterface map. And eventually it blows up when one tries to access it as something of type reflect.Type.

I don't understand why you seemingly keep modules grouped by their constructor's return value in the byInterface data structure, and why you want to print them that way in the --list-modules call, so am hesitant to recommend a fix.

johningve commented

Thank's for looking into this!

The idea is that when someone uses the RegisterModule function, the constructor function will decide which interface will be visible to the module system, through the return value's type. You can see this when using --list-modules once it is fixed :)

❯ ./hotstuff --list-modules
(github.com/relab/hotstuff/consensus).Rules :
         chainedhotstuff
         fasthotstuff
         simplehotstuff
(github.com/relab/hotstuff/modules).Handel :
         handel
(github.com/relab/hotstuff/modules).LeaderRotation :
         round-robin
         carousel
         fixed
         reputation
(github.com/relab/hotstuff/modules).CryptoBase :
         bls12
         ecdsa
(github.com/relab/hotstuff/consensus/byzantine).Byzantine :
         silence
         fork
  • 👍1