Skip to content
This repository was archived by the owner on Aug 23, 2022. It is now read-only.

Add experimental CFG recovery frontend based on Dyninst#386

Closed
Aiethel wants to merge 11 commits intolifting-bits:masterfrom
Aiethel:dyninst
Closed

Add experimental CFG recovery frontend based on Dyninst#386
Aiethel wants to merge 11 commits intolifting-bits:masterfrom
Aiethel:dyninst

Conversation

@Aiethel
Copy link
Contributor

@Aiethel Aiethel commented Mar 9, 2018

Dear McSema maintainers,

I tried to fix #295 and I think I created somewhat decent implementation that passes all basic tests included in original repository.

However there are two things I am not exactly happy about.
No matter what I was not able to get rid of -std=gnu++11 flag, that always gets appended to the end of make flags, but program needs newer standard. (Right now it needs manual overwrite)
Second one is that I had to modified one condition in /BC/Function.cpp I believe it is harmless but you may have another opinion, so let me know and I will try to fix it somehow else.

There are probably more than a few things that are not working, but as it is, it should be a solid basic block.

Regards
Lukáš Korenčik

Aiethel added 2 commits March 9, 2018 16:25
Signed-off-by: Lukáš Korenčik <xkorenc1@fi.muni.cz>
Signed-off-by: Lukáš Korenčik <xkorenc1@fi.muni.cz>
@artemdinaburg
Copy link
Contributor

Did CMAKE_CXX_STANDARD work?

We may also force a standard on the runtimes code. Can @alessandrogario or @pgoodman comment more?

@alessandrogario
Copy link
Contributor

The CMAKE_CXX_STANDARD and CMAKE_CXX_EXTENSIONS variables (leading to g++11) are being set from the remill CMake file: https://github.com/trailofbits/remill/blob/master/CMakeLists.txt#L121 and https://github.com/trailofbits/remill/blob/master/CMakeLists.txt#L125

We should probably avoid setting it like that for everything in the project! In the meanwhile, you can overwrite it directly on your target:

set_target_properties(mcsema-dyninst-disass PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED YES CXX_EXTENSIONS NO)

@Aiethel
Copy link
Contributor Author

Aiethel commented Mar 13, 2018

Thanks I have changed it accordingly. I also made some workaround so there is no need to change things in base project (except for CMakeList of course).

@@ -1,3 +1,22 @@
__gxx_personality_v0 0 C N
Copy link
Contributor

Choose a reason for hiding this comment

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

@kumarak Can you run some of your own IDA-based exception tests, using this as the linux.txt, and let me know if there are issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will update the branch with master and run the exception testcases.

Copy link
Contributor

Choose a reason for hiding this comment

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

The exception tests look good.

@artemdinaburg
Copy link
Contributor

Closing the PR for now since there is an updated version of DynInst-based CFG recovery in development.

@Aiethel Aiethel deleted the dyninst branch April 14, 2019 22:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants