Change procedure linkage table indirect calls to direct calls

[richardlford]

When a function in an external dynamically-loaded library is
called, the call is made indirectly through the
procedure linkage table (PLT). The call into the PLT is
made with an ordinary call instruction, but in the PLT
there is a jump (not call) to the final destination.
In this situation retdec is not properly handling
the parameter passing. This change turns these indirect
calls into direct calls and deletes the PLT code.
This makes the result simpler and allows the parameters
to be properly passed.

[richardlford]

@PeterMatula, could you please review this PR? Thanks.

[PeterMatula]

I will, before the end of this week.

[PeterMatula]

Thanks @richardlford, I think in general it looks good. Few regression tests are failing, but I reviewed the problems and it looks like it is nothing related to output quality regression. Most fails are due to some crash I will have to further investigate, and probably one or two are some minor details that were testing the layout before your changes.
I will try to fix the tests and let you know.

[richardlford]

Few regression tests are failing, but I reviewed the problems and it looks like it is nothing related to output quality regression. Most fails are due to some crash I will have to further investigate, and probably one or two are some minor details that were testing the layout before your changes.
I will try to fix the tests and let you know.

@PeterMatula, sorry, I should have run the tests first. I am now setting up to run the regression tests myself, althrough I do not have Ada Pro so will not be able to run those.

[richardlford]

@PeterMatula, I have investigated the crashes. In my code, after making the PLT calls direct I was deleting the PLT function. But that was causing problems related to virtual calls. When I don't delete the PLT function only 3 tests are failing and those are ones where my changes cause improvement, so the test will need to be adjusted. I expect to have a new commit early next week.

[richardlford]

@PeterMatula, I would like your advice on some issues related to this change and have some questions.

1. In looking at the test cases in

retdec-regression-tests/features/fnc-arguments/prantf

I noticed that for the 32 bit arm binary, that the calls to the dynamically linked functions are in the .text section rather than the
.plt section, though there is something in the .plt section. So my idea of just searching in the .plt section does not work for arm.

2. In looking at the param_return pass that supplies parameter and return types for functions, I noticed the getWrapper function which looks for functions whose body contains only a call (though in the assembly it is actually a jump) to a dynamically linked function. getWrapper expects the function to be in the .plt section, unless it is short. I'm guessing that the short alternative is to deal with architectures like arm that do not put their dynamic calls in the .plt section. Does that seem right to you?

3. The getWrapper function also has code like this:

	// Pattern
	// .text:00008A38                 LDR     R0, =aCCc       ; "C::cc()"
	// .text:00008A3C                 B       puts
	// .text:00008A40 off_8A40        DCD aCCc
	// TODO: make better wrapper detection. In wrapper, wrapped function params
	// should not be set like in this example.
	//
	if (ai && next)
	{
		if (_image->getConstantDefault(next.getEndAddress()))
		{
			auto* l = ai.getInstructionFirst<LoadInst>();
			auto* s = ai.getInstructionFirst<StoreInst>();
			auto* c = next.getInstructionFirst<CallInst>();
			if (l && s && c && isa<GlobalVariable>(l->getPointerOperand())
					&& s->getPointerOperand()->getName() == "r0")
			{
				auto gvA = _config->getGlobalAddress(cast<GlobalVariable>(l->getPointerOperand()));
				if (gvA == next.getEndAddress())
				{
					return nullptr;
				}
			}
		}
	}

which appears to be target dependent. Do you agree? If so, is there a target independent way to do the same thing?

4. In looking at the param_return optimization and its search for wrapped calls, it occurs to me that my changes might better be made in that pass rather than where I am currently making them. Do you have any opinion on that?

Thanks for your help. Dealing with multiple architectures definitely complicate things. By the way, do you have a standard way of producing binaries on all these platforms. Do you use cross-compilation, or do you have native systems for all these architectures?

[richardlford]

@PeterMatula, I did decide to accomplish my goal in the param_return pass rather than with these changes. Since I don't use any of these changes I'm closing this pull request. I've created a new pull request, #1085. I've also created a pull request in the retdec-regressions-tests repo, avast/retdec-regression-tests#117 (comment), to account for the effect of these changes.