Solaris SEGV in test cleanup

[John Bito]

Hi,

I'm just starting to look into CGreen and I realize it may have problems on Solaris 10 that I need to address.  I hope you can give me some guidance as I debug this problem.  The sample strlen test from the tutorial ends with a SEGV:

Running "our_tests"...
Completed "our_tests": 1 pass, 0 failures, 0 exceptions.
Segmentation Fault (core dumped)

The stack shows it's in the cleanup:
#0  0xff15725c in _free_unlocked () from /usr/lib/libc.so.1
#1  0xff157204 in free () from /usr/lib/libc.so.1
#2  0x00011794 in destroy_test_suite (suiteToDestroy=0x259e0) at src/unit.c:68
#3  0x00011ca8 in clean_up_test_run (suite=0x259e0, reporter=0x267e8)
    at src/unit.c:148
#4  0x00011bec in run_test_suite (suite=0x259e0, reporter=0x267e8)
    at src/unit.c:134
#5  0x00011674 in main ()

Here's the test source:
#include "cgreen/cgreen.h"
#include <string.h>

static void strlen_of_hello_should_be_five() {
    assert_equal(strlen("Hello"), 5);
}

TestSuite *our_tests() {
    TestSuite *suite = create_test_suite();
    add_test(suite, strlen_of_hello_should_be_five);
    return suite;
}

int main(int argc, char **argv) {
    return run_test_suite(our_tests(), create_text_reporter());
}

[João Henrique F. de Freitas]

Hello,

In my environment I can't reproduce. But I have one suspect in unit.c:68, this frees something wrong.

Currently I don't have a Solaris 10 box. I will investigate it.

Thanks

[John Bito]

With this patch, the trivial test no longer has SEGV.  I think a complete fix probably needs to be recursive, since a suite may contain another suite.  I haven't become familiar with the flow of control, so that may already be handled.

Index: src/unit.c

--- src/unit.c  (revision 300)
+++ src/unit.c  (working copy)
@@ -63,7 +63,7 @@
        for (i = 0; i < suiteToDestroy->size; i++) {
                UnitTest test = suiteToDestroy->tests[i];
                TestSuite* suite = test.sPtr.suite;
-               if (suite != NULL) {
+               if (test_suite == test.type) {
                        suiteToDestroy->tests[i].sPtr.suite = NULL;
                        free(suite->tests);
                        free(suite);

[John Bito]

I'm still getting SEGV at the same line when the test suite is larger.  I'll have to work on it more a bit later on.

[John Bito]

I think I undertand a bit of the flow, so I believe the destroy function should free test suites recursively.  This patch works for me:

Index: src/unit.c

--- src/unit.c  (revision 300)
+++ src/unit.c  (working copy)
@@ -63,10 +63,9 @@
        for (i = 0; i < suiteToDestroy->size; i++) {
                UnitTest test = suiteToDestroy->tests[i];
                TestSuite* suite = test.sPtr.suite;
-               if (suite != NULL) {
+               if (test_suite == test.type && suite != NULL) {
                        suiteToDestroy->tests[i].sPtr.suite = NULL;
-                       free(suite->tests);
-                       free(suite);
+                       destroy_test_suite(suite);
                }
        }
     free(suiteToDestroy->tests);

[João Henrique F. de Freitas]

Hello,

I will test your patch.

Did you have some specific test to me reproduce this segv?

Thanks

[John Bito]

I am pretty sure that the SEGV is specific to Solaris (maybe the sparc processor).  If you have something like watchmalloc.so that adds heap checks, it complains at the same point that I get SEGV on Solaris.

I don't have another unix system to try at the moment.  I may try compiling on Windows, but only if I get my deliverables finished :-o

Thanks!
John

[John Bito]

Oh yeah - the SEGV occurs (on Solaris) running the sample test that's in my first post.  Here it is, again:

#include "cgreen/cgreen.h"
#include <string.h>

static void strlen_of_hello_should_be_five() {
assert_equal(strlen("Hello"), 5);
}

TestSuite *our_tests() {
TestSuite *suite = create_test_suite();
add_test(suite, strlen_of_hello_should_be_five);
return suite;
}

int main(int argc, char **argv) {
return run_test_suite(our_tests(), create_text_reporter());
}

[João Henrique F. de Freitas]

Hi,

Your patch solves many memory leaks. I am testing and will commit soon.

Thanks.

[João Henrique F. de Freitas]

Hi, see commit 304.

Thanks.

[John Bito]

That's a good patch.  I didn't recognize that a suite initially has no tests array allocated.

It's good to have a nice package and people who look after its quality.

Nice work!