8000 llvm.global_ctors/dtors by tandf · Pull Request #774 · SVF-tools/SVF · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

llvm.global_ctors/dtors #774

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 22, 2022
Merged

llvm.global_ctors/dtors #774

merged 6 commits into from
Jul 22, 2022

Conversation

tandf
Copy link
Contributor
@tandf tandf commented Jul 15, 2022

Sort llvm_global_ctors functions according to their priority. Also support destroy functions. See LLVM Language Reference Manual

@tandf tandf mentioned this pull request Jul 15, 2022
Closed
@tandf tandf mentioned this pull request Jul 15, 2022
Closed
@yuleisui
Copy link
Collaborator

Can this patch pass your tests in #760 now?

@tandf
Copy link
Contributor Author
tandf commented Jul 19, 2022

Yes, it can pass my test now.

@yuleisui
Copy link
Collaborator

Can you indicate which option you have used to pass your test, I assume that -svf-main is also turned on?

Do you think this option should always be turned on if analyzing cpp? or any code could be changed to call addSVFMain if we have cpp constructors?

@tandf
Copy link
Contributor Author
tandf commented Jul 20, 2022

I use -model-array -model-consts -svf-main to run the test. I also merge this pull request to my own analysis, and ran on multiple program that I collected from the wild, and I didn't find anything problem that I'm sure is caused by this change. Nevertheless, it'll be great if you can help me review and test it.

As for your question that whether this option should always be turned on, I think it should. My understanding of -svf-main is that, it creates a new program entry that contains init functions (and destroy functions) created by llvm but are not explicitly called. If my understanding is correct, I think it's necessary to enable this option by default, since it reflects the semantics of the ir. Also, as I mentioned, I tested on multiple cpp IR files collected from the wild with -svf-main, and the results seem to be correct so far.

@yuleisui
Copy link
Collaborator

Can you try to turn off -model-array -model-consts and only enable -svf-main? It should also pass your test.

@tandf
Copy link
Contributor Author
tandf commented Jul 20, 2022

Yes it works with only -svf-main.

For the test, I run with svf-ex. I run on out.ll as attached in #775. I manually added the following content to test destroy functions. After disassembling the out.svf.bc file I can see the correct @svf.main() function injected

@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 6234, void ()* @test1, i8* null }, { i32, void ()*, i8* } { i32 3213, void ()* @test2, i8* null }]
define internal void @test1() #0 section ".text.startup" {
  ret void
}
define internal void @test2() #0 section ".text.startup" {
  ret void
}

@yuleisui
Copy link
Collaborator

This pr is important. To make it easier to maintain and extensible later, would you be able to add some comments for the code? Maybe, you can also put a small example in your code comments.

@tandf
Copy link
Contributor Author
tandf commented Jul 21, 2022

Sure, glad to help. I'll add some comments soon.

yuleisui
yuleisui reviewed Jul 21, 2022
View reviewed changes
@tandf
Copy link
Contributor Author
tandf commented Jul 21, 2022

Hi @yuleisui. In the new commits, I update the pr based on your comments, and also add some comments to explain the logic. Please feel free to tell me if there is anything else I can do.

@yuleisui
Copy link
Collaborator

Thanks and merged

@yuleisui yuleisui merged commit b0ef2ba into SVF-tools:master Jul 22, 2022
@tandf tandf deleted the llvm_global_ctor branch July 22, 2022 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuleisui yuleisui yuleisui left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tandf @yuleisui
0