Vector35 / Vector35/binaryninja-api

two issues with new register calling convention

Open
#8,455 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
1.3k
Forks
298
Avg merge
5d 5h
Merged PRs (30d)
19

Description

The x86 register calling convention added in the recent CC refactor needs two core fixes to be laid out
correctly: callee stack cleanup, and the hidden pointer for an indirectly returned result going in the argument slot following the declared parameters (EAX with no parameters, EDX after one, the stack once EAX/EDX/ECX
are taken). Without them, every Delphi prototype with a stack argument or a string/record/variant result has its arguments shifted.

Found this while working on a delphi plugin, it certainly produces better results but haven't validated it fully by hand:

diff --git a/arch/x86/arch_x86.cpp b/arch/x86/arch_x86.cpp
index 35d58a008..966f53c23 100644
--- a/arch/x86/arch_x86.cpp
+++ b/arch/x86/arch_x86.cpp
@@ -4913,11 +4913,56 @@ public:
                return type && !type->IsFloat() && type->GetWidth() > 4;
        }

+       bool IsStackAdjustedOnReturn() override
+       {
+               return true;
+       }
+
        bool AreStackArgumentsPushedLeftToRight() override
        {
                return true;
        }

+       /*! Delphi passes the hidden pointer for an indirectly returned result in the argument slot
+           *following* the declared parameters: EAX for a parameterless function, EDX after one
+           parameter, and the stack once EAX, EDX and ECX are taken. GetIndirectReturnValueLocation
+           cannot express that, because the slot depends on how many parameters precede it, so the
+           whole layout is computed here instead.
+        */
+       CallLayout GetCallLayout(BinaryView* view, const ReturnValue& returnValue, const vector<FunctionParameter>& params,
+               const std::optional<set<uint32_t>>& permittedRegs) override
+       {
+               CallLayout result = GetDefaultCallLayout(view, returnValue, params, permittedRegs);
+               if (!result.returnValue.has_value() || !result.returnValue->indirect)
+                       return result;
+
+               // A caller that has pinned the return value to an explicit location keeps it.
+               // MarkNonDefaultParameterLocations clears defaultLocation and re-runs this until the
+               // layout agrees with the location it recorded, so moving a pinned location never
+               // terminates.
+               if (!returnValue.defaultLocation)
+                       return result;
+
+               // Lay the call out a second time with the hidden pointer appended as an ordinary trailing
+               // parameter and nothing returned, then split that parameter back off as the return value.
+               vector<FunctionParameter> extended = params;
+               extended.push_back(FunctionParameter("", Type::PointerType(GetArchitecture(), Type::VoidType())));
+               CallLayout appended = GetDefaultCallLayout(view, ReturnValue(Type::VoidType()), extended, permittedRegs);
+               if (appended.parameters.size() != extended.size())
+                       return result;
+
+               ValueLocation hidden = appended.parameters.back();
+               hidden.indirect = true;
+               hidden.returnedPointer = result.returnValue->returnedPointer;
+               appended.parameters.pop_back();
+
+               result.parameters = appended.parameters;
+               result.returnValue = hidden;
+               result.stackAdjustment = appended.stackAdjustment;
+               result.registerStackAdjustments = appended.registerStackAdjustments;
+               return result;
+       }
+

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in arch/x86/arch_x86.cpp at the x86 register calling-convention implementation, especially IsStackAdjustedOnReturn and GetCallLayout, and compare the proposed layout with GetDefaultCallLayout. Verify callee cleanup and that an indirect result pointer occupies the slot after declared parameters: EAX with none, EDX after one, and the stack after EAX, EDX, and ECX are used.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, reverse-engineering
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.