Re: PATCH: Reguire __cxa_atexit for ext/{new, throw}_allocator/deallocate_global.cc

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

On 01/03/2010 20:27, Rainer Orth wrote:
> Dave Korn <dave.korn.cygwin@googlemail> writes:
> 
>> Shouldn't this be xfailed on solaris, rather than skipped on all 
>> non-cxa_atexit targets?
> 
> no, this way lies madness ;-(  I'm strongly opposed to added any 
> target-based xfails or skips if there's a general feature that can be 
> tested for.  If it's not __cxa_atexit, suggest another name for the feature
> test, and we can check for cygwin there in some way.  A prominent example
> (which I'll attack soon) is the large list of different target lists used
> to decide if the target has pthread support.  This is both inconsistent and
> a nightmare to maintain, e.g. if adding a new target.
> 
>> The FAIL in this case is indicating an ABI violation w.r.t order of
>> dtors, which could equally happen on non-cxa_atexit systems that
>> implement some variant scheme (such as e.g. get_exit_frame_monitor
>> hooks!) and it seems useful to me if the testsuite flags that up,
>> regardless of the internal implementation details of how the system
>> handles (or, in this case, fails to handle correctly) finalisation.
> 
> Exactly my suggestion: just give an appropriate name and add your target 
> (or a runtime test) to the feature test.

  Argh.  Look, ok, I don't understand.  You wrote:

> I'm pretty sure this happens because the destructors aren't run in the 
> sequence expected, since Solaris 2 lacks __cxa_atexit().

  As far as I can possibly understand, that is a completely true statement.

> To avoid this problem, the correct solution seems to be to require 
> __cxa_atexit,

  As far as I can possibly understand, that, on the other hand, is a basic
logical fallacy, exactly the same as "A=>B therefore !A=>!B".  The only valid
syllogism is "!B=>!A".  I very seriously object to installing any patch, the
justification for which is a logical fallacy.

  I (think) I understand what you mean by asking for a feature test, but I
can't see how it makes sense to ask for one.  If there is a "feature" that
this testcase verifies, it could be any one of (from least to most specific):

- supports c++
- supports c++, including ctors and dtors
- supports c++, including ctors and dtors, including for static objects
- supports c++, including ctors and dtors, including for static objects, also
does it correctly rather than incorrectly.

  As far as I can see, this is a real genuine bug in the platform support for
one specific target only.  It does not make sense for every single
target-specific failure to create an entire new category called "supports c++
in every way except for this one specific bug".  Really, the testcase should
not be xfailed at all, it should continue to fail because of the very real bug
it exposes; however, since the Solaris target maintainer wants to cut down the
noise in the test results, it's fine to xfail it for that target, since as
Rainer has already explained, there are potential solutions in hand and until
then it's just noise.

  The problem *is* target based, and purely target based.  Any target that
claims to support the C++ language standard should pass this test; any target
that does not, has a bug.  It is not "feature"-dependent, there is not any
feature that says it's ok to get this test wrong.  And this test is not
specifically a test of the cxa_atexit function, so it should not be expected
to fail on targets that use different cxx-abi implementations.

  Sorry if I've missed your point.  I don't think it makes sense to try and
describe every single bug that happens on a single target only in terms of
some hypothetical generalised feature, because as far as I know none of this
is optional, so it all ought to work regardless of how it is implemented.  So
I honestly admit I don't understand why you're asking me this.  (I know you're
not daft, I know you must have a good reason, but I just don't get it!)

    cheers,
      DaveK