Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests fail to compile with pre-release glibc and leatherman-vendored Catch #132

Open
musicinmybrain opened this issue Mar 9, 2021 · 3 comments
Assignees
Labels

Comments

@musicinmybrain
Copy link

Describe the Bug

In the next version of glibc, 2.34, SIGSTKSZ will be changed from a preprocessor constant to an expression involving a runtime function call. This has broken a lot of software that used it as a stack or static buffer size.

In leatherman, a copy of the Catch unit test framework (version 1.x rather than the current 2.x) is bundled in the “vendor” area. It is part of the public API, and cpp-hocon uses it for its unit tests. Since the SIGSTKSZ change, the cpp-hocon build fails with:

In file included from /usr/include/signal.h:315,
                 from /usr/include/leatherman/vendor/catch.hpp:6448,
                 from /builddir/build/BUILD/cpp-hocon-0.3.0/lib/tests/main.cc:2:
/usr/include/leatherman/vendor/catch.hpp:6471:33: error: size of array 'altStackMem' is not an integral constant-expression
 6471 |         static char altStackMem[SIGSTKSZ];
      |                                 ^~~~~~~~
/usr/include/leatherman/vendor/catch.hpp:6522:45: error: size of array 'altStackMem' is not an integral constant-expression
 6522 |     char FatalConditionHandler::altStackMem[SIGSTKSZ] = {};
      |                                             ^~~~~~~~

Expected Behavior

The unit tests build and work

Steps to Reproduce

Steps to reproduce the behavior:

  1. Find a platform with a pre-release version of glibc 2.34 or later, such as Fedora Rawhide
  2. Build cpp-hocon

Environment

  • Version 0.3.0
  • Platform Fedora Rawhide/35 and 34

Additional Context

From the glibc 2.34 release notes:

* Add _SC_MINSIGSTKSZ and _SC_SIGSTKSZ.  When _SC_SIGSTKSZ_SOURCE or
  _GNU_SOURCE are defined, MINSIGSTKSZ and SIGSTKSZ are no longer
  constant on Linux.  MINSIGSTKSZ is redefined to sysconf(_SC_MINSIGSTKSZ)
  and SIGSTKSZ is redefined to sysconf (_SC_SIGSTKSZ).

Here is the upstream Catch issue for both 1.x and 2.x: catchorg/Catch2#2178

I am the maintainer of the cpp-hocon package in Fedora Linux. I have worked around this by patching the CMakeLists files to use the system copy of catch (catch1-devel in Fedora), which contains a workaround.

It would be great if there were a CMake Boolean I could set to use an external or system copy of Catch without patching the build system. I would prefer to do that even after leatherman fixes its copy of Catch.

@MikaelSmith
Copy link
Contributor

This seems like something we should fix. I'm happy to accept a patch if you have one for setting up a boolean to use system Catch. Otherwise this may take a few weeks before I can get to it.

@MikaelSmith MikaelSmith self-assigned this Mar 9, 2021
@musicinmybrain
Copy link
Author

My existing patch is a quick and dirty unconditional one:

# Do not use the obsolete vendored copy of the Catch unit testing library
# included with leatherman.
sed -r -i 's/(LEATHERMAN_COMPONENTS)(\b.+)?(\bcatch\b)/\1\2/' CMakeLists.txt
sed -r -i 's|\$\{LEATHERMAN_CATCH_INCLUDE\}|"%{_includedir}/catch"|' \
    lib/tests/CMakeLists.txt

If I have a chance to write a clean version with a boolean, and without platform-dependent assumptions, I will submit a PR and link it here. It won’t be right away but it might be sooner than a few weeks.

@thesamesam
Copy link

FWIW: glibc-2.34 was released in August.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants