Beefy Boxes and Bandwidth Generously Provided by pair Networks
more useful options
 
PerlMonks  

[Solved] XS debugging "failed to extend arg stack"

by NERDVANA (Deacon)
on Jun 02, 2022 at 17:27 UTC ( [id://11144355]=perlquestion: print w/replies, xml ) Need Help??

NERDVANA has asked for the wisdom of the Perl Monks concerning the following question:

Hi, my Tree::RB::XS module just recently started throwing an exception on new perls built with debugging enabled. It claims I wrote past the end of the stack (SP) during "$iter->next_kv". I've done a fair amount of debugging on it and just don't see the bug. I'm probably missing some magic XS macro... maybe a second pair of eyes can spot it?

next_kv is a method that returns one or more key+value pairs, as a list. So, the function could return up to 2x the number of nodes in the tree, or zero, or usually just one for most of the ways the function can be called. To make things more complicated, I decided to share the implementation of this method with the 'next_keys', 'next_values', and 'next' methods, which are all basically the same algorithm but return different things to the caller. I used perl's "ix" aliasing mechanism.

Here is the XS code.

The line of the test that fails is
is( [ $tree->iter(1)->next_kv(2) ], [ 1, 2, 2, 4 ], 'next_kv' );
which means "starting from element key==1, return the next two key+value pairs, which in this unit test happen to be { 1 => 2, 2 => 4 }

Here is The C code it generates, with a warning added for every occurrence of EXTEND and XSRETURN and usage of ST(x) in the case where ix == 3 (which is what is failing).

My understanding of XS is that in a xsub declared with PPCODE, the SP is pointed at the first argument, so the stack is at least as long as the number of arguments received. If you want to return more elements than the number of arguments passed to you, you need to call EXTEND(SP, n) where n is the number of return values. (in the code, I make the assumption that there was always one argument because it is a method that requires one, and so ST(0) should always be safe to use.)

This is the relevant output of the unit test:

EXTEND(SP, 4 at t/21-iter.t line 167. Accessing ST(0), ST(1) at t/21-iter.t line 167. Accessing ST(2), ST(3) at t/21-iter.t line 167. XSRETURN(4) at t/21-iter.t line 167. not ok 6 - iter_get_multi { ok 1 - next_keys(*) ok 2 - next_keys overshoot ok 3 - reverse next_keys overshoot ok 4 - reverse next_keys for 1 ok 5 - iterate nothing ok 6 - next_values(4) ok 7 - next_nodes(2) } # Failed test 'iter_get_multi' # at t/21-iter.t line 172. # Caught exception in subtest: panic: XSUB Tree::RB::XS::Iter::next_kv + (TreeRBXS.c) failed to extend arg stack: base=56076974fa88, sp=56076 +974faa8, hwm=56076974faa0

If you want to compile it, the source uses Dist::Zilla which I know offends some people. However, you can just grab it from CPAN because the current HEAD of the git repo is exactly what is published right now.

Note that the assertion only fails on debug builds of perl. I was unable to reproduce the error until I realized that. I used:
perlbrew install -j 10 --as p536-debug --noman --debug --thread perl-5.36.0

Replies are listed 'Best First'.
Re: XS debugging "failed to extend arg stack"
by hv (Prior) on Jun 03, 2022 at 02:05 UTC

    For the record, along the way I notice a) that metacpan does not know your module depends on ExtUtils::Depends and Test2::Suite, and b) in the module you appear to invoke Scalar::Util::weaken without loading Scalar::Util.

    After confusing myself a few times, I've come to the conclusion that you've hit an unfortunate case - the EXTEND macro evaluates its arguments several times. Along the way it invokes EXTEND_HWM_SET, which looks like this for a debugging build:

    # define EXTEND_HWM_SET(p, n) \ STMT_START { \ SSize_t ix = (p) - PL_stack_base + (n); \ if (ix > PL_curstackinfo->si_stack_hwm) \ PL_curstackinfo->si_stack_hwm = ix; \ } STMT_END

    So given that n here is the string ix==3 ? 2*n : n, it is quite reasonable things are going to go wrong. If you use some variable other than ix, or (for example) duplicate the EXTEND call inside the if/else blocks to avoid the conditional, then the error goes away.

    Update: #19818

    HTH, Hugo

      I've come to the conclusion that you've hit an unfortunate case - the EXTEND macro evaluates its arguments several times

      It sounds like you're saying that's the problem, but it took a a few reads to realize this isn't actually a problem here. The actual problem is the fact that the macro and the caller both use a var named ix.

      NERDVANA said,

      A good reminder to not use expressions inside macros.

      The relevant lesson is to not use variable names in macros that could possibly be used outside the macro.

        Yes, I probably didn't express myself as clearly as I could have done.

        Of course _any_ variable name "could possibly" be used outside a macro: macros don't come with much by way of safety features. I've been suggesting for a while that we should replace as many as we can with inline functions - which also gives type safety (as much as C has, at least), and protects against repeated evaluation of arguments - but for reasons which escape me there seems little enthusiasm for such a change.

        (I briefly considered suggesting I should write an essay "macros considered harmful", but then I read Considered harmful and thought better of it.)

        And especially since I didn't decide to name the variable 'ix'. 'ix' is defined by the xs tooling in any Perl xsub which uses aliasing, and the EXTEND macro is only used in xsubs anyway. So it's a natural name collision.
      Wow, thanks! A good reminder to not use expressions inside macros. And I'll get the metadata fixed too.
        I don't understand why one shouldn't use expressions inside macros. If you comply with using symbols that don't clash with those of callers, expressions should be no different than a simple variable, as far as I understand
      On your point about the dependencies, I have them listed in the META.json and .yml:

      "configure" : { "requires" : { "ExtUtils::Depends" : "0.405", "ExtUtils::MakeMaker" : "0" } }, "develop" : { "requires" : { "Devel::PPPort" : "3.23", "Pod::Coverage::TrustPod" : "0", "Test::More" : "0.88", "Test::Pod" : "1.41", "Test::Pod::Coverage" : "1.08" } }, "runtime" : { "requires" : { "Carp" : "0", "Exporter" : "0", "XSLoader" : "0", "strict" : "0", "warnings" : "0" } }, "test" : { "requires" : { "Scalar::Util" : "0", "Test2::Suite" : "0.000139", "Test2::V0" : "0", "Time::HiRes" : "0" } }

      Is this more Metacpan's fault for not showing dependencies from phases other than Runtime? or is this sort of fine-grain dependency listing not ready for prime-time?

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://11144355]
Approved by Fletch
Front-paged by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others chanting in the Monastery: (3)
As of 2024-04-20 08:35 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found