Bugfix/pointer dereference bug (#57)

* LLEF Bugfixes

Types.py line 277: bug in ptr_size where width needed to be in bits, not bytes.

Change GoTypePointer class so that it no longer dereferences the addr parameter, which best I can tell is never given indirectly in an eface.

Pointer dereferencing behaviour was depended upon by GoDataInterface, which was amended to do its own dereference where required.

Also added a new type representation for a nil interface, which is where you have a line of code in Go such as:

var myVar any = nil

* New Black version wants new tuple unpacking syntax

Author: f0alex

commands/golang.py

@@ -119,7 +119,7 @@ def __call__(
                     address = int(address_or_name, 0)
                     function_mapping = go_find_func(address)
                     if function_mapping is not None:
-                        (entry, gofunc) = function_mapping
+                        entry, gofunc = function_mapping
                         output_line(f"{hex(entry)} - {gofunc.name} (file address = {hex(gofunc.file_addr)})")
                     else:
                         print_message(MSG_TYPE.ERROR, f"Could not find function containing address {hex(address)}")

common/golang/analysis.py

@@ -131,7 +131,7 @@ def go_get_function_from_pc(pc: pointer, function_start: pointer, function_name:
     """
     record = go_find_func(pc)
     if record is not None:
-        (entry, gofunc) = record
+        entry, gofunc = record
         return (entry, gofunc.name)
     return (function_start, function_name)
 
@@ -335,7 +335,7 @@ def go_annotate_pointer_line(
                         LLEFState.go_state.type_guesses.add(object_ptr, referenced_type_struct)
                         unpacked_data = attempt_object_unpack(proc, object_ptr, settings, col_settings)
                         if unpacked_data:
-                            (_, next_pointer_unpacked) = unpacked_data
+                            _, next_pointer_unpacked = unpacked_data
 
                 # Markup line with identified Go data type.
                 go_type_name = color_string(referenced_type_struct.header.name, col_settings.go_type_color)
@@ -351,7 +351,7 @@ def go_annotate_pointer_line(
                 unpacked_data = attempt_object_unpack(proc, pointer_to_annotate, settings, col_settings)
                 object_at_pointer = None
                 if unpacked_data:
-                    (resolved_type, object_at_pointer) = unpacked_data
+                    resolved_type, object_at_pointer = unpacked_data
                 if object_at_pointer:
                     line += f" {GLYPHS.RIGHT_ARROW.value} {resolved_type} {object_at_pointer}"
 
@@ -420,7 +420,7 @@ def go_stop_hook(exe_ctx: SBExecutionContext, arch: BaseArch, settings: LLEFSett
         return
 
     arg_registers = get_arg_registers(arch)
-    (go_min_version, _) = LLEFState.go_state.pclntab_info.version_bounds
+    go_min_version, _ = LLEFState.go_state.pclntab_info.version_bounds
     if go_min_version >= 17:
         # HOOK TASK 1:
         # Register-straddling interface/string guessing. Needs register-based calling convention (Go >= 1.17).
@@ -452,7 +452,7 @@ def go_stop_hook(exe_ctx: SBExecutionContext, arch: BaseArch, settings: LLEFSett
         pc = frame.GetPC()
         record = go_find_func(pc)
         if record is not None and LLEFState.go_state.moduledata_info is not None:
-            (entry, gofunc) = record
+            entry, gofunc = record
             if entry != LLEFState.go_state.prev_func:
                 LLEFState.go_state.prev_func = entry
                 # Either this function was just called, or we stopped in the middle of it.

common/golang/data.py

@@ -239,3 +239,13 @@ class GoDataPointer(GoData):
 
     def __str__(self) -> str:
         return hex(self.address)
+
+
+@dataclass(frozen=True)
+class GoDataNilInterface(GoData):
+    """
+    An interface set to nil.
+    """
+
+    def __str__(self) -> str:
+        return "<nil interface>"

common/golang/moduledata_parser.py

@@ -55,7 +55,7 @@ def get_name(self, type_section: bytes, name_offset: int, header: TypeHeader) ->
         # Check that pointer + offset doesn't exceed the end pointer for the types section.
         if self.types + name_offset < self.etypes:
             # Module data layout depends on the Go version.
-            (go_min_version, go_max_version) = LLEFState.go_state.pclntab_info.version_bounds
+            go_min_version, go_max_version = LLEFState.go_state.pclntab_info.version_bounds
             if go_min_version >= 17:
                 length, name_offset = read_varint(type_section, name_offset)
                 if self.types + name_offset + length <= self.etypes:
@@ -166,7 +166,7 @@ def parse(self, proc: SBProcess, data: SBData, target: SBTarget) -> Union[Module
 
         offsets = None
 
-        (min_go, max_go) = LLEFState.go_state.pclntab_info.version_bounds
+        min_go, max_go = LLEFState.go_state.pclntab_info.version_bounds
         if min_go == 7 and max_go == 7:
             offsets = GO_MD_7_ONLY
         if min_go >= 8 and max_go <= 15:

common/golang/static.py

@@ -30,7 +30,7 @@ def parse_pclntab(proc: SBProcess, target: SBTarget, buf: SBData, file_addr: int
     err = SBError()
     first8bytes = buf.ReadRawData(err, 0, 8)
     if err.Success() and first8bytes is not None:
-        (magic, pad, min_instr_size, ptr_size) = struct.unpack("<IHBB", first8bytes)
+        magic, pad, min_instr_size, ptr_size = struct.unpack("<IHBB", first8bytes)
 
         parser = PCLnTabParser(file_addr, magic, pad, min_instr_size, ptr_size)
 

common/golang/type_getter.py

@@ -226,7 +226,7 @@ def __map_to_type(self, map_repr: str) -> Union[GoTypeMap, None]:
                 resolved.key_type = key_type
                 resolved.child_type = val_type
 
-                (go_min_version, _) = self.__version
+                go_min_version, _ = self.__version
                 if go_min_version < 24:
                     # Old map type.
                     bucket_str = (

common/golang/types.py

@@ -27,6 +27,7 @@
     GoDataFloat,
     GoDataInteger,
     GoDataMap,
+    GoDataNilInterface,
     GoDataPointer,
     GoDataSlice,
     GoDataString,
@@ -273,7 +274,7 @@ def get_underlying_type(self, depth: int) -> str:
     def extract_at(self, info: ExtractInfo, addr: pointer, dereferenced_pointers: set[pointer], depth: int) -> GoData:
         val = safe_read_unsigned(info, addr, info.ptr_size)
         if val is not None:
-            sign_bit = 1 << (info.ptr_size - 1)
+            sign_bit = 1 << ((info.ptr_size * 8) - 1)
             # convert unsigned to signed
             val -= (val & sign_bit) << 1
             return GoDataInteger(heuristic=Confidence.CERTAIN.to_float(), value=val)
@@ -454,7 +455,7 @@ class GoTypeArray(GoType):
     length: int
 
     def populate(self, type_section: bytes, offset: int, info: PopulateInfo) -> Union[list[int], None]:
-        (sub_elem, sup_slice, this_len) = struct.unpack_from("<" + info.ptr_specifier * 3, type_section, offset)
+        sub_elem, sup_slice, this_len = struct.unpack_from("<" + info.ptr_specifier * 3, type_section, offset)
         self.child_addr = sub_elem
         self.length = this_len
         return [sub_elem, sup_slice]
@@ -516,7 +517,7 @@ class GoTypeChan(GoType):
     direction: int
 
     def populate(self, type_section: bytes, offset: int, info: PopulateInfo) -> Union[list[int], None]:
-        (sub_elem, direction) = struct.unpack_from("<" + info.ptr_specifier * 2, type_section, offset)
+        sub_elem, direction = struct.unpack_from("<" + info.ptr_specifier * 2, type_section, offset)
         self.child_addr = sub_elem
         self.direction = direction
         return [sub_elem]
@@ -552,7 +553,7 @@ class GoTypeFunc(GoType):
 
     def populate(self, type_section: bytes, offset: int, info: PopulateInfo) -> Union[list[int], None]:
         # the size of param count fields and the uncommon offset addition is NOT pointer size dependent.
-        (num_param_in, num_param_out) = struct.unpack_from("<HH", type_section, offset)
+        num_param_in, num_param_out = struct.unpack_from("<HH", type_section, offset)
 
         # We consumed 32 bits. On 32-bit, read the next byte: on 64-bit, need 32 bits of padding.
         offset += info.ptr_size
@@ -625,16 +626,16 @@ class GoTypeInterface(GoType):
 
     def populate(self, type_section: bytes, offset: int, info: PopulateInfo) -> Union[list[pointer], None]:
         self.methods = []
-        (go_min_version, _) = self.version
-        (methods_base, methods_len) = struct.unpack_from(
+        go_min_version, _ = self.version
+        methods_base, methods_len = struct.unpack_from(
             "<" + info.ptr_specifier * 2, type_section, offset + info.ptr_size
         )
         # Each method structure in the methods table is a struct of two 32-bit integers.
         for i in range(methods_len):
             imethod_ptr = methods_base + i * 8
             if info.types <= imethod_ptr < info.etypes:
                 imethod_offset = imethod_ptr - info.types
-                (name_off, type_off) = struct.unpack_from("<II", type_section, imethod_offset)
+                name_off, type_off = struct.unpack_from("<II", type_section, imethod_offset)
 
                 name_off += 1
                 if go_min_version <= 16:
@@ -692,12 +693,21 @@ def extract_at(self, info: ExtractInfo, addr: pointer, dereferenced_pointers: se
                 fail_nicely = True
                 type_ptr = safe_read_unsigned(info, itab_ptr + info.ptr_size, info.ptr_size)
 
-        # Treat this like dereferencing a typed pointer.
         if type_ptr is not None:
-            header = TypeHeader()
-            extractor = GoTypePointer(header=header, version=self.version)
-            extractor.child_type = info.type_structs.get(type_ptr)
-            extracted = extractor.extract_at(info, addr + info.ptr_size, dereferenced_pointers, depth)
+            # Treat this like dereferencing a typed pointer.
+            if type_ptr > 0:
+                interface_type = info.type_structs.get(type_ptr)
+                if interface_type is not None:
+                    data_pointer = safe_read_unsigned(info, addr + info.ptr_size, info.ptr_size)
+                    if data_pointer is not None:
+                        extracted = interface_type.extract_at(info, data_pointer, dereferenced_pointers, depth)
+                    else:
+                        extracted = GoDataBad(heuristic=Confidence.JUNK.to_float())
+                else:
+                    extracted = GoDataBad(heuristic=Confidence.JUNK.to_float())
+            else:
+                # Nil interface
+                extracted = GoDataNilInterface(heuristic=Confidence.HIGH.to_float())
 
         if extracted is None:
             if fail_nicely:
@@ -715,7 +725,7 @@ class GoTypeMap(GoType):
     bucket_type: Union[GoType, None]
 
     def populate(self, type_section: bytes, offset: int, info: PopulateInfo) -> Union[list[pointer], None]:
-        (self.key_addr, self.child_addr, self.bucket_addr) = struct.unpack_from(
+        self.key_addr, self.child_addr, self.bucket_addr = struct.unpack_from(
             "<" + info.ptr_specifier * 3, type_section, offset
         )
         self.key_type = None
@@ -740,7 +750,7 @@ def extract_at(self, info: ExtractInfo, addr: pointer, dereferenced_pointers: se
         extracted: Union[GoData, None] = None
 
         if self.bucket_type is not None:
-            (go_min_version, _) = self.version
+            go_min_version, _ = self.version
             # Minimum version is only lifted to 24 if we are sure the new map implementation is being used
             # (the programmer can choose to use the old version even in 1.24).
             parser: Union[SwissMapParser, NoSwissMapParser]
@@ -778,23 +788,22 @@ def extract_at(self, info: ExtractInfo, addr: pointer, dereferenced_pointers: se
         extracted: Union[GoData, None] = None
 
         if self.child_type:
-            child_ptr = safe_read_unsigned(info, addr, info.ptr_size)
-            if child_ptr is not None:
-                if child_ptr > 0:
-                    if child_ptr not in dereferenced_pointers:
-                        dereferenced_pointers.add(child_ptr)
+            if addr is not None:
+                if addr > 0:
+                    if addr not in dereferenced_pointers:
+                        dereferenced_pointers.add(addr)
                         # changes to dereferenced_pointers are reflected everywhere, so we'll never dereference again
                         # in this extraction.
                         # this is good because we can reduce duplication of displayed information.
-                        dereferenced = self.child_type.extract_at(info, child_ptr, dereferenced_pointers, depth)
+                        dereferenced = self.child_type.extract_at(info, addr, dereferenced_pointers, depth)
                         if not isinstance(dereferenced, GoDataBad):
                             extracted = dereferenced
                         else:
                             # Then this pointer is not of this type - either memory does not exist, or data is illegal.
-                            extracted = GoDataPointer(heuristic=Confidence.JUNK.to_float(), address=child_ptr)
+                            extracted = GoDataPointer(heuristic=Confidence.JUNK.to_float(), address=addr)
                     else:
                         # Circular references. Slightly downgrade confidence.
-                        extracted = GoDataUnparsed(heuristic=Confidence.HIGH.to_float(), address=child_ptr)
+                        extracted = GoDataUnparsed(heuristic=Confidence.HIGH.to_float(), address=addr)
                 else:
                     # A valid, but null, pointer. Of course these come up - but downgrade the confidence.
                     extracted = GoDataPointer(heuristic=Confidence.MEDIUM.to_float(), address=0)
@@ -956,16 +965,16 @@ class GoTypeStruct(GoType):
     fields: list[GoTypeStructField]
 
     def populate(self, type_section: bytes, offset: int, info: PopulateInfo) -> Union[list[pointer], None]:
-        (go_min_version, go_max_version) = self.version
-        (_, fields_addr, fields_len) = struct.unpack_from("<" + info.ptr_specifier * 3, type_section, offset)
+        go_min_version, go_max_version = self.version
+        _, fields_addr, fields_len = struct.unpack_from("<" + info.ptr_specifier * 3, type_section, offset)
         self.fields = []
 
         for i in range(fields_len):
             # Each field struct is 3 pointers wide.
             addr = fields_addr + i * 3 * info.ptr_size
             if info.types <= addr < info.etypes:
                 field_struct_offset = addr - info.types
-                (field_name_ptr, field_type, field_offset) = struct.unpack_from(
+                field_name_ptr, field_type, field_offset = struct.unpack_from(
                     "<" + info.ptr_specifier * 3, type_section, field_struct_offset
                 )
 
@@ -1053,9 +1062,8 @@ def get_underlying_type(self, depth: int) -> str:
         return "unsafe.Pointer"
 
     def extract_at(self, info: ExtractInfo, addr: pointer, dereferenced_pointers: set[pointer], depth: int) -> GoData:
-        child_ptr = safe_read_unsigned(info, addr, info.ptr_size)
-        if child_ptr is not None:
-            return GoDataPointer(heuristic=Confidence.CERTAIN.to_float(), address=child_ptr)
+        if addr is not None:
+            return GoDataPointer(heuristic=Confidence.CERTAIN.to_float(), address=addr)
         return GoDataBad(heuristic=Confidence.JUNK.to_float())
 
 
@@ -1376,7 +1384,7 @@ def parse(self, addr: pointer, nest_depth: int) -> GoData:
                                     entries = None
                                     break
                                 # The next line is well-typed because if entries is None, then we already broke.
-                                entries.extend(from_table)  # type:ignore[union-attr]
+                                entries.extend(from_table)  # type: ignore[union-attr]
 
                     if entries is not None and len(entries) > 0:
                         # A valid map.

common/golang/util.py

@@ -108,7 +108,7 @@ def go_find_func_name_offset(pc: int) -> tuple[str, int]:
     """
     record = go_find_func(pc)
     if record is not None:
-        (entry, gofunc) = record
+        entry, gofunc = record
         return (gofunc.name, pc - entry)
 
     # otherwise, gracefully fail for display purposes