Don't error out if disassembling previous instructions fails (#931)

* minor linting stuff

* fix #922 + add regression test

* [ci] change the coverage change tolerance to 1%

.github/workflows/run-tests.yml

@@ -77,12 +77,12 @@ jobs:
     - name: Run test coverage
       if: matrix.os == 'ubuntu-22.04'
       env:
-        ALLOWED_MARGIN: 0.05
+        ALLOWED_MARGIN: 0.01
         MIN_COVERAGE: 70
       run: |
         current_score=$(curl --silent https://hugsy.github.io/gef/coverage/gef_py.html | grep pc_cov | sed 's?.*<span class="pc_cov">\([^%]*\)%</span>?\1?g')
         bash scripts/generate-coverage-docs.sh
         new_score=$(cat docs/coverage/gef_py.html | grep pc_cov | sed 's?.*<span class="pc_cov">\([^%]*\)%</span>?\1?g')
         echo "New coverage score: ${new_score}% (current ${current_score}%)"
         python${{ env.PY_VER }} -c "( ${new_score} < ${{ env.MIN_COVERAGE}} ) and exit(1)"
-        python${{ env.PY_VER }} -c "( ${new_score} < ( ${current_score} - ${{ env.ALLOWED_MARGIN}} ) ) and exit(2)"
+        python${{ env.PY_VER }} -c "(( abs( ${current_score}-${new_score}) / ${current_score} ) >= ${{ env.ALLOWED_MARGIN}}) and exit(2)"

gef.py

@@ -65,7 +65,6 @@
 import importlib.util
 import inspect
 import itertools
-import json
 import os
 import pathlib
 import platform
@@ -1977,7 +1976,7 @@ def gdb_get_location_from_symbol(address: int) -> Optional[Tuple[str, int]]:
     Return a tuple with the name and offset if found, None otherwise."""
     # this is horrible, ugly hack and shitty perf...
     # find a *clean* way to get gdb.Location from an address
-    sym = gdb.execute(f"info symbol {address:#x}", to_string=True)
+    sym = str(gdb.execute(f"info symbol {address:#x}", to_string=True))
     if sym.startswith("No symbol matches"):
         return None
 
@@ -1990,8 +1989,10 @@ def gdb_get_location_from_symbol(address: int) -> Optional[Tuple[str, int]]:
 
 
 def gdb_disassemble(start_pc: int, **kwargs: int) -> Generator[Instruction, None, None]:
-    """Disassemble instructions from `start_pc` (Integer). Accepts the following named parameters:
-    - `end_pc` (Integer) only instructions whose start address fall in the interval from start_pc to end_pc are returned.
+    """Disassemble instructions from `start_pc` (Integer). Accepts the following named
+    parameters:
+    - `end_pc` (Integer) only instructions whose start address fall in the interval from
+      start_pc to end_pc are returned.
     - `count` (Integer) list at most this many disassembled instructions
     If `end_pc` and `count` are not provided, the function will behave as if `count=1`.
     Return an iterator of Instruction objects
@@ -2091,18 +2092,20 @@ def gef_disassemble(addr: int, nb_insn: int, nb_prev: int = 0) -> Generator[Inst
     nb_insn = max(1, nb_insn)
 
     if nb_prev:
-        start_addr = gdb_get_nth_previous_instruction_address(addr, nb_prev)
-        if start_addr:
-            for insn in gdb_disassemble(start_addr, count=nb_prev):
-                if insn.address == addr: break
-                yield insn
+        try:
+            start_addr = gdb_get_nth_previous_instruction_address(addr, nb_prev)
+            if start_addr:
+                    for insn in gdb_disassemble(start_addr, count=nb_prev):
+                        if insn.address == addr: break
+                        yield insn
+        except gdb.MemoryError:
+            # If the address pointing to the previous instruction(s) is not mapped, simply skip them
+            pass
 
     for insn in gdb_disassemble(addr, count=nb_insn):
         yield insn
 
 
-
-
 def gef_execute_external(command: Sequence[str], as_list: bool = False, **kwargs: Any) -> Union[str, List[str]]:
     """Execute an external command and return the result."""
     res = subprocess.check_output(command, stderr=subprocess.STDOUT, shell=kwargs.get("shell", False))
@@ -4670,6 +4673,8 @@ def do_invoke(self, _: List[str], **kwargs: Any) -> None:
                 value = struct.unpack(fmt, gef.memory.read(addr, size))[0]
                 data += [value]
             sdata = ", ".join(map(hex, data))
+        else:
+            sdata = ""
 
         if args.lang == "bytearray":
             data = gef.memory.read(start_addr, args.length)
@@ -8313,7 +8318,7 @@ def do_invoke(self, argv: List[str]) -> None:
         argc = len(argv)
 
         if argc == 0:
-            ret = gdb.execute("show disable-randomization", to_string=True)
+            ret = gdb.execute("show disable-randomization", to_string=True) or ""
             i = ret.find("virtual address space is ")
             if i < 0:
                 return

tests/api/gef_disasemble.py

@@ -0,0 +1,30 @@
+"""
+`gef.heap` test module.
+"""
+
+import pytest
+
+from tests.utils import ARCH, _target, gdb_run_silent_cmd
+from tests.utils import GefUnitTestGeneric
+
+
+class GefDisassembleApiFunction(GefUnitTestGeneric):
+    """`gef_disassemble` function test module."""
+
+    @pytest.mark.skipif(ARCH not in ("x86_64", "i686"), reason=f"Skipped for {ARCH}")
+    def test_func_gef_disassemble(self):
+        cmd = "gef_disassemble(0x2337100, 4, 4)"
+        res = gdb_run_silent_cmd(f"pi os.linesep.join([str(i) for i in {cmd}])", target=_target("mmap-known-address"))
+        self.assertNoException(res)
+        self.assertIn(
+            ' 0x23370fc                  int3   \\n 0x23370fd                  int3   \\n 0x23370fe                  int3   \\n 0x23370ff                  int3   \\n 0x2337100                  int3   \\n 0x2337101                  int3   \\n 0x2337102                  int3   \\n 0x2337103                  int3   ', res)
+
+    @pytest.mark.skipif(ARCH not in ("x86_64", "i686"), reason=f"Skipped for {ARCH}")
+    def test_func_gef_disassemble_page_border(self):
+        # Regression test for issue #922
+        cmd = "gef_disassemble(0x2337000, 4, 4)"
+        res = gdb_run_silent_cmd(
+            f"pi os.linesep.join([str(i) for i in {cmd}])", target=_target("mmap-known-address"))
+        self.assertNoException(res)
+        self.assertIn(
+            '0x2337000                  int3   \\n 0x2337001                  int3   \\n 0x2337002                  int3   \\n 0x2337003                  int3   ', res)

tests/binaries/mmap-known-address.c

@@ -13,21 +13,37 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <sys/mman.h>
+#include <string.h>
 
 #include "utils.h"
 
 int main(int argc, char **argv, char **envp)
 {
-    void *p = mmap((void *)0x1337000,
-                   getpagesize(),
-                   PROT_READ | PROT_WRITE,
-                   MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED,
-                   -1,
-                   0);
-
-    if (p == (void *)-1)
+    const size_t pgsz = getpagesize();
+
+    void *a1 = mmap((void *)0x1337000,
+            pgsz,
+            PROT_READ | PROT_WRITE,
+            MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED,
+            -1,
+            0);
+
+    if (a1 == (void *)-1)
+        return EXIT_FAILURE;
+
+    memset(a1, 0x41, pgsz);
+
+    void *a2 = mmap((void *)0x2337000,
+                    pgsz,
+                    PROT_READ | PROT_WRITE | PROT_EXEC,
+                    MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED,
+                    -1,
+                    0);
+    if (a2 == (void *)-1)
         return EXIT_FAILURE;
 
+    memset(a2, 0xcc, pgsz);
+
     DebugBreak();
 
     return EXIT_SUCCESS;