Reasons why Ghidra fails to inline a function.

[marsfan]

What are the reasons that the Decompiler might fail to inline a function?

Example

I have been working on reverse-engineering an Arm Cortex-M I have a function that is called twice in a row in the code. However, the decompiler is only able to inline the first call to the function. As the two calls are nearly identical (a single register is changed), I don't see why the inlining would fail in the second case.

Disassembly of function calls

Decompilation of function calls (no inlining)

Decompilation of function calls (with inlining)

Disassembly of called function

Decompilation of called function

[astrelsky]

Can reproduce. Afaik this occurs with every single inlined function. It always fails to inline it more than once even if it is just a return 0;.

[marsfan]

@astrelsky Are you suggesting this might be a bug? I figured it was just my limited understanding of how the de compiler worked.

[astrelsky]

@astrelsky Are you suggesting this might be a bug? I figured it was just my limited understanding of how the de compiler worked.

I think there's a bug somewhere.

[quietust]

I've been using the "inline" flag as a workaround for the fact that all variations of a particular template function (T& std::vector<T>::operator[](size_t offset)) were collapsed into a single code unit - while it works great the first time it's called within a function, every other instance fails as described above.

[NyaMisty]

People finding this issue can try the patch in #4509, which I've posted a CI build for you to test it.

For it to get merged, I believe more unit test would be needed.

[NyaMisty]

I didn't expect such review but you are advised to risk yourself for putting your step further. Hence It may paralyze your feet. That's all i can say.

@homes410 Hi, thanks for your reply! But sorry your words is a little beyond my English skill. Are you advicing me to use your patch more and alert the potential bugs?

As for me, I really love your patch and I'm using it right now (because currently no other analyze tool can do inlining fluently)

Actually what I mean is that I'm afraid Ghidra team would require you to have a unit test. But for me, your patch works perfectly and I'm really grateful for that :)

[astrelsky]

I didn't expect such review but you are advised to risk yourself for putting your step further. Hence It may paralyze your feet. That's all i can say.

@homes410 Hi, thanks for your reply! But sorry your words is a little beyond my English skill. Are you advicing me to use your patch more and alert the potential bugs?

As for me, I really love your patch and I'm using it right now (because currently no other analyze tool can do inlining fluently)

Actually what I mean is that I'm afraid Ghidra team would require you to have a unit test. But for me, your patch works perfectly and I'm really grateful for that :)

@NyaMisty that's probably a spam bot. What it said does not make any sense.

[homes410]

I didn't expect such review but you are advised to risk yourself for putting your step further. Hence It may paralyze your feet. That's all i can say.

@homes410 Hi, thanks for your reply! But sorry your words is a little beyond my English skill. Are you advicing me to use your patch more and alert the potential bugs?
As for me, I really love your patch and I'm using it right now (because currently no other analyze tool can do inlining fluently)
Actually what I mean is that I'm afraid Ghidra team would require you to have a unit test. But for me, your patch works perfectly and I'm really grateful for that :)

@NyaMisty that's probably a spam bot. What it said does not make any sense.

Thanks for your remark about my comment, and I'm happy to hear more about that.
Hope you get to feeling better soon!

[LukeSerne]

I noticed ghidra also fails to inline simple functions with just some "normal" instructions, followed by a branch to a function, i.e. a tail-call. This happened so much in some functions of a binary I was analysing, that the decompiled result of the "parent" function became completely unreadable. Only the first call to these outlined functions was inlined back into the parent function, and all further calls failed to inline.

Luckily, it wasn't too hard to fix these cases, and I made a PR with my changes: #8561 - feel free to try it out and let me know if something doesn't work.

Unfortunately, those fixes still won't work for the example that was posted at the start of this function. The reason is that that example is not a simple straight-line function, but there are (conditional) branches inside. Functions like those can only be inlined once per function.

Ghidra has 2 ways it can inline a function. When it encounters a call to a function that has its "inline" flag set, it first tries to inline the function using the "EZ model". This boils down to just copying over all pcode operations from the function-to-inline to the address of the CALL operation that calls the function. The main advantage of this method is that it can be easily be done multiple times per function. The main disadvantage is that you cannot split a basic block in the middle of an address, so there cannot be any instructions in the function-to-inline that force a basic block end.

If the EZ model doesn't apply, it uses the "hard model". This replaces the call to the function-to-inline by a branch, and the return from the function-to-inline to a branch back to the pcode operation following the call. The main advantage of this method is that it allows more complex control flow in the function-to-inline, but the main disadvantages are that it requires that the pcode operation that follows the CALL operation is at the start of an address, and that it can only be done once per parent function, since the return now always jumps to the same location.

Perhaps a way to fix the limitation of the "hard model" that it can only be used once is to introduce a new varnode that indicates the intended return address of the function-to-inline. Perhaps simply by containing the address itself, and replacing the return by a BRANCHIND operation with that varnode. The downside to this implementation is that this might result in a control flow that is too complex for the decompiler to untangle.
An alternative implementation that instead encodes the return address using a number and then uses a series of INT_EQUAL and CBRANCH operations at the return address will probably not work, since the CBRANCH operations will force the basic block to end, and then you'd have a basic block split in the middle of an address.