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

Fix PE loader to recursivly import symbols and call DllMain #917

Closed
wants to merge 1 commit into from

Conversation

t-moe
Copy link

@t-moe t-moe commented Sep 16, 2021

This PR fixes some of the issues with the PE loader:

Calling DllMain is imperative, in order to initialize constructors (e.g. c++ static initializers and other library init stuff).

The idea proposed by #801 with delaying loading the DLLs until the first symbol is called, raises the question of when to call DllMain(). In this PR I load the DLLs recursively at startup, and also call DLLmain, but I skip the virtual DLLs in the hope that it is faster than #801. Also, by skip loading the virtual DLLs I hope that we can avoid copying the api-*.dlls entirely (which caused build problems in #886).

Unfortunately I was not able to run the testcases on my machine. Can you run the workflow?

Credits go to @crass (#886). I've copied his idea with the init_import method.

Checklist

Which kind of PR do you create?

  • This PR only contains minor fixes.
  • This PR contains major feature update.
  • This PR introduces a new function/api for Qiling Framework.

Coding convention?

  • The new code conforms to Qiling Framework naming convention.
  • The imports are arranged properly.
  • Essential comments are added.
  • The reference of the new code is pointed out.

Extra tests?

  • No extra tests are needed for this PR.
  • I have added enough tests for this PR.
  • Tests will be added after some discussion and review.

Changelog?

  • This PR doesn't need to update Changelog.
  • Changelog will be updated after some proper review.
  • Changelog has been updated in my PR.

Target branch?

  • The target branch is dev branch.

One last thing


@t-moe t-moe mentioned this pull request Sep 16, 2021
15 tasks
@wtdcode wtdcode deleted the branch qilingframework:old_dev October 12, 2021 08:49
@wtdcode wtdcode closed this Oct 12, 2021
@Enteee
Copy link

Enteee commented Oct 13, 2021

@wtdcode this was closed in error, same issue as #918 . Removing dev branches is probably not very good practice...

@wtdcode
Copy link
Member

wtdcode commented Oct 13, 2021

@wtdcode this was closed in error, same issue as #918 . Removing dev branches is probably not very good practice...

Sorry I really don't know removing a branch would close issues related. Wait @t-moe to create a new one.

@Enteee
Copy link

Enteee commented Oct 13, 2021

No worries. Just wanted to point it out. There might be more PRs that were closed this way.

@elicn
Copy link
Member

elicn commented Oct 14, 2021

@wtdcode, about 10 PRs were closed this way.
I thought it was intentional..

@wtdcode
Copy link
Member

wtdcode commented Oct 14, 2021

@wtdcode, about 10 PRs were closed this way. I thought it was intentional..

I would have a look, really sorry.

@elicn
Copy link
Member

elicn commented Apr 24, 2022

Changes were included as part of #1118 .
Thanks @t-moe !

@elicn elicn closed this Apr 24, 2022
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.

4 participants