Trailing comma in expect() causes Segmentation fault 11

[dmeehan1968]

expect(
  stub_func,
  will_return(1),
);

I'm finding myself writing expectations formatted like this because in my tests i can have multiple when() conditions and the line break makes it easier to read.

However, I've noticed that the trailing comma (in this case after will_return() but it doesn't matter what the expectation is, e.g. when(), times() etc) causes the test to fail with a seg fault 11. I suspect that the variadic nature of expect() is picking up some garbage value from the empty argument and that's where the fault is generated. Simply removing the comma solves the problem.

It's a very easy oversight and I've wasted a bunch of time trying to figure out if there was some memory reference issue in my expectations, for it to finally dawn on me what I was doing wrong. In another instance, I could change my SUT code subtly and it would not fault, probably because the garbage value in the macro then references something benign.

I'm using a dynamic library and cgreen-runner in this scenario, not tested non-runner builds to see if that manifests in a different way.

Building using Xcode on macOS, clang set to C99.

It would be great if this could be caught somehow and either get a build time or runtime warning rather than the unhelpful seg fault.

[thoni56]

Thanks for your report!

I tried this:

#include <cgreen/cgreen.h>
#include <cgreen/mocks.h>

Describe(Issue201);
BeforeEach(Issue201) {}
AfterEach(Issue201) {}

static int stub_function() {
    return (int)mock();
}

Ensure(Issue201, extra_comma_does_not_crash) {
    expect(
       stub_func,
       will_return(1),
       );
}

This did not crash using

thomasmacbook:#201 thomas$ gcc -v
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/c++/4.2.1
Apple LLVM version 10.0.1 (clang-1001.0.46.4)
Target: x86_64-apple-darwin18.7.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Instead it said:

gcc    -c -o 201.o 201.c
gcc -shared -o 201.so 201.o -lcgreen
cgreen-runner 201.so
Running "201" (1 test)...
201.c:16: Failure: Issue201 -> extra_comma_does_not_crash 
    Expected call was not made to mocked function [stub_func]
    "Issue201": 1 failure in 1ms.
Completed "201": 1 failure in 1ms.

I'm using latest master.

[dmeehan1968]

Thanks for the feedback, I also saw instances where the tests would succeed, the problem seems to be that the last (empty) macro argument causes cgreen to reference a random memory location, so the arbitrary content of the location may have some bearing on whether it crashes.

For example, when I had other when() etc arguments to the expect(), I could invoke the crash or avoid it by commenting some out or changing the order.

You may find that if you have a suite with more than one test you'll create sufficient complexity to cause it to reference some invalid memory location that will result in the seg fault.

Note that I am compiling using Xcode, but my gcc command shows:

Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/c++/4.2.1
Apple LLVM version 10.0.1 (clang-1001.0.46.4)
Target: x86_64-apple-darwin18.7.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

[thoni56]

Of course. Sorry for not catching that. valgrinding it seems to give some fishy messages. WIll try to investigate more. Could probably need some help with the macro-magic here...

[dmeehan1968]

No worries. I suspect its an inherent flaw in C, relying on passed pointers being valid, and that the preprocessor macros allow empty arguments where normal function calls do not. I imagine that expect() uses macro tricks to determine argument counts, and then interprets the empty last argument as having a valid memory reference, which points into some random memory offset that sometimes leads to a seg fault, sometimes not.

I know one trick is for pointers to reference memory that contains a signature. The signature can then be used to ensure that the reference location is valid before relying on any further values. This typically helps with structs that contain other references.

struct A {
  uint32_t signature;
  void * ptr_to_something;
  // other stuff...
};

void my_func(struct A *a) {
  if(a->signature == 0xF00BA8) {
    // a->ptr_to_something etc. should be safe
  }
}

//...
struct A a = { 0xF00BA8, &something };
my_func(&a);

I've not dug into Cgreen to see if such tricks are used, I was actually quite surprised at the simplicity and elegance of the mock facility, and I've really been enjoying using it, bar this rather easy to overlook gotcha!

The method above might allow Cgreen to validate the expect() arguments and complain if the signatures are invalid, rather than executing code that leads to a seg fault.