cmor.write segfault related to timesteps

[chloemackallah]

In recent testing of CMOR with the ACCESS model, we found an issue when attempting to use cmor.write, but with an incorrect value assigned to argument 'ntimes_passed'.
In this scenario, when the value of 'ntimes_passed' was inconsistent with the actual number of timesteps contained within the data that is passed to cmor.write, the result is a segmentation fault. This made it difficult to track the source of the error.
We would suggest an instructional error message be output if this occurs rather than a segmentation fault. Additionally, perhaps it would be possible to implement a check that the number of time values is consistent and initialise the dimension length accordingly.

[mauzey1]

@doutriaux1 I am currently trying to make a way to check if the number of times passed exceed the length of the time axis.

cmor/Src/cmor.c

Lines 4392 to 4412 in 7c7e48a

	/* -------------------------------------------------------------------- */
	/* Make sure that the number of times being passed to cmor_write */
	/* combined with the number of times written does not exceed the */
	/* length of the time axis. */
	/* -------------------------------------------------------------------- */
	if (ntimes_passed > 0) {
	for(i = 0; i < cmor_vars[var_id].ndims; ++i) {
	if(cmor_axes[cmor_vars[var_id].axes_ids[i]].axis == 'T'
	&& cmor_axes[cmor_vars[var_id].axes_ids[i]].length > 0
	&& (cmor_vars[var_id].ntimes_written + ntimes_passed
	> cmor_axes[cmor_vars[var_id].axes_ids[i]].length)) {
	cmor_handle_error("The number of times passed combined "
	"with the number of times written must "
	"not exceed the length of the time axis",
	CMOR_CRITICAL);
	cmor_pop_traceback();
	return (-1);
	}
	}
	}

I am not sure how well it handles cases where it is writing data to a zfactor. I tried putting this check below the call to cmor_set_refvar since it sets the ntimes_written attribute of the zfactor with ntimes_written of the associated variable minus ntimes_passed.

cmor/Src/cmor.c

Lines 2441 to 2442 in 7c7e48a

	cmor_vars[var_id].ntimes_written =
	cmor_vars[nRefVarID].ntimes_written - ntimes_passed;

It currently passes all of the current tests in the test suite. I also made a test to see if it gets the error for passing more times to cmor_write than the length of the time axis.
https://github.com/PCMDI/cmor/blob/440_cmor_write_segfault_related_to_timesteps/Test/test_python_CMIP6_CV_times_written_exceeded.py

Are there any other tests I could throw at it?

[mauzey1]

@chloemackallah I would like to revisit this issue by figuring out how to replicate the error you were experiencing. Could you submit some sample code that causes this error?

In this scenario, when the value of 'ntimes_passed' was inconsistent with the actual number of timesteps contained within the data that is passed to cmor.write, the result is a segmentation fault.

I wonder if this was already solved in #485, at least for Python code.

[chloemackallah]

@mauzey1 It does look like this was solved in #485 and #500, as that catches the seg fault.

[mauzey1]

@chloemackallah Do you think this issue can be closed now?

[mauzey1]

I will close this for now.