Skip to content

Handle dynamically linked ET_EXECs#112

Merged
MusicalNinjaDad merged 7 commits into
mainfrom
ET_EXEC_dynamic
Nov 19, 2025
Merged

Handle dynamically linked ET_EXECs#112
MusicalNinjaDad merged 7 commits into
mainfrom
ET_EXEC_dynamic

Conversation

@MusicalNinjaDad

@MusicalNinjaDad MusicalNinjaDad commented Nov 19, 2025

Copy link
Copy Markdown
Owner

Summary by Sourcery

Add handling for dynamically linked ET_EXEC binaries by introducing a unified DYNEXE type, updating detection logic and related constants, and updating tests and build scripts accordingly.

New Features:

  • Support dynamically linked ET_EXEC binaries by classifying them as type DYNEXE.

Enhancements:

  • Rename EXE constant to EXEC and redefine PIE as DYNEXE to represent any dynamically linked executable.
  • Update ELF type detection to return DYNEXE for ET_EXEC files with dynamic tables and for ET_DYN files marked PIE.
  • Adjust IsExe and IsLib methods to use the new EXEC bitmask for executable classification.

Build:

  • Extend the testdata build script to compile a dynamically linked hello_dynamic binary.

Tests:

  • Add hello_dynamic test case and update testdata and testpaths to cover dynamically linked executables.

Chores:

  • Bump version to 1.2.1.

@sourcery-ai

sourcery-ai Bot commented Nov 19, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR adds support for dynamically linked ET_EXEC binaries by introducing a new DYNEXE Type in the ELF logic and updating related detection, test fixtures, build scripts, and versioning.

Entity relationship diagram for new test binary entries

erDiagram
    TESTDATA {
        Name string
        Path string
        SnagTo string
        SnagAs string
        Elf Elf
        Dynamic bool
        Exe bool
        Lib bool
        HasInterpreter bool
    }
    ELF {
        Name string
        Path string
        Class EI_CLASS
        Type Type
        Interpreter string
        Dependencies string
    }
    TESTDATA ||--|| ELF : contains
    ELF }o--|| Type : has
Loading

Class diagram for updated ELF Type logic

classDiagram
    class Elf {
        +Name string
        +Path string
        +Class EI_CLASS
        +Type Type
        +Interpreter string
        +Dependencies []string
        +IsExe() bool
        +IsLib() bool
        +IsDyn() bool
    }
    class Type {
        <<enumeration>>
        UNDEF
        EXEC
        DYN
        DYNEXE
    }
    Elf --> Type
Loading

Flow diagram for ELF type detection with DYNEXE support

flowchart TD
    A["elffile.Type == ET_EXEC?"] -->|Yes| B["Has Dynamic Table?"]
    B -->|Yes| C["Type = DYNEXE"]
    B -->|No| D["Type = EXEC"]
    A -->|No| E["elffile.Type == ET_DYN?"]
    E -->|Yes| F["Has PIE flag?"]
    F -->|Yes| G["Type = DYNEXE"]
    F -->|No| H["Type = DYN"]
    E -->|No| I["Other type handling"]
Loading

File-Level Changes

Change Details Files
Introduce and use a new DYNEXE constant for combined executable/dynamic binaries
  • Rename EXE constant to EXEC and PIE constant to DYNEXE
  • Update IsExe and IsLib methods to use the EXEC bitmask
  • Enhance elftype to detect ET_EXEC with dynamic symbols as DYNEXE and replace PIE detection
  • Adjust New() interpreter validation to check for DYNEXE
elf/elf.go
Extend test data to include dynamically linked ET_EXEC test cases
  • Rename existing test entry to P_hello_pie_cgo and update its metadata
  • Add a new P_hello_dynamic entry with Dynamic/Exe/Lib/HasInterpreter flags
  • Replace elf.Type values from PIE/EXE to DYNEXE/EXEC in existing entries
internal/testing/testdata.go
Adjust test path constants to support the new dynamic test binary
  • Introduce P_hello_dynamic path constant
  • Rename P_hello_pie_cgo constant to match updated test data
  • Reorder and clean up the list of test paths
internal/testpaths.go
Update build script to generate the dynamically linked ET_EXEC binary
  • Add a go build command for hello_dynamic using external linkmode and EXE buildmode
internal/testdata/hello/build.sh
Bump project version to include new functionality
  • Update Version constant from "1.2.0" to "1.2.1"
version.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Nov 19, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 76.37%. Comparing base (2723337) to head (895f540).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
elf/elf.go 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
- Coverage   76.59%   76.37%   -0.23%     
==========================================
  Files          13       13              
  Lines         752      766      +14     
==========================================
+ Hits          576      585       +9     
- Misses        119      123       +4     
- Partials       57       58       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • Consider updating the error in New() from “(PIE without interpreter)” to a more generic message (e.g. “executable missing interpreter”) since DYNEXE can also trigger it.
  • In elftype(), you currently ignore the error from DynamicSymbols; handling that error explicitly would prevent false classification when symbol parsing fails.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider updating the error in New() from “(PIE without interpreter)” to a more generic message (e.g. “executable missing interpreter”) since DYNEXE can also trigger it.
- In elftype(), you currently ignore the error from DynamicSymbols; handling that error explicitly would prevent false classification when symbol parsing fails.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@MusicalNinjaDad MusicalNinjaDad enabled auto-merge (squash) November 19, 2025 17:55
@MusicalNinjaDad MusicalNinjaDad merged commit 573fb39 into main Nov 19, 2025
18 checks passed
@MusicalNinjaDad MusicalNinjaDad deleted the ET_EXEC_dynamic branch November 19, 2025 17:55
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