Linking issue on MinGW64 + Clang

[markand]

Hi,

Using msys2 with the clang64 toolchain I get a few linking issue given a basic template. I'm currently investigating the issue.

lld-link: error: undefined symbol: __start_rxcase
>>> referenced by objects.a(test-vfs-zip.c.obj):(.refptr.__start_rxcase)

lld-link: error: undefined symbol: __start_rxsuite
>>> referenced by objects.a(test-vfs-zip.c.obj):(.refptr.__start_rxsuite)

lld-link: error: undefined symbol: __stop_rxcase
>>> referenced by objects.a(test-vfs-zip.c.obj):(.refptr.__stop_rxcase)

lld-link: error: undefined symbol: __stop_rxsuite
>>> referenced by objects.a(test-vfs-zip.c.obj):(.refptr.__stop_rxsuite)
gcc: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [tests/CMakeFiles/test-vfs-zip.dir/build.make:111: tests/test-vfs-zip.exe] Error 1
make[1]: *** [CMakeFiles/Makefile2:921: tests/CMakeFiles/test-vfs-zip.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

[christophercrouzet]

Hi @markand,

Sorry to hear about this issue!

This code is related to the automatic registration of the tests, which is an implementation that is compiler-specific. See https://christophercrouzet.com/blog/dev/rexo-part-2#automatic-test-registration to learn more about how it works.

I'm not familiar whatsoever with MSYS2 whatsoever (I run on Linux) but, as you can read from the snippet on the page that I just linked, the __start_* symbols are used when the macro __unix__ is defined, so maybe it's not the right pick for MSYS2?

If you have a chance, could you please try to compile that snippet while forcing it to pick the _MSC_VER branches and see if it helps at all?

[markand]

I didn't know the framework used non portable compiler extensions for that purpose. I then definitely can't help much there 😜.

Let me check with your proposals.

Edit: setting _MSC_VER does not work with MinGW because it does not understand the compiler extensions, especially _Pragma.

[christophercrouzet]

I really tried hard to write this library in a fully portable way but it simply doesn't seem to be possible with only what the C standard offers us. And that's why most of the other unit test libraries don't offer automatic test registration, and why Criterion and NovaProva also both rely on some compiler-specific and/or debug format specific code.

On the bright side, what is required by Rexo to get the automatic registration to work is very minimal: it only needs to be able to store the tests in a custom data segment, which is something so important to have that I'm assuming every compiler should support it in a way or another.

So, in theory, it's only a matter of extending support for these compilers, which I'm happy doing as long as I have the right hardware/software :sweat_smile:

That being said, I'm not familiar with MSYS2 and MinGW, and saw that you also mentioned clang, so I'm a bit confused here. If Rexo were to support MinGW, would that address the error that you reported with MSYS2?

[markand]

Would it be possible to add a fallback using a static number of tests when the compiler specific support is not available? And possibly configurable through a compile time constant. That would be indeed more portable.

[christophercrouzet]

If my memory serves me right (and it often doesn't!), I believe having already investigated this option and deemed that it was not really possible due to limitations inherent to C (but not C++), hence why the current fallback being used is to manually register test :disappointed:

[markand]

Hmmm, I've tried disabling the automatic test and register all by hand but I still get an error:

ld.lld: error: undefined symbol: __start_rxcase
>>> referenced by objects.a(test-action.c.obj):(.refptr.__start_rxcase)

ld.lld: error: undefined symbol: __start_rxsuite
>>> referenced by objects.a(test-action.c.obj):(.refptr.__start_rxsuite)

ld.lld: error: undefined symbol: __stop_rxcase
>>> referenced by objects.a(test-action.c.obj):(.refptr.__stop_rxcase)

ld.lld: error: undefined symbol: __stop_rxsuite
>>> referenced by objects.a(test-action.c.obj):(.refptr.__stop_rxsuite)
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [tests/CMakeFiles/test-action.dir/build.make:111: tests/test-action.exe] Error 1
make[1]: *** [CMakeFiles/Makefile2:822: tests/CMakeFiles/test-action.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

[christophercrouzet]

Registering the tests manually does not disable the automatic discovery of tests. The code just looks at the current compiler and picks one of the 2 approaches available: https://github.com/christophercrouzet/rexo/blob/d2e5cee1730d67859d8a4aea7004990a4925e9e0/include/rexo.h#L4701-L4750

But the current logic always fall back to compiling GCC-compatible code even though the compiler might not fall into this category, as it might be what's happening in your case.

So I just made a change along these lines:

#if defined(_MSC_VER) || defined(__GNUC__)
    #define RX__TEST_AUTOMATIC_DISCOVERY 1
#else
    #define RX__TEST_AUTOMATIC_DISCOVERY 0
#endif

If neither of these symbols is set, then the code related to the automatic test discovery is disabled.

Note that it's difficult to fully test this change on my end so maybe it needs some more tweaks.

I made a new branch here with these changes: https://github.com/christophercrouzet/rexo/tree/disable-test-discovery

If this still doesn't work out, please double-check the values of _MSC_VER, __GNUC__, and RX__TEST_AUTOMATIC_DISCOVERY.

Ideally, it would be best to figure out what needs to be done to make automatic test discovery work with MinGW. It looks like it's available from Linux so maybe I'll give it a go over the week-end!

[markand]

I admit that it's handy to have auto generated tests but isn't that overkill regarding the portability issue? I mean, I've been using greatest prior to rexo and dislike its output (which led me to rexo instead) format but basically the process was:

GREATEST_TEST
some_test(void)
{
}

GREATEST_SUITE(oh_my_suite)
{
    GREATEST_RUN_TEST(some_test);
}

So yes, you have to register yourself that line GREATEST_RUN_TEST each time you write one but thanks to the compiler you get a warning saying that a function some_test is defined but never used (in case you forgot to call GREATEST_RUN_TEST) so you can't really forget to run some tests unless you ignore all compiler warnings.

Honestly but that's obviously my humble opinion, I prefer having tests ran in the order I've defined rather than using those compiler intrisics magic that as you can see does not work very well on non-popular compilers.

In the meantime I'll try your branch but I think a user definable macro to disable entirely the automatic logic could be handy.

[christophercrouzet]

I admit that it's handy to have auto generated tests but isn't that overkill regarding the portability issue?

I don't think it's overkill. Manually registering tests can be a lot of boilerplate, which is fine as long as it adds some value. For example, the boilerplate required to manually manage memory (e.g.: malloc) versus gargabe collectors can be worth it since it gives a more granular control over memory resources. But in the case of manual test registration, this doesn't add any value at all over automatic registration—in fact it gives an opportunity to introduce errors since it's easy to add new tests and forget to register them (the unused warning that you mentioned doesn't always show up depending on the compiler flags used), and it adds some burden when it comes to maintaining these tests. It unnecessary boilerplate can be avoided, then why not?

Honestly but that's obviously my humble opinion, I prefer having tests ran in the order I've defined rather than using those compiler intrisics magic that as you can see does not work very well on non-popular compilers.

This should be possible. Right now it's sorting the tests by name:

https://github.com/christophercrouzet/rexo/blob/d2e5cee1730d67859d8a4aea7004990a4925e9e0/include/rexo.h#L6664-L6671

But it should be possible to sort them according to the order they're defined in, using __LINE__ or maybe __COUNTER__.

Note that the compiler-specific code used is not magical whatsoever. It's something that compilers must implement in order for programs to write data into custom memory sections, which is a feature absolutely necessary for system programming. So I'm expecting every single compiler to be able to do that somehow, it's just a matter of supporting them all as the needs come, and it shouldn't be too difficult as long as I can have access to these compilers.

In the meantime I'll try your branch but I think a user definable macro to disable entirely the automatic logic could be handy.

That's something that could definitely be done as well! I'll look into it over the week-end if I find some time.

[christophercrouzet]

FYI, I don't think that my previous changes were working so I made new changes to the same branch https://github.com/christophercrouzet/rexo/tree/disable-test-discovery.

It also includes a new macro RX_DISABLE_TEST_DISCOVERY and an example on how to use it: https://github.com/christophercrouzet/rexo/blob/336c0986fd83a45e11bf49464027e7671a5cf41c/tests/no-discovery.c.

[markand]

I confirm that RX_DISABLE_TEST_DISCOVERY finally fixes the MSYS2+clang issue! thanks for that quick change.

[christophercrouzet]

Awesome, thanks for testing and letting me know @markand, I'm glad it helps!

I just installed MSYS2 with Clang on a Windows 10 machine to test the compiler-specific code and it looks like the current implementation works just fine for me?

I'm getting a few warnings related to long long not being C89-compliant but other than that I don't get any “undefined symbol” compiler error—I managed to compile and run the tests just fine by simply opening the MSYS2 MinGW x64 and running clang -I ./include -o ./fixture.exe ./tests/fixture.c && ./fixture.exe from Rexo's root directory.

Which, I guess is to be expected since it's leveraging Clang under the hood.

I added a new commit to the same branch to make sure that MinGW's environment pick up the right code path, but it will basically run the same code than what is currently in the main branch, so this might still error for you.

Could it be that your set-up of MSYS2 is somehow incomplete? For my tests, I followed the installation steps described on their home page https://www.msys2.org and only added the clang package using pacman -S mingw-w64-x86_64-clang.

[markand]

I have only tested the automatic de-registration macro, not the fix on the original features but will do as well.

[christophercrouzet]

Closing as it has been merged!