ThrowTheSwitch / Ceedling

Ruby-based unit testing and build system for C projects
http://throwtheswitch.org
Other
597 stars 246 forks source link

Segmentation fault after adding test #920

Closed jquside closed 2 months ago

jquside commented 2 months ago

Hello all,

I'm currently testing a function with several preparation steps. These steps are mostly function calls and check the result to continue or exit. This function calls are mocked and because each of them have different return error codes, I have a test for every possible error code.

At certain point there is a loop which exit with two values. These values are returned by a pointer. Adding the second value test raises a segmentation fault.

Source Code:

// Pull register until the FIFOs are ready to recover data
    uint32_t dataInfo = 0, registerValue = 0;

    do {
        err = getDataInfoEnvelope(dev, &registerValue);
        if (err) {
            return err;
        }

        dataInfo = registerValue & ENVELOPE_DONE_MASK;

        if (ENVELOPE_DONE_FAIL == dataInfo) {
            break;
        }

        // usleep_reentrant(10 * THOUSAND); // Sleep for 10 ms (don't overpull)
        usleep(10);
    } while (ENVELOPE_DONE_OK != dataInfo);

    // Stop Envelope
    err = setStopEnvelope(dev);
    if (err) {
        return err;
    }

    // Get the total of data
    uint32_t upDataCTR = (registerValue & ENVELOPE_DATA_COUNT_UP_MASK) >> 1;
    uint32_t lpDataCTR = (registerValue & ENVELOPE_DATA_COUNT_LP_MASK) >> 11;

    // Extract data from the FIFOs
    float newUpper, newLower;

    if (ENVELOPE_DONE_OK != dataInfo) {
        return E_GET_ENVELOPE;
    }

Working test code

void test_should_return_E_SET_CONTROL_ENVELOPE_when_setStopEnvelope_fails(void){
    int result = 0;
    uint32_t error_code = E_SET_CONTROL_ENVELOPE;

    // Bunch of ExpectAnyArgsAndReturn(SUCCESS) mock calls

    // getDataInfoEnvelope
    uint32_t registerValue = ENVELOPE_DONE_OK;
    getDataInfoEnvelope_ExpectAndReturn(dev, &registerValue, SUCCESS);
    getDataInfoEnvelope_IgnoreArg_info();
    getDataInfoEnvelope_IgnoreArg_dev();
    getDataInfoEnvelope_ReturnThruPtr_info(&registerValue);

    // setStopEnvelope
    setStopEnvelope_ExpectAnyArgsAndReturn(error_code);

    result = getEnvelope(dev, npk, delayStep, delayStart, delayEnd, dacStep, dacStart, dacEnd, totalLasers, relativeDelayLaser1, relativeDelayLaser2, delayLower, delayUpper, lowerBound, upperBound, envelope_size);
    TEST_ASSERT_EQUAL_INT32(error_code, result);

    free(dev);
}

Segmentation fault code After adding this test if fails.

void test_should_return_E_GET_ENVELOPE(void){
    int result = 0;
    uint32_t error_code = E_GET_ENVELOPE;

    // Bunch of ExpectAnyArgsAndReturn(SUCCESS) mock calls

    // getDataInfoEnvelope
    uint32_t registerValue =  ENVELOPE_DONE_FAIL;
    getDataInfoEnvelope_ExpectAndReturn(dev, &registerValue, SUCCESS);
    getDataInfoEnvelope_IgnoreArg_info();
    getDataInfoEnvelope_IgnoreArg_dev();
    getDataInfoEnvelope_ReturnThruPtr_info(&registerValue);

    // setStopEnvelope, SUCCESS to enter the if condition
    setStopEnvelope_ExpectAnyArgsAndReturn(SUCCESS);

    result = getEnvelope(dev, npk, delayStep, delayStart, delayEnd, dacStep, dacStart, dacEnd, totalLasers, relativeDelayLaser1, relativeDelayLaser2, delayLower, delayUpper, lowerBound, upperBound, envelope_size);
    TEST_ASSERT_EQUAL_INT32(error_code, result);

    free(dev);
}

CMOCKMEM defines looks like are not having major impact. I was using default options and I've been changing values to see the result changes. It doesn't affect at all.

I'm under an ubuntu docker with: Ceedling:: 0.31.1 Unity:: 2.5.4 CMock:: 2.5.4 CException:: 1.3.3

Thank you all :)

mvandervoord commented 2 months ago

It's a bit hard to trace, since this isn't all the code in the function under test. For example, you appear to be freeing dev at the bottom of the functions, but this doesn't appear to be dynamically allocated anywhere. If you ARE allocating it somewhere, perhaps it's being realloc'd and therefore has moved, etc? If not, this function call is likely the problem... freeing something that doesn't need to be freed can cause random behavior that changes from test to test.

Unrelated: I believe you second test wants to have the assertion updated to compare to SUCCESS, not err_code. :)

jquside commented 2 months ago

It's a bit hard to trace, since this isn't all the code in the function under test. For example, you appear to be freeing dev at the bottom of the functions, but this doesn't appear to be dynamically allocated anywhere. If you ARE allocating it somewhere, perhaps it's being realloc'd and therefore has moved, etc? If not, this function call is likely the problem... freeing something that doesn't need to be freed can cause random behavior that changes from test to test.

Unrelated: I believe you second test wants to have the assertion updated to compare to SUCCESS, not err_code. :)

Hi,

you are right, I just remove the dev variable malloc line to avoid show "too much".

This is the last test version. Previous version I was using SetUp and TearDown to malloc and free a global variable, it also failed.

I have 32 function tests in this file. I've comment all previous test and the last one make the tests fails.

The failing test is almost identical to the previous. It just change the return value to exit the loop and fail a few lines below.

Is more error safe using the error_code variable because not all the tests asserts with SUCCESS.

thank you very much :)

mvandervoord commented 2 months ago

I'm not seeing any problems in the portion of the code that you have posted. A segfault most often happens when dereferencing a pointer that hasn't been initialized properly. My best guess is that you have code in the function that you're testing, most likely below the error check, which uses a pointer. That pointer probably isn't pointing at something valid in the context of this test.

This is a fairly common problem in testing. Often our release code initializes variables in one function and then uses them in another... and the initialization can get missed when testing.

Depending on how strict your compiler is, you might attempt to push the definition of registerValue outside of the test to make it a module variable in the test. If this fixes things, you're running a compiler which doesn't like passing a pointer to a stack variable. If this doesn't fix things (and in most cases it shouldn't), you've at least ruled out one potential pointer issue.

jquside commented 2 months ago

Hello Mark,

thank you for the advices. After following your indications I finally fixed the code to pass the test.

I refucktored the while loop.

while(true) {
        err = getDataInfoEnvelope(dev, &registerValue);
        if (err) {
            return err;
        }

        if (    (ENVELOPE_DONE_FAIL == (registerValue & ENVELOPE_DONE_FAIL_MASK))
            ||  (ENVELOPE_DONE_OK   == (registerValue & ENVELOPE_DONE_OK_MASK))) {
            break;
        }

        // usleep_reentrant(10 * THOUSAND); // Sleep for 10 ms (don't overpull)
        usleep(10);
}

dataInfo = registerValue & ENVELOPE_DONE_OK_MASK;

the define values:

define ENVELOPE_DONE_OK (0x00000001)

define ENVELOPE_DONE_OK_MASK (0x00000001)

define ENVELOPE_DONE_FAIL (0x80000000)

define ENVELOPE_DONE_FAIL_MASK (0x80000000)

previously the if was doing a comparison with dataInfo variable. dataInfo was registerValue & ENVELOPE_DONE_OK_MASK.

I did the experiment of changing the SUCCESS test, which was using the return to pointer value equal to ENVELOPE_DONE_OK, to ENVELOPE_DONE_FAIL. Then it failed to the segfault situation. This draw my attention to the dataInfo variable value.

Thank you very much.