Skip to content
This repository has been archived by the owner on May 11, 2020. It is now read-only.

Fix type conversion panic for host-provided funcs #178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix type conversion panic for host-provided funcs #178

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion exec/native_compile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (vm *VM) tryNativeCompile() error {
}

for i := range vm.funcs {
if _, isGoFunc := vm.funcs[i].(*goFunction); isGoFunc {
if _, isGoFunc := vm.funcs[i].(goFunction); isGoFunc {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum...
this kind of type assertion, using the "comma-ok" variant, shouldn't panic.
so I must admit I am not completely clear on what panic this should cure.

that said, it seems indeed that we should probably type-assert for goFunction:

$> git grep goFunction
exec/func.go:38:type goFunction struct {
exec/func.go:43:func (fn goFunction) call(vm *VM, index int64) {
exec/native_compile.go:94:              if _, isGoFunc := vm.funcs[i].(*goFunction); isGoFunc {
exec/native_compile.go:181:             if _, isGoFunc := vm.funcs[i].(*goFunction); isGoFunc {
exec/vm.go:130:                 vm.funcs[i] = goFunction{

(as we only ever fill vm.funcs with goFunction{...} values...)

Copy link
Author

@ericflo ericflo Nov 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my fault for underspecifying - it passes this check, so in the code below it (line 98) it panics trying to access it as a compiledFunction

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I dont understand how we would hit this panic. Consider that vm.funcs is of type []function, and function is an interface type. The goFunction type only implements the necessary interface methods on itself, it does not implement it on its pointer receiver.

As such, I was thinking only goFunction{} could implement function, not a *goFunction ? Now I'm not sure ...

Either way, the current logic is negated, considering we only want to run the loop logic on the func if it is of type compiledFunction.

What do you think of a fix like this?

fn, isCompiledFn := vm.funcs[i].(compiledFunction)
if !isCompiledFn {
  continue
}

continue
}

Expand Down