Fixes #1600, #752: invalid symbol.get results (#1605)

This commit adds a fix and tests for #1600 and #752.

* https://github.com/pwndbg/pwndbg/issues/1600
* https://github.com/pwndbg/pwndbg/issues/752

Generally, for an example like this:

```cpp
struct A {
    void foo(int, int) { };
};

int main() {
    A a;
    a.foo(1, 1);
}
```

The output for `info symbol <address of A::foo>` returns:

```
'A::foo(int, int) [clone.isra.0] + 3 in section .text of /root/pwndbg/tests/gdb-tests/tests/binaries/a.out\n'
```

We then used this code to parse this:

```py
    # Expected format looks like this:
    # main in section .text of /bin/bash
    # main + 3 in section .text of /bin/bash
    # system + 1 in section .text of /lib/x86_64-linux-gnu/libc.so.6
    # No symbol matches system-1.
    a, b, c, _ = result.split(maxsplit=3)

    if b == "+":
        return "%s+%s" % (a, c)
    if b == "in":
        return a

    return ""
```

The `result.split(maxsplit=3)` here splitted the string to:

```py
['A::foo(int,',
 'int)',
 '[clone.isra.0] + 3 in section .text of /root/pwndbg/tests/gdb-tests/tests/binaries/a.out\n']
```

And since `b` was not `"+"` or `"in"` we eventually returned an empty
string instead of the `A::foo(int, int)` which would be expected here.

pwndbg/gdblib/symbol.py

@@ -84,19 +84,25 @@ def get(address: int, gdb_only=False) -> str:
                 res = pwndbg.ida.Name(address) or pwndbg.ida.GetFuncOffset(address)
                 return res or ""
 
-    # Expected format looks like this:
-    # main in section .text of /bin/bash
-    # main + 3 in section .text of /bin/bash
-    # system + 1 in section .text of /lib/x86_64-linux-gnu/libc.so.6
-    # No symbol matches system-1.
-    a, b, c, _ = result.split(maxsplit=3)
-
-    if b == "+":
-        return "%s+%s" % (a, c)
-    if b == "in":
-        return a
+    # See https://github.com/bminor/binutils-gdb/blob/d1702fea87aa62dff7de465464097dba63cc8c0f/gdb/printcmd.c#L1594-L1624
+    # The most often encountered formats looks like this:
+    #   "main in section .text of /bin/bash"
+    #   "main + 3 in section .text of /bin/bash"
+    #   "system + 1 in section .text of /lib/x86_64-linux-gnu/libc.so.6"
+    #   "No symbol matches system-1"
+    # But there are some others that we have to account for as well
+    if " in section " in result:
+        loc_string, _ = result.split(" in section ")
+    elif " in load address range of " in result:
+        loc_string, _ = result.split(" in load address range of ")
+    elif " overlay section " in result:
+        result, _ = result.split(" overlay section ")
+        loc_string, _ = result.split(" in ")
+    else:
+        loc_string = ""
 
-    return ""
+    # If there is 'main + 87' we want to replace it with 'main+87' etc.
+    return loc_string.replace(" + ", "+")
 
 
 @pwndbg.lib.memoize.reset_on_objfile

tests/gdb-tests/tests/binaries/makefile

@@ -38,7 +38,7 @@ GLIBC_2_33=$(PWD)/glibcs/2.33
 
 .PHONY : all clean
 
-CUSTOM_TARGETS = reference_bin_pie.out reference_bin_nopie.out
+CUSTOM_TARGETS = reference_bin_pie.out reference_bin_nopie.out symbol_1600_and_752.out
 
 all: $(LINKED) $(LINKED_ASM) $(COMPILED_GO) $(CUSTOM_TARGETS)
 
@@ -113,7 +113,7 @@ issue_1565.out: issue_1565.c
 
 clean :
 	@echo "[+] Cleaning stuff"
-	@rm -f $(COMPILED) $(LINKED) $(COMPILED_ASM) $(LINKED_ASM) $(COMPILED_GO)
+	@rm -f $(COMPILED) $(LINKED) $(COMPILED_ASM) $(LINKED_ASM) $(COMPILED_GO) *.out *.o
 
 
 reference_bin_pie.out: reference-binary.c
@@ -123,3 +123,6 @@ reference_bin_pie.out: reference-binary.c
 reference_bin_nopie.out: reference-binary.c
 	@echo "[+] Building reference_bin_nopie.out"
 	${ZIGCC} -fno-pie -o reference_bin_nopie.out reference-binary.c
+
+symbol_1600_and_752.out: symbol_1600_and_752.cpp
+	${CXX} -O0 -ggdb -Wno-pmf-conversions symbol_1600_and_752.cpp -o symbol_1600_and_752.out

tests/gdb-tests/tests/binaries/symbol_1600_and_752.cpp

@@ -0,0 +1,23 @@
+void break_here(void* p) { }
+
+struct A {
+    __attribute__((noinline))
+    void foo(int, int) { break_here(0); }
+
+    void call_foo() { foo(1, 2); }
+};
+
+int main() {
+    break_here((void*)main);
+    break_here((void*)break_here);
+
+    // code for issue 1600
+    break_here((void*)&A::foo);
+
+    // just another check for mangled symbols
+    break_here((void*)&A::call_foo);
+
+    // code for issue 752
+    A a;
+    a.call_foo();
+}

tests/gdb-tests/tests/test_context_commands.py

@@ -9,6 +9,7 @@ import tests
 USE_FDS_BINARY = tests.binaries.get("use-fds.out")
 TABSTOP_BINARY = tests.binaries.get("tabstop.out")
 SYSCALLS_BINARY = tests.binaries.get("syscalls-x64.out")
+MANGLING_BINARY = tests.binaries.get("symbol_1600_and_752.out")
 
 
 def test_context_disasm_show_fd_filepath(start_binary):
@@ -185,3 +186,39 @@ def test_context_disasm_syscalls_args_display(start_binary):
         "   0x4000a5                add    byte ptr [rax], al\n"
         "────────────────────────────────────────────────────────────────────────────────\n"
     )
+
+
+def test_context_backtrace_show_proper_symbol_names(start_binary):
+    start_binary(MANGLING_BINARY)
+    gdb.execute("break A::foo")
+    gdb.execute("continue")
+
+    backtrace = gdb.execute("context backtrace", to_string=True).split("\n")
+
+    assert backtrace[0] == "LEGEND: STACK | HEAP | CODE | DATA | RWX | RODATA"
+    assert (
+        backtrace[1]
+        == "─────────────────────────────────[ BACKTRACE ]──────────────────────────────────"
+    )
+
+    assert re.match(r" ► f 0   0x[0-9a-f]+ A::foo\(int, int\)", backtrace[2])
+
+    # Match A::call_foo()+38 or similar: the offset may change so we match \d+ at the end
+    assert re.match(r"   f 1   0x[0-9a-f]+ A::call_foo\(\)\+\d+", backtrace[3])
+
+    # Match main+87 or similar offset
+    assert re.match(r"   f 2   0x[0-9a-f]+ main\+\d+", backtrace[4])
+
+    # Match __libc_start_main+243 or similar offset
+    # Note: on Ubuntu 22.04 there will be __libc_start_call_main and then __libc_start_main
+    # but on older distros there will be only __libc_start_main
+    # Let's not bother too much about it and make it the last call assertion here
+    assert re.match(
+        r"   f 3   0x[0-9a-f]+ (__libc_start_main|__libc_start_call_main)\+\d+", backtrace[5]
+    )
+
+    assert (
+        backtrace[-2]
+        == "────────────────────────────────────────────────────────────────────────────────"
+    )
+    assert backtrace[-1] == ""

tests/gdb-tests/tests/test_symbol.py

@@ -0,0 +1,30 @@
+import gdb
+
+import pwndbg
+import tests
+
+MANGLING_BINARY = tests.binaries.get("symbol_1600_and_752.out")
+
+
+def test_symbol_get(start_binary):
+    start_binary(MANGLING_BINARY)
+    gdb.execute("break break_here")
+
+    def get_next_ptr():
+        gdb.execute("continue")
+
+        # To fetch the value of 'p' it must be set first
+        # and it will be set by the program copying from register to the stack
+        # (we pass `to_string=True` to suppress the context output)
+        gdb.execute("nextret", to_string=True)
+        p = int(gdb.parse_and_eval("p"))
+        return pwndbg.gdblib.symbol.get(p)
+
+    assert get_next_ptr() == "main"
+
+    assert get_next_ptr() == "break_here(void*)"
+
+    # Test for the bug https://github.com/pwndbg/pwndbg/issues/1600
+    assert get_next_ptr() == "A::foo(int, int)"
+
+    assert get_next_ptr() == "A::call_foo()"