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

Cleanup #385

Merged
merged 4 commits into from Mar 29, 2021
Merged

Cleanup #385

merged 4 commits into from Mar 29, 2021

Conversation

atlas0fd00m
Copy link
Contributor

an ARM analysis bugfix
an ELF cleanup fix (to simply not complain so much about certain relocs)
a Cobra Cluster set of bugfixes
approve now, before this list grows! ;)

@@ -597,7 +597,7 @@ def applyRelocs(elf, vw, addbase=False, baseaddr=0):
vw.makeImport(rlva, "*", dmglname)
vw.setComment(rlva, name)

elif rtype in (Elf.R_386_32, Elf.R_386_COPY):
elif rtype in (Elf.R_386_32, Elf.R_386_COPY, Elf.R_X86_64_TPOFF64):
Copy link
Contributor

@rakuy0 rakuy0 Mar 29, 2021

Choose a reason for hiding this comment

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

Soo, since this change prompted 0 complaints in our unittests, can you give a rundown of what this changes/fixes? Am I just over thinking it and it's just silencing some errors that don't matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just telling the ELF parser that this type of relocation is normal, and not to complain about "not handling it".

Copy link
Contributor

@rakuy0 rakuy0 left a comment

Choose a reason for hiding this comment

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

So...we have no unit tests for the cluster code. Like at all. It might be out of scope for this PR, but if you ever find yourself in that kind of mood, could you knock some simple ones out, since you seem to be a big user of it and know how it should function?

Other than that, just a small question.

@rakuy0 rakuy0 mentioned this pull request Mar 29, 2021
@atlas0fd00m
Copy link
Contributor Author

i've been giving unittests for clustering some thought. i have code that heavily uses it, but it's kinda my special sauce behind Icharus and LightningStrike. i'm contemplating all of that right now. i think it might make sense to wrap in something.

i have to admit, with the current state of Vtrace unittests causing such issues (like i roll the dice and lean toward late-night pushes to hope the unittests pass), i'm hesitant to add something that makes use of networking.

@atlas0fd00m atlas0fd00m merged commit 0fe1433 into vivisect:master Mar 29, 2021
@atlas0fd00m atlas0fd00m deleted the cleanup branch March 29, 2021 15:14
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.

None yet

2 participants