rosbit/luago

Crash on C.lua_error call

Closed this issue · 13 comments

Hi!

I really like this project so I spend more time on it and noticed that if I have:

type Vector2 struct {
	X, Y float64
}

func New() *Vector2 {
	return &Vector2{}
}

And then try to do:

local v1 = vector2.new()
v1.t = 1

I get:

fatal error: exitsyscall: syscall frame is no longer valid

goroutine 1 gp=0xc0000061c0 m=0 mp=0x644780 [syscall, locked to thread]:
runtime.cgocall(0x4e7c40, 0xc000119928)
..../.gobrew/current/go/src/runtime/cgocall.go:157 +0x4b fp=0xc000119900 sp=0xc0001198c8 pc=0x40b64b
github.com/rosbit/luago._Cfunc_lua_error(0x123e058)
_cgo_gotypes.go:290 +0x47 fp=0xc000119928 sp=0xc000119900 pc=0x4dbf87
github.com/rosbit/luago.go_struct_set.func7(0x123e058)
.../luago-test/luago/go-meta-value.go:351 +0x3b fp=0xc000119960 sp=0xc000119928 pc=0x4e235b
github.com/rosbit/luago.go_struct_set(0x123e058, {0x52a100?, 0xc000014150?, 0x13?})
.../luago-test/luago/go-meta-value.go:351 +0x2f2 fp=0xc000119a18 sp=0xc000119960 pc=0x4e2192
github.com/rosbit/luago.go_obj_set(0x123e058)
.../luago-test/luago/go-meta-value.go:477 +0xe5 fp=0xc000119a58 sp=0xc000119a18 pc=0x4e2ee5
_cgoexp_0f81209084c0_go_obj_set(0x7ffef4783d0c)
_cgo_gotypes.go:653 +0x1e fp=0xc000119a70 sp=0xc000119a58 pc=0x4e6e9e
runtime.cgocallbackg1(0xc0000061c0, 0x43c8e0, 0xc0000061c0)
..../.gobrew/current/go/src/runtime/cgocall.go:403 +0x2a5 fp=0xc000119b30 sp=0xc000119a70 pc=0x40bd85
runtime: g 1: unexpected return pc for runtime.cgocallbackg1 called from 0xc000119b38
stack: frame={sp:0xc000119a70, fp:0xc000119b30} stack=[0xc000118000,0xc00011a000)
0x000000c000119970: 0x0000000000000013 0x000000c000014178
0x000000c000119980: 0x000000c0001199e0 0x00000000004db36c <github.com/rosbit/luago.(*ptrStore).lookup+0x00000000000000cc>
0x000000c000119990: 0x0000000000525a60 0x000000c0001081b0
0x000000c0001199a0: 0x000000c000000004 0x0000000000000001
0x000000c0001199b0: 0x0000000000000199 0x000000000051ec00

Row number might be a bit off since I've edited it a bit. But it's this part:

	fv := structE.FieldByName(name)
	if !fv.IsValid() {
		pushString(ctx, "name not found")
		C.lua_error(ctx) // <-- here
		return 1
	}

Is this something that you can look into?

The idea here is that it should just report that "t" doesn't exist or some sort, but as you can see, this becomes a crash. It would be nice if this was handled more gracefully.

Thanks!

Code v1.t = 1 will cause crash because the base value of v1 is a go struct and it has no field named t. Go struct will not act like lua table. If you want to acquire the ability of lua table, you can make a go map as the base value.

I know it doesn't have the field name t. The point here is to handle it gracefully. Like doing:

	fv := structE.FieldByName(name)
	if !fv.IsValid() {
		pushString(ctx, fmt.Sprintf("%s not found", name))
		C.lua_error(ctx)
		return 1
	}

But the problem is the C.lua_error(ctx) call where we get this crash.

Good suggestion. I update it. Thank you.

But that didn't fix the crash.

Crash occurs only in development scenarios. In production environment, such codes like v1.t = 1 should be revised or removed.
If no crash occurs, I am afraid that bad assigning action will be ignored easily, and luago doesn't know how to process the rest codes.

So, the idea is that if someone writes Lua code that uses a member in a Go struct that doesn't exist, one should get a nice little error message about it, not crash. Is there no way to do this so it doesn't crash?

lua_error in cgo will crash. After all there's difference between C and cgo.
Do you want to use luago in application for users who can write lua codes? In such case, my suggestion is pure go lua implementation is your choice.
For myself, both application and embedding lua codes are written by myself. So lua_error doesn't matter me.

Can you explain why it crashes? Why would stacking an error message crash the Lua runtime? Shouldn't the stacking of a message and calling C.lua_error just print some stack trace and exit(1)?

Good idea. Stack tracing message is printed before calling C.lua_error.

Yay! That seems to work! No crash now!

Edit: Scratch that. I didn't have the C.lua_error(ctx) call in my test here. If I remove that call it works fine. It still crashes if the call to C.lua_error(ctx) is made.

------
key "t" not found
stack traceback:
	vector-test.lua:9: in main chunk
------
fatal error: exitsyscall: syscall frame is no longer valid

goroutine 1 gp=0xc0000061c0 m=0 mp=0x6427a0 [syscall, locked to thread]:
runtime.cgocall(0x4e6b40, 0xc000117960)
...

So the output works but as soon as we call C.lua_error(ctx) we get the crash.

Yay! That seems to work! No crash now!

Edit: Scratch that. I didn't have the C.lua_error(ctx) call in my test here. If I remove that call it works fine. It still crashes if the call to C.lua_error(ctx) is made.

That's true. If you don't want to crash in such cases, you can fork the codes and remove the C.lua_error(ctx).

Will do! Thanks!