Telescope display ret addr #1071 #1889

dmur1 · 2023-08-25T18:27:44Z

seeks to fix #1071

codecov · 2023-08-25T18:36:29Z

Codecov Report

Attention: Patch coverage is 88.09524% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 59.44%. Comparing base (e8f423a) to head (ad14944).
Report is 15 commits behind head on dev.

Files	Patch %	Lines
pwndbg/commands/telescope.py	85.00%	1 Missing and 2 partials ⚠️
pwndbg/gdblib/stack.py	89.47%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1889      +/-   ##
==========================================
- Coverage   59.55%   59.44%   -0.12%     
==========================================
  Files         187      191       +4     
  Lines       23484    24327     +843     
  Branches     2278     2421     +143     
==========================================
+ Hits        13985    14460     +475     
- Misses       8754     9091     +337     
- Partials      745      776      +31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

disconnect3d · 2023-08-27T09:45:44Z

pwndbg/gdblib/stack.py

+ stack = pwndbg.gdblib.vmmap.find(sp)
+
+ # Enumerate all return addresses
+ frame = gdb.newest_frame()


What happens if GDB can't figure out stack frames?

not sure 🤷‍♂️ this pattern is used in a lot of places ( just getting the frame and assuming its valid ) fwiw

pwndbg/gdblib/stack.py

disconnect3d · 2023-08-27T09:50:59Z

There are couple of things here:

Is this relevant no matter where return addresses are located? If we telescope a heap memory, do we want a return address to show there?
If we show return addresses, should we also show saved BP addresses if they are there? I am not sure but I bet the frame object has this info
We need to document this behavior, and likely allow the user to disable it
This will probably crash if the stack frame makes no sense for GDB. You should be able to try it out by doing set $rsp=$rsp+8 or set $rsp=0x1234 and seeing what it will do then

disconnect3d · 2023-10-07T11:06:27Z

@dmur1 ping :P

OBarronCS · 2023-12-01T06:31:08Z

This would be great to add. Perhaps a compromise for the issue of performance (reading the entire stack every time) would be to pass in a range to yield_return_addresses, like yield_return_addresses(start,stop), so that when this is used in telescope or the context output, it will only ever search a very small range of addresses in most cases, and not the entire stack. Additionally, keeping it configurable at a global level, as well as perhaps a flag could exist to pass to telescope to disable it for a given run, too.

disconnect3d · 2024-01-10T19:03:57Z

@dmur1 any chance u can update this PR/fix conflicts etc?

dmur1 · 2024-02-22T09:47:29Z

@dmur1 any chance u can update this PR/fix conflicts etc?

yes!

as per pwndbg#1889 (comment)

disconnect3d · 2024-02-25T00:45:29Z

@OBarronCS would u perhaps want to make a review of this PR? :)

disconnect3d · 2024-02-25T00:53:09Z

@dmur1 RE: non-existent stack frame - run gdb --quiet /bin/ls and then:

pwndbg> set context-sections ''
...
pwndbg> entry
...
pwndbg> stack
00:0000│ r13 rsp ret0 0x7fffffffe370 ◂— 0x1
01:0008│ ret1         0x7fffffffe378 —▸ 0x7fffffffe604 ◂— '/usr/bin/ls'
02:0010│ ret2         0x7fffffffe380 ◂— 0x0
03:0018│ rcx          0x7fffffffe388 —▸ 0x7fffffffe610 ◂— 'SHELL=/bin/bash'
04:0020│              0x7fffffffe390 —▸ 0x7fffffffe620 ◂— 'PWD=/root/pwndbg'
05:0028│              0x7fffffffe398 —▸ 0x7fffffffe631 ◂— 'NIX_PROFILES=/nix/var/nix/profiles/default /root/.nix-profile'
06:0030│              0x7fffffffe3a0 —▸ 0x7fffffffe66f ◂— 'LOGNAME=root'
07:0038│              0x7fffffffe3a8 —▸ 0x7fffffffe67c ◂— 'XDG_SESSION_TYPE=tty'

pwndbg> set $rsp=0x1234

pwndbg> stack
Exception occurred: telescope: 'NoneType' object has no attribute 'start' (<class 'AttributeError'>)
For more info invoke `set exception-verbose on` and rerun the command
or debug it by yourself with `set exception-debugger on`
pwndbg> set exception-verbose on
Set whether to print a full stacktrace for exceptions raised in Pwndbg commands to 'on'.
pwndbg> stack
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /root/pwndbg/pwndbg/commands/__init__.py:190 in __call__                                         │
│                                                                                                  │
│   187 │                                                                                          │
│   188 │   def __call__(self, *args: Any, **kwargs: Any) -> str | None:                           │
│   189 │   │   try:                                                                               │
│ ❱ 190 │   │   │   return self.function(*args, **kwargs)                                          │
│   191 │   │   except TypeError as te:                                                            │
│   192 │   │   │   print(f"{self.function.__name__.strip()!r}: {self.function.__doc__.strip()}"   │
│   193 │   │   │   pwndbg.exception.handle(self.function.__name__)                                │
│                                                                                                  │
│ /root/pwndbg/pwndbg/commands/__init__.py:346 in _OnlyWhenRunning                                 │
│                                                                                                  │
│   343 │   @functools.wraps(function)                                                             │
│   344 │   def _OnlyWhenRunning(*a: Any, **kw: Any) -> Optional[T]:                               │
│   345 │   │   if pwndbg.gdblib.proc.alive:                                                       │
│ ❱ 346 │   │   │   return function(*a, **kw)                                                      │
│   347 │   │   else:                                                                              │
│   348 │   │   │   print(f"{function.__name__}: The program is not being run.")                   │
│   349 │   │   │   return None                                                                    │
│                                                                                                  │
│ /root/pwndbg/pwndbg/commands/telescope.py:159 in telescope                                       │
│                                                                                                  │
│   156 │   for reg in pwndbg.gdblib.regs.common:                                                  │
│   157 │   │   name_values[pwndbg.gdblib.regs[reg]].append(reg)                                   │
│   158 │                                                                                          │
│ ❱ 159 │   for idx, retaddr in enumerate(pwndbg.gdblib.stack.yield_return_addresses()):           │
│   160 │   │   name_values[retaddr].append(f"ret{idx}")                                           │
│   161 │                                                                                          │
│   162 │   if not inverse:                                                                        │
│                                                                                                  │
│ /root/pwndbg/pwndbg/gdblib/stack.py:171 in yield_return_addresses                                │
│                                                                                                  │
│   168 │   │   frame = frame.older()                                                              │
│   169 │                                                                                          │
│   170 │   # Find all of them on the stack                                                        │
│ ❱ 171 │   start, stop = stack.start, stack.end                                                   │
│   172 │   while addresses and start < sp < stop:                                                 │
│   173 │   │   value = pwndbg.gdblib.memory.u(sp)                                                 │
│   174                                                                                            │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'NoneType' object has no attribute 'start'

Pls fix :)

pwndbg/commands/telescope.py

disconnect3d · 2024-02-25T00:56:13Z

pwndbg/commands/telescope.py

 for i in range(start, stop, step):
- values = list(reg_values[i])
+ values = list(name_values[i])


Isn't this a list already?

@disconnect3d i think its name_values = collections.defaultdict(lambda: [])

what do you think?

It's already a list in this case (print(type(name_values[i])) to show this). In line 180, we also iterate name_values[i+width] as a list.

pwndbg#1889 (comment)

fixes: pwndbg#1889 (comment)

dmur1 · 2024-03-01T09:39:26Z

@dmur1 RE: non-existent stack frame - run gdb --quiet /bin/ls and then:

pwndbg> set context-sections ''
...
pwndbg> entry
...
pwndbg> stack
00:0000│ r13 rsp ret0 0x7fffffffe370 ◂— 0x1
01:0008│ ret1         0x7fffffffe378 —▸ 0x7fffffffe604 ◂— '/usr/bin/ls'
02:0010│ ret2         0x7fffffffe380 ◂— 0x0
03:0018│ rcx          0x7fffffffe388 —▸ 0x7fffffffe610 ◂— 'SHELL=/bin/bash'
04:0020│              0x7fffffffe390 —▸ 0x7fffffffe620 ◂— 'PWD=/root/pwndbg'
05:0028│              0x7fffffffe398 —▸ 0x7fffffffe631 ◂— 'NIX_PROFILES=/nix/var/nix/profiles/default /root/.nix-profile'
06:0030│              0x7fffffffe3a0 —▸ 0x7fffffffe66f ◂— 'LOGNAME=root'
07:0038│              0x7fffffffe3a8 —▸ 0x7fffffffe67c ◂— 'XDG_SESSION_TYPE=tty'

pwndbg> set $rsp=0x1234

pwndbg> stack
Exception occurred: telescope: 'NoneType' object has no attribute 'start' (<class 'AttributeError'>)
For more info invoke `set exception-verbose on` and rerun the command
or debug it by yourself with `set exception-debugger on`
pwndbg> set exception-verbose on
Set whether to print a full stacktrace for exceptions raised in Pwndbg commands to 'on'.
pwndbg> stack
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /root/pwndbg/pwndbg/commands/__init__.py:190 in __call__                                         │
│                                                                                                  │
│   187 │                                                                                          │
│   188 │   def __call__(self, *args: Any, **kwargs: Any) -> str | None:                           │
│   189 │   │   try:                                                                               │
│ ❱ 190 │   │   │   return self.function(*args, **kwargs)                                          │
│   191 │   │   except TypeError as te:                                                            │
│   192 │   │   │   print(f"{self.function.__name__.strip()!r}: {self.function.__doc__.strip()}"   │
│   193 │   │   │   pwndbg.exception.handle(self.function.__name__)                                │
│                                                                                                  │
│ /root/pwndbg/pwndbg/commands/__init__.py:346 in _OnlyWhenRunning                                 │
│                                                                                                  │
│   343 │   @functools.wraps(function)                                                             │
│   344 │   def _OnlyWhenRunning(*a: Any, **kw: Any) -> Optional[T]:                               │
│   345 │   │   if pwndbg.gdblib.proc.alive:                                                       │
│ ❱ 346 │   │   │   return function(*a, **kw)                                                      │
│   347 │   │   else:                                                                              │
│   348 │   │   │   print(f"{function.__name__}: The program is not being run.")                   │
│   349 │   │   │   return None                                                                    │
│                                                                                                  │
│ /root/pwndbg/pwndbg/commands/telescope.py:159 in telescope                                       │
│                                                                                                  │
│   156 │   for reg in pwndbg.gdblib.regs.common:                                                  │
│   157 │   │   name_values[pwndbg.gdblib.regs[reg]].append(reg)                                   │
│   158 │                                                                                          │
│ ❱ 159 │   for idx, retaddr in enumerate(pwndbg.gdblib.stack.yield_return_addresses()):           │
│   160 │   │   name_values[retaddr].append(f"ret{idx}")                                           │
│   161 │                                                                                          │
│   162 │   if not inverse:                                                                        │
│                                                                                                  │
│ /root/pwndbg/pwndbg/gdblib/stack.py:171 in yield_return_addresses                                │
│                                                                                                  │
│   168 │   │   frame = frame.older()                                                              │
│   169 │                                                                                          │
│   170 │   # Find all of them on the stack                                                        │
│ ❱ 171 │   start, stop = stack.start, stack.end                                                   │
│   172 │   while addresses and start < sp < stop:                                                 │
│   173 │   │   value = pwndbg.gdblib.memory.u(sp)                                                 │
│   174                                                                                            │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'NoneType' object has no attribute 'start'

Pls fix :)

fixed :)

OBarronCS · 2024-03-07T07:11:45Z

pwndbg/commands/telescope.py

 for i in range(start, stop, step):
- values = list(reg_values[i])
+ values = list(name_values[i])


It's already a list in this case (print(type(name_values[i])) to show this). In line 180, we also iterate name_values[i+width] as a list.

OBarronCS · 2024-03-07T07:15:45Z

pwndbg/commands/telescope.py

+ name_values = collections.defaultdict(lambda: [])
+ for reg in regs.common:


Perhaps we add a type + comment here:

# Map of address to string (register name, retX) : DefaultDict[int, list[str]]

I had to look ahead and read the rest of the code to see what the types/structure of the data here was, a type would add some readability/save time for future contributors.

OBarronCS · 2024-03-07T07:15:54Z

pwndbg/commands/telescope.py

- # Find all registers which show up in the trace, map address to regs
- regs: dict[int, str] = {}
+ # Find all names which show up in the trace
+ names = {}


names: dict[int, str] = {}

OBarronCS · 2024-03-07T07:19:28Z

pwndbg/commands/telescope.py

 (
- regs_or_frame_offset(addr, bp, regs, longest_regs),


This is no longer called - frame offset isn't being printed.

OBarronCS · 2024-03-07T07:49:35Z

pwndbg/commands/telescope.py

+ separator,
+ )
+ ),
+ T.register(names[addr].ljust(longest_names)),


There are a couple changes that were discarded during the rebase that we should continue to have:

To bring back both stackframe base offset printing and looks like some logic for reverse printing (#1978), could replace whole section with:

if inverse: line_offset = addr - (stop + ptrsize) + (telescope.offset * ptrsize) idx_offset = int((start - stop - ptrsize) / ptrsize) - (i + telescope.offset) else: line_offset = addr - start + (telescope.offset * ptrsize) idx_offset = i + telescope.offset line = T.offset( "%02x%s%04x%s" % ( idx_offset, delimiter, line_offset, separator, ) ) + " ".join( ( regs_or_frame_offset(addr, bp, names, longest_names), pwndbg.chain.format(addr), ) )

gsingh93 · 2024-03-07T16:43:22Z

@dmur1 could you rebase this?

disconnect3d · 2024-04-28T10:48:05Z

@dmur1 ping about rebase/conflicts

disconnect3d reviewed Aug 27, 2023

View reviewed changes

pwndbg/gdblib/stack.py Outdated Show resolved Hide resolved

smiley added 3 commits February 22, 2024 15:32

add yield_return_addresses to gdblib/stack.py

163cacf

rewrite retaddr in terms of yield_return_addresses

d7b9467

make telescope print return addresses on the stack

eeabdbf

dmur1 force-pushed the telescope-display-ret-addr-#1071 branch from d810007 to eeabdbf Compare February 22, 2024 15:37

dmur1 marked this pull request as draft February 22, 2024 15:39

dmur1 added 2 commits February 22, 2024 15:55

fix extra import for the linter

b7ad885

better way to compute start and stop

5d5eac9

as per pwndbg#1889 (comment)

dmur1 marked this pull request as ready for review February 22, 2024 18:07

disconnect3d reviewed Feb 25, 2024

View reviewed changes

pwndbg/commands/telescope.py Outdated Show resolved Hide resolved

disconnect3d reviewed Feb 25, 2024

View reviewed changes

dmur1 added 2 commits March 1, 2024 09:34

fix issue where vmmap.find(sp) would return None

eaebef2

pwndbg#1889 (comment)

cache regs = pwndbg.gdblib.regs above and use regs

ad14944

fixes: pwndbg#1889 (comment)

OBarronCS reviewed Mar 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Telescope display ret addr #1071 #1889

Telescope display ret addr #1071 #1889

dmur1 commented Aug 25, 2023

codecov bot commented Aug 25, 2023 •

edited

disconnect3d Aug 27, 2023

dmur1 Feb 22, 2024

disconnect3d commented Aug 27, 2023

disconnect3d commented Oct 7, 2023

OBarronCS commented Dec 1, 2023

disconnect3d commented Jan 10, 2024

dmur1 commented Feb 22, 2024

disconnect3d commented Feb 25, 2024

disconnect3d commented Feb 25, 2024 •

edited

disconnect3d Feb 25, 2024

dmur1 Mar 1, 2024

OBarronCS Mar 7, 2024

dmur1 commented Mar 1, 2024

OBarronCS Mar 7, 2024

OBarronCS Mar 7, 2024

OBarronCS Mar 7, 2024

OBarronCS Mar 7, 2024

OBarronCS Mar 7, 2024

gsingh93 commented Mar 7, 2024

disconnect3d commented Apr 28, 2024

		name_values = collections.defaultdict(lambda: [])
		for reg in regs.common:

Telescope display ret addr #1071 #1889

Are you sure you want to change the base?

Telescope display ret addr #1071 #1889

Conversation

dmur1 commented Aug 25, 2023

codecov bot commented Aug 25, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

disconnect3d commented Aug 27, 2023

disconnect3d commented Oct 7, 2023

OBarronCS commented Dec 1, 2023

disconnect3d commented Jan 10, 2024

dmur1 commented Feb 22, 2024

disconnect3d commented Feb 25, 2024

disconnect3d commented Feb 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmur1 commented Mar 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsingh93 commented Mar 7, 2024

disconnect3d commented Apr 28, 2024

codecov bot commented Aug 25, 2023 •

edited

disconnect3d commented Feb 25, 2024 •

edited