Skip to content

Simplify unnecessary return paths in relocation and loader modules - #709

Open
inquilabee wants to merge 2 commits into
angr:masterfrom
inquilabee:shipgate-refactor-return-paths
Open

Simplify unnecessary return paths in relocation and loader modules#709
inquilabee wants to merge 2 commits into
angr:masterfrom
inquilabee:shipgate-refactor-return-paths

Conversation

@inquilabee

Copy link
Copy Markdown

Summary

Repo health: 53/100

Our analysis found 2,209 format/lint issues (under ShipGate's stricter rules) and about 57 refactoring opportunities across the reviewed scope. Security scans were clean; code duplication is modest (~2.5%); cyclomatic complexity and maintainability index flag several modules; structural refactor suggestions remain in relocation and binding helpers.

This pull request is a sample of those findings — 23 files, ~129 focused line changes — and is not a full format pass or refactor cleanup.

What you are doing right

  • No secrets detected and no high-severity security patterns in scope (gitleaks, bandit, semgrep clean).
  • Code duplication is modest — ~2.5% (jscpd), just above the 2% threshold.
  • No unused functions or modules flagged by dead-code scanners in scope.
  • Your declared ruff configuration (E, F, I, TID, UP at line-length 120) already passes cleanly on the main package.

What you could improve

Refactoring (included in example PR)

  • About 57 structural refactor opportunities — unnecessary return assignments (RET504) and redundant else branches after return (RET505) across ELF, PE, Mach-O, and loader modules. This PR simplifies 98 of those patterns in 23 files.

Maintainability & complexity (follow-up)

  • P95 cyclomatic complexity (10) exceeds the configured ceiling (7); 75 functions/blocks flagged by complexity metrics.
  • Several modules have low maintainability index (rank C) — e.g. binding helpers and large backend parsers.

Dependencies (follow-up)

  • pyelftools and pyxbe are declared but reported unused by deptry (may be optional/runtime paths).

Duplication (follow-up)

  • jscpd reports ~2.5% duplicate code — slightly above the 2% threshold; repeated string literals in Mach-O binding and loader utilities.

Lint / style (follow-up)

  • Under ShipGate's bundled ruff profile (100-char lines), ~2,100 style findings remain — mostly line length (E501), pathlib migration (PTH), and naming conventions in tests. Your project ruff config intentionally uses 120-char lines, so many are policy differences rather than bugs.

Changes

  • cle/backends/elf/relocation/ppc.py — collapse unnecessary result temporaries before return (20 sites).
  • cle/backends/macho/structs.py, cle/backends/macho/symbol.py, cle/backends/macho/macho.py — RET505/RET504 simplifications in Mach-O parsing.
  • cle/backends/pe/pe.py, cle/backends/pe/relocation/generic.py — remove unnecessary relocation temporaries.
  • cle/backends/coff.py, cle/backends/backend.py, cle/backends/relocation.py — simplify return paths in COFF/PE relocation helpers.
  • cle/loader.py, cle/memory.py — streamline loader and memory serialization returns.
  • cle/backends/elf/metaelf.py, cle/backends/elf/symbol_type.py, cle/backends/elf/variable_type.py — ELF symbol/type helpers.
  • cle/backends/externs/__init__.py, cle/backends/java/soot.py, cle/backends/tls/*.py — minor return-path cleanup in extern and TLS backends.

Verification

ruff check cle/ tests/          # All checks passed
python3 -m compileall cle/ -q   # Syntax OK on changed modules

(Full uv sync + pytest requires native build of pyvex/archinfo; not run locally in this review environment.)


Review summary produced with ShipGate — a policy-first quality orchestrator that runs linters, formatters, security scanners, and refactor checks from one catalog. This PR used check + refactor check --strict to find issues; docs.

inquilabee and others added 2 commits July 29, 2026 11:55
Collapse RET504/RET505 patterns across ELF, PE, Mach-O, and loader
modules — behavior-preserving refactors from static analysis review.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant