Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support getting RELR relocations from dynamic section #509

Merged
merged 1 commit into from
Oct 20, 2023
Merged

Support getting RELR relocations from dynamic section #509

merged 1 commit into from
Oct 20, 2023

Conversation

medhefgo
Copy link
Contributor

No description provided.

@sevaa
Copy link
Contributor

sevaa commented Sep 27, 2023

A binary to test that against would be nice.

@medhefgo
Copy link
Contributor Author

A binary to test that against would be nice.

test_relr.py already has one? The test is already extended to also poke the dynamic section.

@@ -182,6 +193,16 @@ def get_relocation(self, n):
self._cached_relocations = list(self.iter_relocations())
return self._cached_relocations[n]


class RelrRelocationSection(Section, RelrRelocationTable):
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather use composition than multiple inheritance, unless there's a strong reason to do the latter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It follows the same style already used by RelocationSection.

@eliben
Copy link
Owner

eliben commented Oct 16, 2023

@sevaa other than the test comment, LGTY?

@sevaa
Copy link
Contributor

sevaa commented Oct 16, 2023

Never really understood how do relocations work, but all this looks reasonable.

@eliben eliben merged commit b0d7a76 into eliben:master Oct 20, 2023
3 checks passed
@medhefgo medhefgo deleted the relr branch October 20, 2023 13:46
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