Fix test/owstats for 32bit architecture (i386)

[lpenz]

owamp/config.h does a #define _FILE_OFFSET_BITS 64, which changes the size of off_t defined in fcntl.h from 4 to 8 in i386 architectures. That, in turn, changes the size of the structures in owamp/owamp.h that have off_t members.

test/owstats was including fcntl.h before including owamp/config.h, which made it create shorter structures in the stack. When the test called api.c functions that had the structure with the correct size, the stack was overrun, generating a segmentation fault.

This commit adds a #include <owamp/config.h> at the start of test/owstats.c to fix this specific issue.

[lpenz]

I've seen the problem and tested the solution in an i386 Debian jessie container.

This is a fix for a very specific issue; it doesn't solve the underlying problem, which is the fact that owamp/config.h is not being included before system includes, even though it has configurations that affect them. Ideally, owamp/config.h should be included as the first header in every .c file, and not indirectly from owamp/owamp.h - that would provide a clear policy and avoid this kind of issue in the future, and get rid of other more subtle, undiscovered issues that owamp might have right now because of this. This opinion is based on https://stackoverflow.com/questions/28637542/ensure-config-h-is-included-once and https://www.gnu.org/software/autoconf/manual/autoconf-2.66/html_node/Configuration-Headers.html

[lpenz]

I've updated the commit to include owamp/owamp.h at the beginning of every test.

The underlying problem, though, can still come through I2util, as the files there are compiled without _FILE_OFFSET_BITS

[vvidic]

Thanks for fixing the test includes, the change looks good to me.

Not sure about the problem with I2util, can you explain that a bit?

[lpenz]

I can try :)

The standard linux headers (fcnlt.h, for instance) have code equivalent to this:

#if defined _FILE_OFFSET_BITS && _FILE_OFFSET_BITS == 64
typedef __off_t off_t; // "native" version, 32 bits in i386, 64 bits in amd64
#else
typedef __off64_t off_t;
#endif

owamp/config.h makes a #define _FILE_OFFSET_BITS 64, so it's important to include it before the system headers to get consistent sizes for off_t (and struct stat, and others) in all object files. This is essentially what I did in the test files.

The problem with I2util is that it has its config.h doesn't have this definition. That means that I2util files are using the "native" version of off_t, while owamp files are always using the 64 bit version, and they are linked together. If we ever call an I2util function from owamp with a pointer to a struct that has off_t or another affected type, we can end up writing to wrong addresses, because the field positions will shift.

Using the issue in owstat as an example: OWPSessionHeaderRec has 2 off_t and one struct stat in the middle. The address of OWPSessionHeaderRec.test_spec is one in object files that were compiled with _FILE_OFFSET_BITS defined and another in files that were compiled without it. The contents of the structure will be messed up if we pass a pointer from a function in object file to a function in the other. That's literally what was causing the SIGSEGV in owstats.

I have inspected I2util's source, and I have not found any instances of that, but there's nothing to warn people if that changes. It can be a subtle bug, only on i386.

[vvidic]

Ok, merged the change into 4.2.0 branch and owstats test works now on i386.

Also updated configure.ac in I2util to use the same offset value as owamp:

owamp/config.h:#define _FILE_OFFSET_BITS 64
I2util/I2util/config.h:#define _FILE_OFFSET_BITS 64

[lpenz]

Awesome, thanks for doing this!