From: Janne H. <ja...@hy...> - 2004-12-27 22:42:21
|
The following test case in the testing suite causes a segfault in garbage collection: --cut-- (* NOTE this is a copy of Skaller's trivial DynArray test. Apparently he hit the nail on the head with the first try, since test_triv causes a segfault if you run Gc.major () after test_triv. If you change the initial size of the DynArray to one or bigger, the crash does not occur. *) open DynArray let test_triv () = let a = make 0 in (* ZERO here causes a segfault later in GC??? *) let b = copy a in assert (length a == 0); assert (length b == 0); Gc.major () --cut-- It is enabled in the testing suite. If you want to run only this case, build mktest and run: $ ./mktest --author=jh --module=DynArray $ ./extlib_test Even though I didn't try, the crash should be reproducible outside of the testing suite. Tested with the latest CVS version of extlib. ciao, janne |
From: skaller <sk...@us...> - 2004-12-27 23:19:34
|
On Tue, 2004-12-28 at 09:42, Janne Hellsten wrote: > The following test case in the testing suite causes a segfault in > garbage collection: > > --cut-- > (* NOTE this is a copy of Skaller's trivial DynArray test. Apparently > he hit the nail on the head with the first try, Nope -- someone else reported this problem, I just coded it into a test. >open DynArray > > let test_triv () = > let a = make 0 in (* ZERO here causes a segfault later in GC??? *) > let b = copy a in > assert (length a == 0); > assert (length b == 0); > Gc.major () > --cut-- Hmm. If we had unit test level control in the test harness, we could add/not add Gc.major() after every test. > $ ./mktest --author=jh --module=DynArray > $ ./extlib_test Ah, you added --module :) -- John Skaller, mailto:sk...@us... voice: 061-2-9660-0850, snail: PO BOX 401 Glebe NSW 2037 Australia Checkout the Felix programming language http://felix.sf.net |
From: Janne H. <ja...@hy...> - 2004-12-27 23:32:52
|
skaller wrote: > Nope -- someone else reported this problem, I just > coded it into a test. Oh OK. Well thanks to the original reporter then :) I was not aware of this, so I've been thinking it's a new bug. > Hmm. If we had unit test level control in the test > harness, we could add/not add Gc.major() after every > test. Yep that's right. We could have it as a flag for Util.run_test. However, this can also be done in a regression tester fashion by isolating the test into a new test module (new file) and then adding the Gc.major () call by hand. I think this approach is perfectly reasonable since GC bugs should not be that frequent. There is also a possibility (because we have your mktest) to compile a fresh new executable out of each test. This way each test would be run in a separate process and thus they would not interfere with each other. There are also other variations such as randomizing the order of test execution inside a single process etc etc. >>$ ./mktest --author=jh --module=DynArray >>$ ./extlib_test > > Ah, you added --module :) Yes! $ ./mktest --author=js --author=jh --module=DynArray --module=ExtString calculates an intersection of all modules DynArray and ExtString that have js or jh as the author. I did not add --test=foo, since there's not much need for it yet. ciao, janne |
From: skaller <sk...@us...> - 2004-12-28 00:59:28
|
On Tue, 2004-12-28 at 10:32, Janne Hellsten wrote: > skaller wrote: > > Nope -- someone else reported this problem, I just > > coded it into a test. > > Oh OK. Well thanks to the original reporter then :) > > I was not aware of this, so I've been thinking it's a new bug. I'm too lazy to search the archives, but segfaults were reported with Dynarray some time ago. Much more recently someone actually found a problem with 0 length Dynarrays. Dynarray has been around for a while, so it is possible this caused the original problem. This kind of bug is very hard to find, which is a good reason to get rid of unnecessary unsafe accesses and add assertions. IMHO. This should not impact production code where assertions are turned off, and unsafe accesses replace all safe ones. > > Hmm. If we had unit test level control in the test > > harness, we could add/not add Gc.major() after every > > test. > > Yep that's right. We could have it as a flag for Util.run_test. > However, this can also be done in a regression tester fashion by > isolating the test into a new test module (new file) and then adding the > Gc.major () call by hand. That's true. > I think this approach is perfectly reasonable > since GC bugs should not be that frequent. Hmmm. What *other* kinds of bugs do you expect to find?? > There is also a possibility (because we have your mktest) to compile a > fresh new executable out of each test. This way each test would be run > in a separate process and thus they would not interfere with each other. Yes, but the harness generator doesn't handle units (unit tests) only groups of them. > There are also other variations such as randomizing the order of test > execution inside a single process etc etc. Yes, and calling the test multiple times too... > > Ah, you added --module :) > I did not add --test=foo, since there's not much need for it yet. It seems unlikely there will be any 'systematic' naming of tests, so probably the option should be --test=mmm_aa_ttt i.e. specify the whole filename. That test gets added into the test set, which is empty initially if the option is specified. Hmmm.. a more likely option is --exe=foobar which names the output executable, that way you have several testers at once. In particular a standard one, and one you're using to check the tests you're working on. -- John Skaller, mailto:sk...@us... voice: 061-2-9660-0850, snail: PO BOX 401 Glebe NSW 2037 Australia Checkout the Felix programming language http://felix.sf.net |
From: Janne H. <ja...@hy...> - 2004-12-28 10:02:59
|
skaller wrote: > I'm too lazy to search the archives, but segfaults were reported > with Dynarray some time ago. Much more recently someone actually > found a problem with 0 length Dynarrays. Even DynArray.create makes 0 length DynArrays, so it would be quite surprising if this was the actual bug. On the other hand, DynArray.copy uses a magic "idup" which is used only in two places in the module. This might well be the culprit. >> I think this approach is perfectly reasonable >>since GC bugs should not be that frequent. > > > Hmmm. What *other* kinds of bugs do you expect to find?? So far I have found only one bug that manifests itself in the GC cycle. Here are the bugs I've found from BitSet: - BitSet.compare logic mistakes, didn't handle different bit vector lengths - BitSet.differentiate et al. read past the bit vector thus producing a random answer for some test cases. Since these were reads, they didn't cause problems in GC. These kinds of bugs may or may not segfault, usually they will not if they only read a few bytes off. - Inconsistent behaviour: BitSet.is_set should throw an exception for negative indices (since others do as well!) but it doesn't. In the future if a Date/Time modules get added into ExtLib and should there be any bugs, I doubt these will be related to GC. Same goes for path handling. > --test=mmm_aa_ttt > --exe=foobar Yep, been thinking about the same options. I'll probably add these during this week. ciao, janne |
From: Nicolas C. <war...@fr...> - 2004-12-29 13:40:20
|
> The following test case in the testing suite causes a segfault in > garbage collection: I reported it on Caml list. Looks like the following is making crash too : let test() = let a = Obj.new_block 0 0 in let b = Obj.dup a in Gc.major() ;; I'm not sure I understand why, since "a" is a valid block, it looks like a caml bug to me. I'll fix the idup method on CVS until I got more informations on this. Nicolas |
From: Janne H. <ja...@hy...> - 2004-12-29 21:22:41
|
Just for the record: I patched my O'Caml 3.08.2 byterun/obj.c by commenting out line 107 "if (sz == 0) return arg;" O'Caml bootstrap went without problems and the problem with non-tempfixed DynArray disappeared. ciao, janne Nicolas Cannasse wrote: >>The following test case in the testing suite causes a segfault in >>garbage collection: > > > I reported it on Caml list. > Looks like the following is making crash too : > > let test() = > let a = Obj.new_block 0 0 in > let b = Obj.dup a in > Gc.major() > ;; > > I'm not sure I understand why, since "a" is a valid block, it looks like a > caml bug to me. > I'll fix the idup method on CVS until I got more informations on this. |
From: skaller <sk...@us...> - 2004-12-30 00:17:26
|
On Thu, 2004-12-30 at 08:22, Janne Hellsten wrote: > Just for the record: > > I patched my O'Caml 3.08.2 byterun/obj.c by commenting out line 107 > > "if (sz == 0) return arg;" > > O'Caml bootstrap went without problems and the problem with > non-tempfixed DynArray disappeared. In native code too? :-> -- John Skaller, mailto:sk...@us... voice: 061-2-9660-0850, snail: PO BOX 401 Glebe NSW 2037 Australia Checkout the Felix programming language http://felix.sf.net |
From: Janne H. <ja...@hy...> - 2004-12-30 00:20:36
|
Umm yes, I forgot to add, that strangely it seems that ocamlopt is using byterun/obj.c even if it looks like it's only meant for bytecode. Already deleted the patched compiler though, so I can't go and re-check. ciao, janne skaller wrote: > On Thu, 2004-12-30 at 08:22, Janne Hellsten wrote: > >>Just for the record: >> >>I patched my O'Caml 3.08.2 byterun/obj.c by commenting out line 107 >> >> "if (sz == 0) return arg;" >> >>O'Caml bootstrap went without problems and the problem with >>non-tempfixed DynArray disappeared. > > > In native code too? :-> > > |