Project

General

Profile

Bug #10220

libproc ia32 Pstack_iter() should leverage ctf

Added by Robert Mustacchi 9 months ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
2019-01-11
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

libproc provides the Pstack_iter() function. When debugging i386 processes, it tries to guess the number of arguments using a heuristic that generally fails and falls back to assuming there are always six arguments for a given function on the stack. This then confuses folks using mdb and pstack. On the amd64 side we use a combination of CTF data and libsaveargs. In the 32-bit case we don't need to use libsaveargs because everything is passed on the stack (generally); however, we can leverage the existence of CTF data to get a more accurate number of arguments. The use of CTF and stack searching isn't 100% accurate right now because we're not looking at all the variants in the ABI like small structures or other things being passed, but this is a vast improvement that deals with almost all of the day to day cases.

To test this I went through and wrote a sample program to exercise this behavior:

#include <stdio.h>

void
a8(int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8)
{
        a1++;
        a2++;
        a3++;
        a4++;
        a5++;
        a6++;
        a7++;
        a8++;
        printf("a8 %d %d %d %d %d %d %d %d\n", a1, a2, a3, a4, a5, a6, a7, a8);
}

void
a6(int a1, int a2, int a3, int a4, int a5, int a6)
{

        a1++;
        a2++;
        a3++;
        a4++;
        a5++;
        a6++;
        a8(a1, a2, a3, a4, a5, a6, 23, 123);
        printf("a6 %d %d %d %d %d %d\n", a1, a2, a3, a4, a5, a6);
}

void
a4(int a1, int a2, int a3, int a4)
{
        a1++;
        a2++;
        a3++;
        a4++;
        a6(a1, a2, a3, a4, 0, -1);
        printf("a4 %d %d %d %d\n", a1, a2, a3, a4);
}

void
a2(int a1, int a2)
{
        a1++;
        a2++;
        a4(a1, a2, 1, 0);
        printf("a2 %d %d\n", a1, a2);
}

void
a0(void)
{
        a2(2, 1);
        printf("a0!\n");
}

int
main(void)
{
        a0();
}


I built this as a 32-bit program and added CTF to it so we could see the difference. Here's an example of what mdb shows before when we break after we step into a8:
$ mdb ./mdb
> a8+6::bp
> :c
mdb: stop at a8+6
mdb: target stopped at:
a8+6:           addl   $0x1,0x8(%ebp)
> $C
08047b28 a8+6(5, 4, 3, 2, 1, 0)
08047b58 a6+0x39(5, 4, 3, 2, 1, 0)
08047b88 a4+0x2e(4, 3, 2, 1)
08047ba8 a2+0x1d(3, 2, 8047c40, 80512da)
08047bc8 a0+0x12(80613b4)
08047bd8 main+0x16(8047bdc, fef52388, 8047c18, 8050fb8, 1, 8047c40)
08047c18 _start_crt+0x97(1, 8047c40, fefd0e28, 0, 0, 0)
08047c34 _start+0x1a(1, 8047d5c, 0, 8047d60, 8047d70, 8047d7b)

Note how a2 and a0 have extra arguments. a8 should have 8 arguments; however, it's impossible to tell if mdb would have previously found more or not because the stack printing code limits at 6 arguments. With an updated libproc, we now see:

> a8+6::bp
> :c
mdb: stop at a8+6
mdb: target stopped at:
a8+6:           addl   $0x1,0x8(%ebp)
> $C
08047af8 a8+6(5, 4, 3, 2, 1, 0)
08047b28 a6+0x39(5, 4, 3, 2, 1, 0)
08047b58 a4+0x2e(4, 3, 2, 1)
08047b78 a2+0x1d(3, 2)
08047b98 a0+0x12()
08047ba8 main+0x16()
08047be8 _start_crt+0x97(1, 8047c10, fefd0e28, 0, 0, 0)
08047c04 _start+0x1a(1, 8047d2c, 0, 8047d30, 8047d40, 8047d4b)

To prove that we get all the arguments, we can also use pstack on the process that's stopped in place by mdb. Here's the new version:

4735:   mdb
 0805114f a8       (5, 4, 3, 2, 1, 0, 17, 7b) + 6
 080511d2 a6       (5, 4, 3, 2, 1, 0) + 39
 08051227 a4       (4, 3, 2, 1) + 2e
 08051265 a2       (3, 2) + 1d
 08051292 a0       () + 12
 080512bd main     () + 16
 08050fb8 _start_crt (1, 8047c10, fefd0e28, 0, 0, 0) + 97
 08050e8a _start   (1, 8047d2c, 0, 8047d30, 8047d40, 8047d4b) + 1a

Note how we now have 8 arguments for a8 since pstack has a higher default argument limit than mdb.

Now, let's see this applied to something much larger, like startd. HEre's a unified diff of the pstack output:

$ diff -u before after 
--- before      2018-12-26 16:14:09.168380001 +0000
+++ after       2018-12-26 16:14:16.043303357 +0000
@@ -1,91 +1,91 @@
 3048:  /lib/svc/bin/svc.startd
 --------------------- thread# 1 / lwp# 1 ---------------------
  feec6e05 sigsuspend (8047e00)
- 0807604d main     (8047e3c, fef42388, 8047e70, 805af53, 1, 8047e7c) + 20e
+ 0807604d main     (8047e3c, fef42388) + 20e
  0805af53 _start   (1, 8047f54, 0, 8047f6c, 8047f8c, 8047f93) + 83
 --------------------- thread# 2 / lwp# 2 ---------------------
  feec6795 ioctl    (3, 63746502, 80b4fa8)
- fed92fbd ct_event_read_critical (3, fed3efac, fed3efa8, 1, fef3cd60, fedc2000) + 16
- 0805eb21 fork_configd_thread (ffffffff, 0, 0, 0) + 14b
+ fed92fbd ct_event_read_critical (3, fed3efac) + 16
+ 0805eb21 fork_configd_thread (ffffffff) + 14b
  feec212b _thrp_setup (fec30240) + 88
  feec22c0 _lwp_start (fec30240, 0, 0, 0, 0, 0)
 ------- thread# 3 / lwp# 3 [restarter_timeouts_event] --------
  feec2319 lwp_park (0, 0, 0)
- feebbff8 cond_wait_queue (80b3f80, 80b3f68, 0, 8056657, 0, 8059110) + 6a
- feebc2cc cond_wait_common (80b3f80, 80b3f68, 0, fef35000, fec30a40, 80afee4) + 27b
- feebc689 __cond_wait (80b3f80, 80b3f68, fe91efa8, fef35000, fef35000, fec30a40) + a8
- feebc6c4 cond_wait (80b3f80, 80b3f68, 1, 807180d, fec30a40, fef35000) + 2e
- feebc70d pthread_cond_wait (80b3f80, 80b3f68, fe91efc8, 8071961, fec30a40, 0) + 24
- 080719d0 restarter_timeouts_event_thread (0, 0, 0, 0) + 90
+ feebbff8 cond_wait_queue (80b3f80, 80b3f68, 0) + 6a
+ feebc2cc cond_wait_common (80b3f80, 80b3f68, 0) + 27b
+ feebc689 __cond_wait (80b3f80, 80b3f68) + a8
+ feebc6c4 cond_wait (80b3f80, 80b3f68) + 2e
+ feebc70d pthread_cond_wait (80b3f80, 80b3f68) + 24
+ 080719d0 restarter_timeouts_event_thread (0) + 90
  feec212b _thrp_setup (fec30a40) + 88
  feec22c0 _lwp_start (fec30a40, 0, 0, 0, 0, 0)
 ------------ thread# 4 / lwp# 4 [restarter_event] ------------
  feec2319 lwp_park (0, 0, 0)
- feebbff8 cond_wait_queue (80b3fb0, 80b3f98, 0, feb4a873, 809c290, 809c680) + 6a
- feebc2cc cond_wait_common (80b3fb0, 80b3f98, 0, feebbc28, 80aff04, 0) + 27b
- feebc689 __cond_wait (80b3fb0, 80b3f98, fe81ff88, 2, fef35000, 825fbb0) + a8
- feebc6c4 cond_wait (80b3fb0, 80b3f98, fe81ffa8, 8070189, 80aff04, 0) + 2e
- feebc70d pthread_cond_wait (80b3fb0, 80b3f98, fe81ffc8, 8073377) + 24
- 0807308e restarter_event_thread (0, 0, 0, 0) + 72
+ feebbff8 cond_wait_queue (80b3fb0, 80b3f98, 0) + 6a
+ feebc2cc cond_wait_common (80b3fb0, 80b3f98, 0) + 27b
+ feebc689 __cond_wait (80b3fb0, 80b3f98) + a8
+ feebc6c4 cond_wait (80b3fb0, 80b3f98) + 2e
+ feebc70d pthread_cond_wait (80b3fb0, 80b3f98) + 24
+ 0807308e restarter_event_thread (0) + 72
  feec212b _thrp_setup (fec31240) + 88
  feec22c0 _lwp_start (fec31240, 0, 0, 0, 0, 0)
 ------- thread# 5 / lwp# 5 [restarter_contracts_event] -------
  feec6795 ioctl    (85, 63746502, 81a1418)
- fed92fbd ct_event_read_critical (85, fe720fac, fe720fc8, 8073fe3) + 16
- 0807412b restarter_contracts_event_thread (0, 0, 0, 0) + 156
+ fed92fbd ct_event_read_critical (85, fe720fac) + 16
+ 0807412b restarter_contracts_event_thread (0) + 156
  feec212b _thrp_setup (fec31a40) + 88
  feec22c0 _lwp_start (fec31a40, 0, 0, 0, 0, 0)
 ----------------- thread# 6 / lwp# 6 [wait] ------------------
  feec6017 portfs   (5, 5, fe621fa0, 0, 0, 0)
- 08076bab wait_thread (0, 0, 0, 0) + 36
+ 08076bab wait_thread (0) + 36
  feec212b _thrp_setup (fec32240) + 88
  feec22c0 _lwp_start (fec32240, 0, 0, 0, 0, 0)
 ----------------- thread# 7 / lwp# 7 [graph] -----------------
  feec2319 lwp_park (0, 0, 0)
- feebbff8 cond_wait_queue (80b2fac, 80b2f94, 0, 1, 0, feb18000) + 6a
- feebc2cc cond_wait_common (80b2fac, 80b2f94, 0, feeba846, 80943b8, 0) + 27b
- feebc689 __cond_wait (80b2fac, 80b2f94, 80afd08, 80b7548, fef35000, fec32a40) + a8
- feebc6c4 cond_wait (80b2fac, 80b2f94, 0, 80b7548, fec32a40, 80b7548) + 2e
- feebc70d pthread_cond_wait (80b2fac, 80b2f94, fe522fc8, 8067c24) + 24
- 08067cae graph_thread (0, 0, 0, 0) + 2b2
+ feebbff8 cond_wait_queue (80b2fac, 80b2f94, 0) + 6a
+ feebc2cc cond_wait_common (80b2fac, 80b2f94, 0) + 27b
+ feebc689 __cond_wait (80b2fac, 80b2f94) + a8
+ feebc6c4 cond_wait (80b2fac, 80b2f94) + 2e
+ feebc70d pthread_cond_wait (80b2fac, 80b2f94) + 24
+ 08067cae graph_thread (0) + 2b2
  feec212b _thrp_setup (fec32a40) + 88
  feec22c0 _lwp_start (fec32a40, 0, 0, 0, 0, 0)
 ----------- thread# 8 / lwp# 8 [repository_event] ------------
  feec7698 door     (a, fe423c28, 0, 0, 0, 3)
- feeb3e5b door_call (a, fe423c28, 2d4, feacaff6) + 96
+ feeb3e5b door_call (a, fe423c28) + 96
  feacb06a make_door_call_retfd (a, fe423f68, 8, fe423c94, 2d4, fe423c90) + 85
- fead4cd7 _scf_notify_wait (80afa48, 80b8d40, 275, 80c3000) + d6
- 080672fb repository_event_thread (0, 0, 0, 0) + 12c
+ fead4cd7 _scf_notify_wait (80afa48, 80b8d40, 275) + d6
+ 080672fb repository_event_thread (0) + 12c
  feec212b _thrp_setup (fec33240) + 88
  feec22c0 _lwp_start (fec33240, 0, 0, 0, 0, 0)
 -------------- thread# 9 / lwp# 9 [graph_event] --------------
  feec2319 lwp_park (0, 0, 0)
- feebbff8 cond_wait_queue (80b2f80, 80b2f68, 0, feb4a873, 81fe620, 809ce40) + 6a
- feebc2cc cond_wait_common (80b2f80, 80b2f68, 0, feeba846, 80b2f74, 0) + 27b
- feebc689 __cond_wait (80b2f80, 80b2f68, fe324f98, 80b7008, fef35000, 0) + a8
- feebc6c4 cond_wait (80b2f80, 80b2f68, fe324fb8, 80b7008, 0, 80b7008) + 2e
- feebc70d pthread_cond_wait (80b2f80, 80b2f68, fe324fc8, 8061d71, fef35000, fec33a40) + 24
- 08061c50 graph_event_thread (0, 0, 0, 0) + 7e
+ feebbff8 cond_wait_queue (80b2f80, 80b2f68, 0) + 6a
+ feebc2cc cond_wait_common (80b2f80, 80b2f68, 0) + 27b
+ feebc689 __cond_wait (80b2f80, 80b2f68) + a8
+ feebc6c4 cond_wait (80b2f80, 80b2f68) + 2e
+ feebc70d pthread_cond_wait (80b2f80, 80b2f68) + 24
+ 08061c50 graph_event_thread (0) + 7e
  feec212b _thrp_setup (fec33a40) + 88
  feec22c0 _lwp_start (fec33a40, 0, 0, 0, 0, 0)
 ------------------- thread# 180 / lwp# 180 -------------------
  feec78de door     (fc443cc8, 4, 0, fc443e00, f5f00, a)
- fe9875fb door_upcall (8104dd0, fc443cc8, 138, 0, 0, fe98758c) + 6f
+ fe9875fb door_upcall (8104dd0, fc443cc8, 138, 0, 0) + 6f
  feec78fb __door_return () + 4b
 -------------------- thread# 26 / lwp# 26 --------------------
  feec2319 lwp_park (0, fd235f18, 0)
- feebbff8 cond_wait_queue (feb89818, feb89838, fd235f18, fee9da38, 3, fd235eb8) + 6a
- feebc2cc cond_wait_common (feb89818, feb89838, fd235f18, feebabd9, fed40680, 0) + 27b
- feebc53a __cond_timedwait (feb89818, feb89838, fd235f9c, feb72000, feb72000, 0) + 111
- feebc574 cond_timedwait (feb89818, feb89838) + 35
- feb44be3 umem_update_thread (0, 0, 0, 0) + 1f9
+ feebbff8 cond_wait_queue (feb89818, feb89838, fd235f18) + 6a
+ feebc2cc cond_wait_common (feb89818, feb89838, fd235f18) + 27b
+ feebc53a __cond_timedwait (feb89818, feb89838, fd235f9c) + 111
+ feebc574 cond_timedwait (feb89818, feb89838, fd235f9c) + 35
+ feb44be3 umem_update_thread (0) + 1f9
  feec212b _thrp_setup (fec3c240) + 88
  feec22c0 _lwp_start (fec3c240, 0, 0, 0, 0, 0)
 ------------------- thread# 184 / lwp# 184 -------------------
  feec78de door     (fdd2acc8, 4, 0, fdd2ae00, f5f00, a)
- fe9875fb door_upcall (8104dd0, fdd2acc8, 138, 0, 0, fe98758c) + 6f
+ fe9875fb door_upcall (8104dd0, fdd2acc8, 138, 0, 0) + 6f
  feec78fb __door_return () + 4b
 -------------------- thread# 47 / lwp# 47 --------------------
  feec78de door     (fc047cc8, 4, 0, fc047e00, f5f00, a)
- fe9875fb door_upcall (8104dd0, fc047cc8, 138, 0, 0, fe98758c) + 6f
+ fe9875fb door_upcall (8104dd0, fc047cc8, 138, 0, 0) + 6f
  feec78fb __door_return () + 4b

If we look at a lot of the functions here, they now have the correct number of arguments. Take the cond* family or door_upcalls(). Everything now has the right arguments.

History

#1

Updated by Electric Monk 9 months ago

  • Status changed from New to Closed

git commit 8f697f9a1a91761e318c95fdebf19480d38ce836

commit  8f697f9a1a91761e318c95fdebf19480d38ce836
Author: Robert Mustacchi <rm@joyent.com>
Date:   2019-01-18T19:33:25.000Z

    10220 libproc ia32 Pstack_iter() should leverage ctf
    Reviewed by: Dan McDonald <danmcd@joyent.com>
    Reviewed by: Jason King <jason.king@joyent.com>
    Reviewed by: John Levon <john.levon@joyent.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF