Skip to content

Fix data flow analysis for multiple text sections#220

Merged
encounter merged 9 commits into
encounter:mainfrom
stravant:multiple-text-sections
Jul 17, 2025
Merged

Fix data flow analysis for multiple text sections#220
encounter merged 9 commits into
encounter:mainfrom
stravant:multiple-text-sections

Conversation

@stravant

@stravant stravant commented Jul 6, 2025

Copy link
Copy Markdown
Contributor
  • Data flow analysis results were only keyed by the symbol (function) address. That doesn't work if there are multiple text sections. The result from the first function in one text section will stomp the result from the first function in another because both have address zero.

  • Remove the ambiguity by keying off of the section address as well.

stravant added 5 commits July 5, 2025 21:42
* Data flow analysis results were only keyed by the symbol (function)
  address. That doen't work if there are multiple text sections, the
  result from the first function in one section will stomp the result
  from the first function in another because both have address zero.

* Remove the ambiguity by keying off of the section address as well.
@stravant

stravant commented Jul 6, 2025

Copy link
Copy Markdown
Contributor Author

PR does build, not sure why clippy is failing for stuff I didn't touch.

Comment thread objdiff-core/src/obj/mod.rs Outdated
Comment thread objdiff-core/src/obj/mod.rs Outdated
symbol: &Symbol,
result: Box<dyn FlowAnalysisResult>,
) {
let key = symbol.section.unwrap_or_default() as u64 + symbol.address;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This key seems like it could easily overlap, the section indices are small numbers

@stravant stravant Jul 13, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, mixed up which was the section address vs section number.

This thing gets called a lot so I want a cheap solution, so I just multiplied by a GB. If there's a single section bigger than a GB other things will break before this.

@encounter

Copy link
Copy Markdown
Owner

Thanks!

@encounter
encounter merged commit 3385f58 into encounter:main Jul 17, 2025
24 checks passed
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.

2 participants