boostorg / stacktrace

C++ library for storing and printing backtraces.
https://boost.org/libs/stacktrace
422 stars 69 forks source link

libboost_stacktrace_from_exception doesn't build on macos-14 #165

Closed pdimov closed 3 months ago

pdimov commented 3 months ago

With:

clang-darwin.compile.c++ bin.v2/libs/stacktrace/build/clang-darwin-15/release/arm_64/cxxstd-11-iso/threading-multi/visibility-hidden/from_exception.o
libs/stacktrace/build/../src/from_exception.cpp:170:2: error: On this platform memory leaks are possible if capturing stacktrace from         exceptions is enabled and exceptions are thrown concurrently         and libc++ runtime is used.                 Define `BOOST_STACKTRACE_LIBCXX_RUNTIME_MAY_CAUSE_MEMORY_LEAK` to         suppress this error if the library would not be used with libc++         runtime (for example, it would be only used with GCC runtime).                 Otherwise, disable the boost_stacktrace_from_exception library build         (for example by `./b2 boost.stacktrace.from_exception=off` option).
#error On this platform memory leaks are possible if capturing stacktrace from \
 ^
1 error generated.

    "clang++"   -std=c++11 -fvisibility-inlines-hidden -fPIC -O3 -Wall -fvisibility=hidden -Wno-inline --target=arm64-apple-darwin  -DBOOST_ALL_NO_LIB=1 -DBOOST_COBALT_USE_STD_PMR=1 -DNDEBUG   -I"."  -c -o "bin.v2/libs/stacktrace/build/clang-darwin-15/release/arm_64/cxxstd-11-iso/threading-multi/visibility-hidden/from_exception.o" "libs/stacktrace/build/../src/from_exception.cpp"

...failed clang-darwin.compile.c++ bin.v2/libs/stacktrace/build/clang-darwin-15/release/arm_64/cxxstd-11-iso/threading-multi/visibility-hidden/from_exception.o...

(https://github.com/boostorg/boost/actions/runs/9216984858/job/25358236933)

It's exceedingly unlikely that the library will be used with libstdc++ under macOS when built with Clang, so the default on this platform should be to not build the library.

pdimov commented 3 months ago

One easy way to fix that would be to add

<architecture>arm,<target-os>darwin,<toolset>clang:<build>no

to the boost_stacktrace_from_exception requirements.

That's not quite right because we'd ideally want to be able to override this via boost.stacktrace.from_exception=on, but this isn't possible currently because on is the default value of this feature, so we can't know whether it was explicitly given.

If we really care about that, we can change this to

feature.feature boost.stacktrace.from_exception : default on off : optional propagated ;

and then add <boost.stacktrace.from_exception>default to the conditions above.

I'd like to have this fixed because it breaks the boostorg/boost CI at the moment, so comments are appreciated.

pdimov commented 3 months ago

Or perhaps, given that according to https://github.com/boostorg/stacktrace/issues/163 the feature never works on non-x86 by default, the right incantation would be

<build>no
<boost.stacktrace.from_exception>on:<build>yes
<architecture>x86:<build>yes

?

Not sure. @grisumbras any comments?

grisumbras commented 3 months ago

Optional features aren't added to the property set by default, so if there is <boost.stacktrace.from_exception>on in the property set, then it was explicitly added.

And wouldn't your suggestion add both <build>no and <build>yes to the property set? It should probably use a conditional property with a rule

rule build_from_exception( props * )
{
  local result = [ property.select <build> : $(props) ] ;
  if $(result) { return $(result) ; }

  local enabled = [ property.select <boost.stacktrace.from_exception> ] ;
  switch $(enabled:G)
  {
    case on:   return ;
    case off:  return <build>no;
  }

  if [ property.select <architecture> ] != x86
  {
    return <build>no ;
  }
}
pdimov commented 3 months ago

And wouldn't your suggestion add both <build>no and <build>yes to the property set?

I thought b2 was smart enough to figure out that the more specific one of these takes precedence.

pdimov commented 3 months ago

Yes, I just tried

    <build>no
    <architecture>x86:<build>yes
    <boost.stacktrace.from_exception>on:<build>yes

and it works as I expected.

This is not quite correct though because it doesn't handle <boost.stacktrace.from_exception>off correctly, it should be

    <build>no
    <architecture>x86:<build>yes
    <boost.stacktrace.from_exception>on:<build>yes
    <architecture>x86,<boost.stacktrace.from_exception>off:<build>no

I think.

grisumbras commented 3 months ago

To be honest, this looks too cryptic, so I'd still choose the rule.

pdimov commented 3 months ago

Eh, with the additon, no dice

C:/boost-git/develop/tools/build/src/build\targets.jam:1133: in evaluate-requirements from module targets
error: Can not evaluate conditional properties <architecture>x86,<boost.stacktrace.from_exception>off:<build>no <architecture>x86:<build>yes <boost.stacktrace.from_exception>on:<build>yes <conditional>@Jamfile<C:\boost-git\develop>%Jamfile<C:\boost-git\develop>.clang-darwin-cxxstd-11 <conditional>@Jamfile<C:\boost-git\develop>%Jamfile<C:\boost-git\develop>.handle-static-runtime <conditional>@Jamfile<C:\boost-git\develop>%boostcpp.deduce-address-model <conditional>@Jamfile<C:\boost-git\develop>%boostcpp.deduce-architecture <conditional>@Jamfile<C:\boost-git\develop>%threadapi-feature.detect <conditional>@object(check-target-builds-worker)@10081%check <python>2.7,<target-os>windows:<python.interpreter>C:/Python27\python <python>3.9,<target-os>windows:<python.interpreter>C:/Python39\python <target-os>linux:<library>/C:/boost-git/develop/libs/stacktrace/build//dl <toolset>como-linux:<define>_GNU_SOURCE=1 <toolset>como:<link>static <toolset>msvc,<runtime-link>shared:<threading>multi
pdimov commented 3 months ago

The rule is way more cryptic to me, but I don't care one way or the other, as long as it works.

pdimov commented 3 months ago

Have I mentioned that each time I encounter an optional property, it doesn't quite work?

pdimov commented 3 months ago

Your rule, after I fix the seven or so syntax errors, doesn't seem to work.

grisumbras commented 3 months ago

Fixed version:

import property ;
rule build-from-exception ( props * )
{
    local result = [ property.select <build> : $(props) ] ;
    if $(result) { return $(result) ; }

    local enabled = [ property.select <boost.stacktrace.from_exception> : $(props) ] ;
    switch $(enabled:G=)
    {
        case "on" :  return ;
        case off  :  return <build>no ;
    }

    local arch = [ property.select <architecture> : $(props) ] ;
    if $(arch) && ( $(arch) = x86 )
    {
        return <build>no ;
    }
}
pdimov commented 3 months ago

I don't think this works either; we want <build>no when the architecture isn't x86, not when it is.

And separately, when I tried a fixed version of your previous suggestion, b2 build boost.stacktrace.from_exception=on didn't build the library for some reason.

grisumbras commented 3 months ago

The reason my original rule didn't work is exactly because it returned <build>no when <architecture> was not in the property set. It went like this:

  1. My rule is evaluated, it returns <build>no because no <architecture>.
  2. boostcpp.deduce-architecture is evaluated, it returns <architecture>x86.
  3. My rule is evaluated again with the updated property set. The property set has <build>no from step 1, so that's what it is returning.

The fixed rule in step 1 doesn't return <build>no if no <architecture>, so on step 3 it can see that the architecure is in fact x86.

Since all targets in Boost tree have <conditional>@boostcpp.deduce-architecture, this will work for all deducible architectures. For non-deducible architectures I don't have a solution, I'm afraid.

Well, I do: make a config check:

import property ;
import configure ;
import boostcpp ;
rule build-from-exception ( props * )
{
    local enabled = [ property.select <boost.stacktrace.from_exception> : $(props) ] ;
    switch $(enabled:G=)
    {
        case "on" :  return ;
        case off  :  return <build>no ;
    }

    local arch = [ property.select <architecture> : $(props) ] ;
    if $(arch) && ( $(arch) = x86 )
    {
        return ;
    }

    local filtered = [ boostcpp.toolset-properties $(props) ] ;
    local deduced = [ configure.find-builds "default architecture" : $(filtered)
        : /boost/architecture//x86 ] ;
    if $(deduced)
    {
        return ;
    }

    return <build>no ;
}

Do we want to go there?

pdimov commented 3 months ago

Why didn't you post a working version of the previous rule? Is it

rule build-from-exception ( props * )
{
    local result = [ property.select <build> : $(props) ] ;
    if $(result) { return $(result) ; }

    local enabled = [ property.select <boost.stacktrace.from_exception> : $(props) ] ;
    switch $(enabled:G=)
    {
        case "on" :  return ;
        case off  :  return <build>no ;
    }

    local arch = [ property.select <architecture> : $(props) ] ;
    if $(arch) && ( $(arch) = x86 )
    {
        return ;
    }

    return <build>no ;
}

?

pdimov commented 3 months ago

No, that doesn't work either :-)

grisumbras commented 3 months ago

Why didn't you post a working version of the previous rule?

https://github.com/boostorg/stacktrace/issues/165#issuecomment-2130302661 worked for me. Note, that I edited it after posting (it had a typo).

pdimov commented 3 months ago

I don't see how

    local arch = [ property.select <architecture> : $(props) ] ;
    if $(arch) && ( $(arch) = x86 )
    {
        return <build>no ;
    }

can possibly work, since it checks the wrong condition (= instead of !=).

We want to build when the architecture is x86, not when it isn't.

It "worked" because = x86 is never true because of the missing :G=.

This one seems to work for me, although I'm not sure how to test it for non-x86:

rule build-stacktrace-from-exception ( props * )
{
    local build = [ property.select <build> : $(props) ] ;
    local enabled = [ property.select <boost.stacktrace.from_exception> : $(props) ] ;
    local arch = [ property.select <architecture> : $(props) ] ;

    if $(build)
    {
        return $(build) ;
    }

    switch $(enabled:G=)
    {
        case  "on" :  return ;
        case "off" :  return <build>no ;
    }

    if $(arch) && ( $(arch:G=) != x86 )
    {
        return <build>no ;
    }
}
grisumbras commented 3 months ago

Sorry for the garbage I posted before. This one I tested with b2 libs/stacktrace/build (builds), b2 libs/stacktrace/build boost.stacktrace.from_exception=on (builds), b2 libs/stacktrace/build boost.stacktrace.from_exception=off (doesn't build), b2 libs/stacktrace/build architecture=arm (doesn't build), b2 libs/stacktrace/build architecture=arm boost.stacktrace.from_exception=off (doesn't build), and b2 libs/stacktrace/build architecture=arm boost.stacktrace.from_exception=on (tries to build, but fails as expected).

import property ;
rule build-from-exception ( props * )
{
    local enabled = [ property.select <boost.stacktrace.from_exception> : $(props) ] ;
    switch $(enabled:G=)
    {
        case "on" :  return ;
        case off  :  return <build>no ;
    }

    local arch = [ property.select <architecture> : $(props) ] ;
    if $(arch) && ( $(arch:G=) != x86 )
    {
        return <build>no ;
    }
}

It's basically what you posted, but I removed checking for <build>, as it is unnecessary.

pdimov commented 3 months ago

PR created: https://github.com/boostorg/stacktrace/pull/166.