0004310: add unit test for generating stacktraces

View Issue Details [ Jump to Notes ]

Project

Tags

No tags attached.

Checked infolog.txt for Errors

Attached Files

Relationships

Relationships

Notes
~0012896 MajorBoredom (reporter) 2014-03-06 07:01	I'd like to understand this code and implement the test if someone can give me a good starting point.

~0012897 abma (administrator) 2014-03-06 12:47 Last edited: 2014-03-06 12:47 View 2 revisions	https://github.com/spring/spring/blob/develop/test/engine/System/testThreadPool.cpp https://github.com/spring/spring/blob/develop/test/CMakeLists.txt#L296 https://github.com/spring/spring/blob/develop/rts/System/Platform/CrashHandler.h to make a test you either need to modifiy the CrashHandler functions so it returns the stacktrace which can be "tested" for beeing valid or the output has to be captured somehow.

~0012899 jK (developer) 2014-03-08 13:38	log unit test reads back all LOG() calls, duno how but maybe it is usable for external (=other cpp) code, too.

~0012913 MajorBoredom (reporter) 2014-03-12 06:34	Ok, I have looked into this. Two points: 1) We need a UNIT_TEST macro which works somewhat like the HEADLESS macro. It would allow certain code to be excluded from compilation. This is dangerous for a unit test, but it is somewhat necessary to break the module dependency chain. For example, CrashHandler.cpp would ultimately need SpringTime.cpp, SpringApp.cpp, and quite a bit of their initialization code. For a unit test you would like to exclude certain parts (clearing the watchdog timer for example). Otherwise, the test is more like an integration or regression test and less like a unit test. 2) I do not understand the rationale behind passing hThread to Stacktrace() specifying the stack to be "traced". This seems inherently dangerous. The Linux platform code implements a "internal_pthread_backtrace" which allows a foreign running thread to be traced. This essentially makes a memcpy of the entire pthread's stack and uses the top 20% (bottom 20% of the stack since it is allocated backward) of this for the trace. The implementation is also somewhat error-prone since it treats any uint value that is within the range of the stack segment as if it were a pointer. Given that linux stacks tend to be in the upper portion of the program address space, it would not be too difficult to interpret certain large signed negative values as stack pointers. Would it be unreasonable to limit Stacktrace() to reporting about the current running thread? Otherwise you cannot be sure about the top of the stack, and that is usually where the programmer is looking for the bug/error/problem. However I am new here and do not make such big decisions.

~0012916 jK (developer) 2014-03-12 11:46 Last edited: 2014-03-12 11:47 View 2 revisions	> We need a UNIT_TEST macro which works somewhat like the HEADLESS macro. https://github.com/spring/spring/blob/develop/test/CMakeLists.txt#L22 like this? > I do not understand the rationale behind passing hThread to Stacktrace() specifying the stack to be "traced". We have a watchdog, a thread that checks from time to time if the other threads hang in a deadloop and if they do it prints the stacktrace for those, so you know where it hangs. Also I know "internal_pthread_backtrace" isn't nice, but the documentation of gdb's libunwind are very hidden on the internet and to that time I didn't found any information how to use it (these days I collected some sources, so I would use libunwind etc. when I would have to recode it).

~0012922 MajorBoredom (reporter) 2014-03-12 21:39 Last edited: 2014-03-12 23:25 View 2 revisions	I could do this, but before I start: jK are you planning to implement libunwind? This way I do not write code that you are already planning for. I think we should hold off on the unit test for stack traces until libunwind. Otherwise we are writing a unit test for very non-deterministic code. It is difficult to write a meaningful test that produces the same behaviour at test time as it does at runtime.

~0012924 abma (administrator) 2014-03-12 23:56 Last edited: 2014-03-12 23:57 View 3 revisions	atm this feature request is only about adding a unit-test. its not clear what is broken or if its broken at all. atm it looks like: - several linux distributions show invalid stacktraces - windows stacktraces are partly invalid, sth. like every second address is wrong this is why this test should be added, to see/check if it works in general, and then, if it fails fix it. idk whats wrong atm, it only seems like the printed stacktraces are invalid in several cases.

~0012925 MajorBoredom (reporter) 2014-03-13 03:44	The Linux version's algorithm is designed to generate the trace by treating any pointer-sized values in the stack segment as if they were pointers within that segment. So any other values (generated inside Lua, etc) that happen to coincide with this range of values will produce an erroneous trace. No need for a unit test to see this, it is clearly designed this way. Just read rts/System/Platform/Linux/thread_backtrace.cpp:60-87 in develop. Yes I agree that adding libunwind would be a separate feature and not part of this request. But it is pointless to write a unit test for the current algorithm if it deliberately produces a random guess! It could easily pass at test time and fail at runtime, or vice versa.

~0012927 abma (administrator) 2014-03-13 05:26	okok, don't missunderstand me: imo when a test exists, its much easier to fix / improve the existing code. when you fix the existing code then you always need to recompile spring which takes a lot more time than compiling/linking the test. there is nothing against fixing/improving the existing code. +1 for every improvement there as current stacktraces are very inaccurate it seems.

~0012932 jK (developer) 2014-03-13 21:55 Last edited: 2014-03-13 21:55 View 2 revisions	> jK are you planning to implement libunwind? no > But it is pointless to write a unit test for the current algorithm if it deliberately produces a random guess! It's a bit more than a guess. It searches for the longest `valid` looking stacktrace and takes it. Also it is only for watchdog stacktrace, crash ones use the standard backtrace(). > I could do this, but before I start: If you do prepare for much undocumented stuff ;) It will everything other than easy cause the interface is not obvious Here an entry point of how libunwind looks like: http://www.nongnu.org/libunwind/man/libunwind-ptrace(3).html So imo it's not worth to implement, the gain is limited. only this might be a reason: > +1 for every improvement there as current stacktraces are very inaccurate it seems. but afaik you can't use libunwind under windows so it won't help much (under linux you can always use gdb).

~0012934 silentwings (reporter) 2014-03-13 23:58	Under windows too, I've used gdb a few times on spring and it's never failed me.

Notes

Date Modified	Username	Field	Change
Issue History
2014-02-17 09:50	abma	New Issue
2014-02-17 09:50	abma	Severity	minor => feature
2014-02-17 09:50	abma	Product Version	97.0 =>
2014-02-17 09:50	abma	Target Version	=> 97.0
2014-03-06 07:01	MajorBoredom	Note Added: 0012896
2014-03-06 12:47	abma	Note Added: 0012897
2014-03-06 12:47	abma	Note Edited: 0012897	View Revisions
2014-03-08 13:38	jK	Note Added: 0012899
2014-03-12 06:34	MajorBoredom	Note Added: 0012913
2014-03-12 11:46	jK	Note Added: 0012916
2014-03-12 11:47	jK	Note Edited: 0012916	View Revisions
2014-03-12 21:39	MajorBoredom	Note Added: 0012922
2014-03-12 23:25	MajorBoredom	Note Edited: 0012922	View Revisions
2014-03-12 23:56	abma	Note Added: 0012924
2014-03-12 23:56	abma	Note Edited: 0012924	View Revisions
2014-03-12 23:57	abma	Note Edited: 0012924	View Revisions
2014-03-13 03:44	MajorBoredom	Note Added: 0012925
2014-03-13 05:26	abma	Note Added: 0012927
2014-03-13 21:55	jK	Note Added: 0012932
2014-03-13 21:55	jK	Note Edited: 0012932	View Revisions
2014-03-13 23:58	silentwings	Note Added: 0012934
2014-03-23 18:37	abma	Target Version	97.0 =>
2014-06-01 01:10	abma	Target Version	=> 98.0
2014-06-04 16:42	abma	Target Version	98.0 =>

Issue History