berry-lang/berry

SIGTRAP when calling function returned from member function in class

ElfEars opened this issue · 11 comments

Edit (02/12/2022): Updated the code
The following code causes a SIGTRAP. Tested in both 1.1.0 and the current master branch.

def testfunc(arg)
	
	print(arg)
	
end

class TestClass
	
	
	def member(name)
		
		return testfunc
		
	end
	
	def setmember(name, value)
		
		
		
	end
	
	
end


var hey = TestClass("test")

hey.test()

gdb reports a use after free but that is not what ultimately kills the process
Stack trace is as follows:

#0  0x77d66ad7 in ntdll!RtlCaptureStackContext () from C:\WINDOWS\SysWOW64\ntdll.dll
#1  0x77d116b7 in ntdll!RtlAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#2  0x77d113ee in ntdll!RtlAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#3  0x77db518c in ntdll!RtlpNtSetValueKey () from C:\WINDOWS\SysWOW64\ntdll.dll
#4  0x77d667ea in ntdll!RtlCaptureStackContext () from C:\WINDOWS\SysWOW64\ntdll.dll
#5  0x77d116b7 in ntdll!RtlAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#6  0x77d113ee in ntdll!RtlAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#7  0x77d0ec4c in ntdll!RtlReAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#8  0x77d0e0a0 in ntdll!RtlReAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#9  0x77d0da33 in ntdll!RtlReAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#10 0x77db6037 in ntdll!RtlpNtSetValueKey () from C:\WINDOWS\SysWOW64\ntdll.dll
#11 0x77d0e4b7 in ntdll!RtlReAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#12 0x77d0e0a0 in ntdll!RtlReAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#13 0x77d0da33 in ntdll!RtlReAllocateHeap () from C:\WINDOWS\SysWOW64\ntdll.dll
#14 0x76d675f9 in realloc () from C:\WINDOWS\SysWOW64\msvcrt.dll
#15 0x03870000 in ?? ()
#16 0x0018a1ad in be_realloc (vm=0x38716b8, ptr=0x38733b8, old_size=576, new_size=672) at src/be_mem.c:100
#17 0x00198678 in be_vector_push (vm=0x38716b8, vector=0x38716f4, data=0x0) at src/be_vector.c:42
#18 0x00198d77 in precall (vm=0x38716b8, func=0x3873900, nstack=1, mode=0) at src/be_vm.c:223
#19 0x0019e5a1 in vm_exec (vm=0x38716b8) at src/be_vm.c:1141
#20 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#21 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x38738e0, argc=1) at src/be_vm.c:1286
#22 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#23 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#24 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#25 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#26 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#27 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#28 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x38738a0, argc=1) at src/be_vm.c:1286
#29 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#30 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#31 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#32 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#33 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#34 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#35 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3873058, argc=1) at src/be_vm.c:1286
#36 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#37 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#38 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#39 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#40 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#41 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#42 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3873018, argc=1) at src/be_vm.c:1286
#43 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#44 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#45 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#46 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#47 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#48 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#49 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872fd8, argc=1) at src/be_vm.c:1286
#50 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#51 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#52 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#53 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#54 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#55 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#56 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872f98, argc=1) at src/be_vm.c:1286
#57 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#58 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#59 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#60 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#61 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#62 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#63 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872f58, argc=1) at src/be_vm.c:1286
#64 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#65 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#66 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#67 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#68 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#69 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#70 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872f18, argc=1) at src/be_vm.c:1286
#71 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#72 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#73 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#74 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#75 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#76 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#77 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872ed8, argc=1) at src/be_vm.c:1286
#78 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#79 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#80 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#81 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#82 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#83 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#84 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872e98, argc=1) at src/be_vm.c:1286
#85 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#86 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#87 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#88 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#89 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#90 0x0019e9de in do_closure (vm=0x38716b8, pos=1, argc=1) at src/be_vm.c:1238
#91 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872e58, argc=1) at src/be_vm.c:1286
#92 0x00195b09 in ins2str (vm=0x38716b8, idx=0) at src/be_strlib.c:126
#93 0x00195be9 in be_val2str (vm=0x38716b8, index=1) at src/be_strlib.c:145
#94 0x00171ddd in be_tostring (vm=0x38716b8, index=1) at src/be_api.c:275
#95 0x00173f88 in l_print (vm=0x38716b8) at src/be_baselib.c:40
#96 0x0019e5bc in vm_exec (vm=0x38716b8) at src/be_vm.c:1142
#97 0x0019e9de in do_closure (vm=0x38716b8, pos=0, argc=0) at src/be_vm.c:1238
#98 0x0019ed20 in be_dofunc (vm=0x38716b8, v=0x3872e08, argc=0) at src/be_vm.c:1286
#99 0x0017f6e3 in m_pcall (vm=0x38716b8, data=0x11ffc54) at src/be_exec.c:303
#100 0x0017f0b4 in be_execprotected (vm=0x38716b8, f=0x17f6b9 <m_pcall>, data=0x11ffc54) at src/be_exec.c:118
#101 0x0017f724 in be_protectedcall (vm=0x38716b8, v=0x3872e08, argc=0) at src/be_exec.c:315
#102 0x00173950 in be_pcall (vm=0x38716b8, argc=0) at src/be_api.c:1013
#103 0x0019f4bf in doscript (vm=0x38716b8, name=0x3871688 "virtualsegfault.be", args=0) at default/berry.c:217
#104 0x0019f543 in load_script (vm=0x38716b8, argc=1, argv=0x38715d4, args=0) at default/berry.c:233
#105 0x0019fa58 in analysis_args (vm=0x38716b8, argc=1, argv=0x38715d4) at default/berry.c:396
#106 0x0019fa8e in main (argc=2, argv=0x38715d0) at default/berry.c:404

I can't reproduce the crash, even with make text which enables maximum pointer checks. I tested on MacOS and on Tasmota (ESP32).

Did you try on the latest version?

I can't reproduce the crash, even with make text which enables maximum pointer checks. I tested on MacOS and on Tasmota (ESP32).

Did you try on the latest version?

Yeah, I compiled the current master branch. I'm working from a Windows 10 PC so maybe it's OS specific.
GCC's sanitizers don't seem to have windows versions so I'll look into compiling with Clang...

Just an update. Tried compiling with Clang but it didn't like some of the pointer casting in be_exec.c
be_exec.c appears in the backtrace but unsure if related.
Clang's output is as follows:

[Compile] src/be_exec.c
src/be_exec.c:84:9: error: incompatible pointer types passing 'bjmpbuf' (aka 'int[16]') to parameter of type 'void **'
      [-Werror,-Wincompatible-pointer-types]
        exec_throw(vm->errjmp);
        ^~~~~~~~~~~~~~~~~~~~~~
src/be_exec.c:45:40: note: expanded from macro 'exec_throw'
#define exec_throw(j)       be_longjmp((j)->b, 1)
                            ~~~~~~~~~~~^~~~~~~~~~
src/be_exec.h:30:51: note: expanded from macro 'be_longjmp'
  #define be_longjmp(env, v)    __builtin_longjmp(env, v)
                                                  ^~~
src/be_exec.c:117:5: error: incompatible pointer types passing 'bjmpbuf' (aka 'int[16]') to parameter of type 'void **'
      [-Werror,-Wincompatible-pointer-types]
    exec_try(vm->errjmp) {
    ^~~~~~~~~~~~~~~~~~~~
src/be_exec.c:44:40: note: expanded from macro 'exec_try'
#define exec_try(j) if      (be_setjmp((j)->b) == 0)
                             ~~~~~~~~~~^~~~~~~
src/be_exec.h:29:50: note: expanded from macro 'be_setjmp'
  #define be_setjmp(env)        __builtin_setjmp(env)
                                                 ^~~
2 errors generated.

After recompiling on a different computer, I realize that my test case was incorrect. This has now been fixed.
While trying to hone in on the minimum case that caused an error I switched builds to the current master however I was still editing a script file located in the previous build's folder. This meant that I wasn't testing the version of the script that I was actually editing and thus getting outdated errors.

Can we close the issue?

Can you try testing the new code on your build to see if you can replicate it?

Sure. Where is the new test code?

I edited my first post to add it, but just to clarify, here's the specific code that I'm getting an error on:

def testfunc(arg)
	
	print(arg)
	
end

class TestClass
	
	
	def member(name)
		
		return testfunc
		
	end
	
	def setmember(name, value)
		
		
		
	end
	
	
end


var hey = TestClass("test")

hey.test()

Ha yes, I'm getting a crash as well. Now I can investigate.

It appears that there are two problems:

  1. When calling hey.test() Berry thinks that it is calling a method of this class and it passes an implicit self as the first argument, and this means that the instance of TestClass ends up in arg, and then is printed.
  2. When Berry wants to print the instance of TestClass it tries to lookup tostring to stringify the class, which causes it to call testfunc again, which causes it to print self again. This recursive cycle continues until the stack is exhausted.

So the fix for this problem is to somehow differentiate between calling a method of a class and a function returned by the member special function.

  1. When calling hey.test() Berry thinks that it is calling a method of this class and it passes an implicit self as the first argument, and this means that the instance of TestClass ends up in arg, and then is printed.

This is normal behavior. Since member() is returning a function and there is basically no way to know if it's a method or a function, we assume by convention that it is a method. Hence self is passed as implicit first argument. Not doing this would severely limit the possibilities of having dynamic methods.

tostring() is always delicate to handle. For example exception in tostring() can easily yield to a call to abort() which also causes a crash. Let's think of how to improve this.