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

[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):

Rcpp/inst/include/Rcpp/exceptions_impl.h

Lines 24 to 44 in 6570751

	// 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.

[eddelbuettel]

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 .

[eddelbuettel]

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}.

[eddelbuettel]

Could your __MUSL__ be replaced by gcc and not glibc ?

[bastistician]

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.

[eddelbuettel]

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?

[eddelbuettel]

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

[Enchufa2]

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.

[eddelbuettel]

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.)

[eddelbuettel]

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

[bastistician]

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!

[eddelbuettel]

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.

[bastistician]

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