Skip to content

fix: add base addr to symbol offset#120

Merged
not-matthias merged 3 commits intomainfrom
cod-1356-unresolved-addresses-with-pie-benchmark-executable
Oct 1, 2025
Merged

fix: add base addr to symbol offset#120
not-matthias merged 3 commits intomainfrom
cod-1356-unresolved-addresses-with-pie-benchmark-executable

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Sep 22, 2025

Changes in this PR:

  • Merge the load bias and base address calculation of unwind_info and perf_map
    • (we previously had 2 separate implementations - it's now easier to verify and maintain)
  • Add the load bias to the symbol offset
  • Add test to for the_algorithms to ensure we get the right addresses

@not-matthias not-matthias marked this pull request as ready for review September 22, 2025 08:54
@not-matthias not-matthias force-pushed the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch from db1cf59 to a502bc9 Compare September 22, 2025 08:54
@not-matthias not-matthias requested review from GuillaumeLagrange, art049 and Copilot and removed request for art049 September 22, 2025 08:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes symbol address calculation by adding the base address to symbol offsets instead of the load bias, and consolidates the address calculation logic between unwind_info and perf_map.

  • Merged load bias and base address calculation into a shared elf_helper module
  • Changed symbol offset calculation to use base address rather than load bias for better accuracy
  • Added test coverage for the_algorithms binary to verify correct address calculations

Reviewed Changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/run/runner/wall_time/perf/elf_helper.rs New shared module containing consolidated address calculation logic
src/run/runner/wall_time/perf/unwind_data.rs Refactored to use shared helper and updated function signatures
src/run/runner/wall_time/perf/perf_map.rs Updated to use shared helper and improved symbol address calculation
src/run/runner/wall_time/perf/mod.rs Added new elf_helper module and updated function call
testdata/perf_map/the_algorithms.bin Added new test binary
.gitattributes Added LFS tracking for new test binary
snapshots/* Updated test snapshots reflecting the address calculation changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@not-matthias not-matthias force-pushed the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch from a502bc9 to 085ae48 Compare September 26, 2025 11:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@not-matthias not-matthias force-pushed the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch from 085ae48 to 2cfe850 Compare September 26, 2025 13:09
@not-matthias not-matthias force-pushed the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch 3 times, most recently from 7e28e1a to 04eaff7 Compare September 29, 2025 12:59
@art049 art049 requested review from GuillaumeLagrange and removed request for GuillaumeLagrange September 30, 2025 11:47
Copy link
Contributor

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

olgtm

@not-matthias not-matthias force-pushed the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch 4 times, most recently from ab5af05 to d03f8bd Compare September 30, 2025 15:55
@not-matthias not-matthias force-pushed the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch from e7adb6f to 2ee3731 Compare September 30, 2025 16:08
@art049 art049 removed the request for review from GuillaumeLagrange October 1, 2025 08:39
@not-matthias not-matthias merged commit 2ee3731 into main Oct 1, 2025
9 checks passed
@not-matthias not-matthias deleted the cod-1356-unresolved-addresses-with-pie-benchmark-executable branch October 1, 2025 09:15
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.

3 participants

Comments