Skip to content

switch to a whitelist for #include <execinfo.h> #1400

Closed

Description

@bastistician

Build-time feature detection does not seem to be an option here, but I'd like to suggest using a whitelist for backtrace() functionality rather than an ever-growing stoplist (e.g., #1338, #796):

// disable demangler on platforms where we have no support
#ifndef RCPP_DEMANGLER_ENABLED
# if defined(_WIN32) || \
defined(__FreeBSD__) || \
defined(__NetBSD__) || \
defined(__OpenBSD__) || \
defined(__DragonFly__) || \
defined(__CYGWIN__) || \
defined(__sun) || \
defined(_AIX) || \
defined(__MUSL__) || \
defined(__HAIKU__) || \
defined(__ANDROID__)
# define RCPP_DEMANGLER_ENABLED 0
# elif defined(__GNUC__) || defined(__clang__)
# include <execinfo.h>
# define RCPP_DEMANGLER_ENABLED 1
# else
# define RCPP_DEMANGLER_ENABLED 0
# endif
#endif

What brought me here is that the __MUSL__ exception (added long ago in #449) is an unreliable kludge as musl itself doesn't define such a macro (by design).

AFAICS, a suitable whitelist should be as simple as

defined(__GLIBC__) || defined(__APPLE__)

where execinfo is built-in. The additional compiler check (gcc|clang) seems to be redundant (probably a relict). The suggested change is similar in spirit to the original commit in #449, which was discarded simply because it forgot to cover macOS. BSDs have long provided execinfo, too, but a web search suggests they require separate linking with -lexecinfo.

Activity

  1. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    Philosophically speaking, with default behaviour working as is, I am finding that the specific exclusion list is working well. When someone on an exotic system has a need, they usually send a PR. That worked well so far.

    So with that I would prefer it you could devise a way to detect your musl environment. We could also wrap an outer control variable around so that deficient systems that cannot have exceptions can opt to skip them using such an outer variable (off by default, of course). How exactly do things work out here for you? Do you fall into the wrong bucket because you still have gcc?

    But it is all complicated and we probably have whitelists in other places. So open for discussion. We have time til the next release in January anyway .

  2. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    I see this has come up recently-ish?

    6075   │  66854     ripley @item
    6076   │  79948     ripley Some headers are not portable: we have just mentioned @file{malloc.h}
    6077   │  79948     ripley and often @acronym{CRAN} submissions attempt to use @file{endian.h}.
    6078   │  85662     hornik The latter is a @code{glibc} extension: some OSes have
    6079   │  79948     ripley @file{machine/endian.h} or @file{sys/endian.h} but some have neither.
    6080   │  83523     ripley @c macOS has machine/endian.h.  Solaris had none.
    6081   │  87498     hornik Header @file{execinfo.h} is only available on a few OSes: formerly nor
    6082   │  87498     hornik in MacOS nor Solaris, and currently not on Linux systems (such as Alpine
    6083   │  87498     hornik Linux) using @code{musl}.  Nor is header @file{fpu_control.h} available
    6084   │  87495     ripley on macOS nor @code{musl}.
    
  3. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    Could your __MUSL__ be replaced by gcc and not glibc ?

  4. bastistician commented on Oct 20, 2025

    @bastistician
    Author

    I learned the hard way (already a few years ago) that a huge fraction of available R packages fail their installation with something like:

    In file included from ..../lib/Rcpp/include/Rcpp.h:60,
                     from RcppExports.cpp:4:
    ..../lib/Rcpp/include/Rcpp/exceptions_impl.h:39:12: fatal error: execinfo.h: No such file or directory
       39 | #  include <execinfo.h>
          |            ^~~~~~~~~~~~
    compilation terminated.
    

    (Yes, gcc.)
    Of course, I can work around that by setting, e.g., CXXFLAGS="${CXXFLAGS} -D__MUSL__" in my R's config.site as expected by current Rcpp. But I think it would be safer and simpler to test for the (developer) platforms where execinfo is known to work rather than try to exclude all others. __GLIBC__ is certainly not defined by musl.

  5. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    Yes so with the initial comment I made above, could I ask you to maybe devise and test a strategy to detect your build environment reliably and that would free you from have to add the -D__MUSL__ workaround?

    Maybe (defined(__GNUC__) && !defined(__GLIBC__)) does that if musl refuses to supply something better?

  6. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    I will also try to do a code search here at GitHub to see some other best practices for shielding #include <execinfo.h>.

  7. Enchufa2 commented on Oct 20, 2025

    @Enchufa2
    Member

    What about __has_include?

    #if defined __has_include
    #  if __has_include (<execinfo.h>)
    #    include <execinfo.h>
    #  endif
    #endif
    

    10 years ago it was a new-ish preprocessor feature, but nowadays it should be everywhere.

  8. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    Excellent. I was bemoaning all morning that we'd need a test for existence of an include file and forgot we had one! (Just back in from a bike ride.)

  9. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    Technically C++17 per cpp reference but the guard you suggest should do.

  10. bastistician commented on Oct 20, 2025

    @bastistician
    Author

    The __has_include test has the nice little side effect of also succeeding automatically under __ANDROID_API__ >= 33, which introduced <execinfo.h>.

    I guess there is no guarantee that <execinfo.h>, if it exists, will always provide the anticipated backtrace() and backtrace_symbols(), but defined(__GLIBC__) || defined(__APPLE__) should be OK even without a __has_include() check.

    Thanks for the quick responses!

  11. eddelbuettel commented on Oct 20, 2025

    @eddelbuettel
    Member

    I think we prefer switching to that over switching to the whitelist you advocate for.

    The net effect should be the same, and for you and the at most three other musl users 😜 life will be so much easier.

  12. bastistician commented on Oct 22, 2025

    @bastistician
    Author

    Thanks! I can confirm that the patch works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions