Add spec for inlining plt entries #134
Conversation
|
I tried this on a couple of examples and it works great! |
| and fun_spec_type = | ||
| | Summary of (t -> Constr.t -> Tid.t -> Constr.t * t) | ||
| | Inline | ||
| | Inline of Tid.t |
There was a problem hiding this comment.
This is actually quite nice!
| else | ||
| None | ||
|
|
||
| let spec_inline_plt (to_inline : Sub.t Seq.t) (sub : Sub.t) (_ : Arch.t) |
There was a problem hiding this comment.
Comments here might be quite welcome, when we debug this in 3 months...
There was a problem hiding this comment.
This is not a PLT pattern, neither it is guaranteed that a function that has @ symbol is a PLT (or any other) stub, nor that the @ symbol is the separator (there is at least one more separator). So this code is quite brittle and most likely will break in the future.
The long story. In order to maintain the injectivity of the tids to name mapping, we have to ensure that all functions also have unique names. This invariant is maintained on the Bap.Std.Program level, and when a new function is added we check that it is unique and if it is not we mangle its name (using @<addr> is just one of the strategies that is used). See BinaryAnalysisPlatform/bap#808 for more information on that, and here is the code that performs mangling. As you can see, the strategy can't guarantee that in case of two functions having the same name, we give the mangled name to the stub.
You might also interested in the stub-resolver ABI pass that @gitoleg is implementing as a part of BinaryAnalysisPlatform/bap#1036 (Oleg, btw can you push this pass as a separate PR? As there are some questions about x86/primus part of that PR, but this particular module is independent of that and can be already merged.)
|
Looks good! Modulo some explanation of our strategy for finding the body of |
|
Ok, we have cleaned up BinaryAnalysisPlatform/bap#1036, if you guys can pull it and verify that it rectifies issue #132 it would be wonderful. |
Thanks a bunch! Will try it now. |
|
Let's close this for now, since it's likely the fix will be tied to some things on the BAP side which are not yet pushed. |
|
yep, your tests already passing on our side. I think we will push it soonish |
Refers to the bug found in #132