Add IR fixups for RenderScript ABI mismatch between ARMV7 frontend and x86 backend

[ldrumm]

Summary

Expression evaluation for function calls to certain public RenderScript API functions in libRSCPURef can segfault.

[`slang`](https://android.googlesource.com/platform/frameworks/compile/slang),
the compiler frontend for RenderScript embeds an ARM specific triple in IR that is shipped in the app, after
generating IR that has some assumptions that an ARM device is the target.
As the IR is then compiled on a device of unknown (at time the IR was generated at least) architecture,
when calling RenderScript API function as part of debugger expressions, we have to perform a fixup pass that
removes those assumptions right before the module is sent to be generated by the llvm backend.

This issue is caused by multiple problems with the ARMv7-specific assumptions encoded in the LLVM IR. x86 large value returns use a hidden first argument (mapping to llvm::Attribute::StructRet), which can't be picked up by the JIT due to the mismatch between IR generated by the slang frontend and llvm backend. This means that code generated by bcc did not necessarily match the default SysV Linux/Android ABI used by the LLDB JIT

[omjavaid]

I dont have a lot of background in this area of the code. Can you kindly take a look.

[clayborg]

Sean Callanan would have more expertise in this area.

1. List Item

[tfiala]

I've got a fix to make this work in Xcode, but I'm currently stuck behind the missing Condition.cpp from r277011 to verify.

[ldrumm]

In D18059#499521, @tfiala wrote:

I've got a fix to make this work in Xcode, but I'm currently stuck behind the missing Condition.cpp from r277011 to verify.

Thanks Todd. I don't really deal with XCode, so that's a real help.

Regarding r277012, I'm a little confused about the message "Also, fixed up typos in RenderScript code that could not possibly compile." Is this code broken somehow? It build for us with GCC, and green dragon seems fine with it. Is there some particular compiler for which this is broken because everything "Works for Me"?

[tfiala]

In D18059#499522, @ldrumm wrote:

In D18059#499521, @tfiala wrote:

I've got a fix to make this work in Xcode, but I'm currently stuck behind the missing Condition.cpp from r277011 to verify.

Thanks Todd. I don't really deal with XCode, so that's a real help.

My pleasure! We expect to need to do this on the Xcode side since we aren't using cmake like everyone else.

Regarding r277012, I'm a little confused about the message "Also, fixed up typos in RenderScript code that could not possibly compile." Is this code broken somehow? It build for us with GCC, and green dragon seems fine with it. Is there some particular compiler for which this is broken because everything "Works for Me"?

Nope nope you're fine. I tried to quickly fix my comment in follow ups to my commit message emails. I looked too quickly at the filenames and figured the error was introduced from the RenderScript change. It was in fact from Saleem's change. It was in files that he couldn't compile unless he was on Xcode, so the typos came in there.

[ldrumm]

In D18059#499525, @tfiala wrote:

In D18059#499522, @ldrumm wrote:

In D18059#499521, @tfiala wrote:

I've got a fix to make this work in Xcode, but I'm currently stuck behind the missing Condition.cpp from r277011 to verify.

Thanks Todd. I don't really deal with XCode, so that's a real help.

My pleasure! We expect to need to do this on the Xcode side since we aren't using cmake like everyone else.

Regarding r277012, I'm a little confused about the message "Also, fixed up typos in RenderScript code that could not possibly compile." Is this code broken somehow? It build for us with GCC, and green dragon seems fine with it. Is there some particular compiler for which this is broken because everything "Works for Me"?

Nope nope you're fine. I tried to quickly fix my comment in follow ups to my commit message emails. I looked too quickly at the filenames and figured the error was introduced from the RenderScript change. It was in fact from Saleem's change. It was in files that he couldn't compile unless he was on Xcode, so the typos came in there.

Ah! I see the email. Thanks for the clarification.

Best

Luke

[tfiala]

Ah! I see the email. Thanks for the clarification.

Best

Luke

Sure thing :-) Sorry for the confusion.

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt

 add_lldb_library(lldbPluginRenderScriptRuntime
   RenderScriptRuntime.cpp
+  RenderScriptExpressionOpts.cpp
+  RenderScriptx86ABIFixups.cpp
   )

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptExpressionOpts.h

+//===-- ExpressionOpts.h ----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_RENDERSCRIPT_EXPROPTS_H
+#define LLDB_RENDERSCRIPT_EXPROPTS_H
+
+// C Includes
+// C++ Includes
+// Other libraries and framework includes
+#include "llvm/IR/Module.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Target/TargetOptions.h"
+
+// Project includes
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/Process.h"
+#include "lldb/lldb-private.h"
+
+#include "RenderScriptRuntime.h"
+#include "RenderScriptx86ABIFixups.h"
+
+// RenderScriptRuntimeModulePass is a simple llvm::ModulesPass that is used during expression evaluation to apply
+// RenderScript-specific fixes for expression evaluation.
+// In particular this is used to make expression IR conformant with the ABI generated by the slang frontend. This
+// ModulePass is executed in ClangExpressionParser::PrepareForExecution whenever an expression's DWARF language is
+// eLanguageTypeExtRenderscript
+
+class RenderScriptRuntimeModulePass : public llvm::ModulePass
+{
+public:
+    static char ID;
+    RenderScriptRuntimeModulePass(const lldb_private::Process *process) : ModulePass(ID), m_process_ptr(process) {}
+
+    bool
+    runOnModule(llvm::Module &module);
+
+private:
+    const lldb_private::Process *m_process_ptr;
+};
+
+namespace lldb_private
+{
+namespace lldb_renderscript
+{
+struct RSIRPasses : public lldb_private::LLVMUserExpression::IRPasses
+{
+    RSIRPasses(lldb_private::Process *process);
+
+    ~RSIRPasses();
+};
+} // namespace lldb_renderscript
+} // namespace lldb_private
+#endif

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptExpressionOpts.cpp

+//===-- ExpressionOpts.cpp --------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// C Includes
+// C++ Includes
+#include <string>
+
+// Other libraries and framework includes
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/LegacyPassManager.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Target/TargetOptions.h"
+
+#include "clang/Basic/TargetOptions.h"
+
+// Project includes
+#include "lldb/Core/Log.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+
+#include "RenderScriptExpressionOpts.h"
+#include "RenderScriptRuntime.h"
+#include "RenderScriptx86ABIFixups.h"
+
+using namespace lldb_private;
+using namespace lldb_renderscript;
+
+// [``slang``](https://android.googlesource.com/platform/frameworks/compile/slang),
+// the compiler frontend for RenderScript embeds an ARM specific triple in IR that is shipped in the app, after
+// generating IR that has some assumptions that an ARM device is the target.
+// As the IR is then compiled on a device of unknown (at time the IR was generated at least) architecture,
+// when calling RenderScript API function as part of debugger expressions, we have to perform a fixup pass that
+// removes those assumptions right before the module is sent to be generated by the llvm backend.
+
+namespace
+{
+bool
+registerRSDefaultTargetOpts(clang::TargetOptions &proto, const llvm::Triple::ArchType &arch)
+{
+    switch (arch)
+    {
+        case llvm::Triple::ArchType::x86:
+            proto.Triple = "i686--linux-android";
+            proto.CPU = "atom";
+            proto.Features.push_back("+long64");
+        // Fallthrough for common x86 family features
+        case llvm::Triple::ArchType::x86_64:
+            proto.Features.push_back("+mmx");
+            proto.Features.push_back("+sse");
+            proto.Features.push_back("+sse2");
+            proto.Features.push_back("+sse3");
+            proto.Features.push_back("+ssse3");
+            proto.Features.push_back("+sse4.1");
+            proto.Features.push_back("+sse4.2");
+            break;
+        case llvm::Triple::ArchType::mipsel:
+            // pretend this is `arm' for the front-end
+            proto.Triple = "armv7-none-linux-android";
+            proto.CPU = "";
+            proto.Features.push_back("+long64");
+            break;
+        case llvm::Triple::ArchType::mips64el:
+            // pretend this is `aarch64' for the front-end
+            proto.Triple = "aarch64-none-linux-android";
+            proto.CPU = "";
+            break;
+        default:
+            return false;
+    }
+    return true;
+}
+} // end anonymous namespace
+
+bool
+RenderScriptRuntimeModulePass::runOnModule(llvm::Module &module)
+{
+    bool changed_module = false;
+    Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE | LIBLLDB_LOG_EXPRESSIONS));
+
+    std::string err;
+    llvm::StringRef real_triple = m_process_ptr->GetTarget().GetArchitecture().GetTriple().getTriple();
+    const llvm::Target *target_info = llvm::TargetRegistry::lookupTarget(real_triple, err);
+    if (!target_info)
+    {
+        if (log)
+            log->Warning("couldn't determine real target architecture: '%s'", err.c_str());
+        return false;
+    }
+
+    llvm::Optional<llvm::Reloc::Model> reloc_model = llvm::None;
+    assert(m_process_ptr && "no available lldb process");
+    switch (m_process_ptr->GetTarget().GetArchitecture().GetMachine())
+    {
+        case llvm::Triple::ArchType::x86:
+            changed_module |= fixupX86FunctionCalls(module);
+            // For some reason this triple gets totally missed by the backend, and must be set manually.
+            // There a reference in bcc/Main.cpp about auto feature-detection being removed from LLVM3.5, but I can't
+            // see that discussion anywhere public.
+            real_triple = "i686--linux-android";
+            break;
+        case llvm::Triple::ArchType::x86_64:
+            changed_module |= fixupX86_64FunctionCalls(module);
+            break;
+        case llvm::Triple::ArchType::mipsel:
+        case llvm::Triple::ArchType::mips64el:
+            // No actual IR fixup pass is needed on MIPS, but the datalayout
+            // and targetmachine do need to be explicitly set.
+
+            // bcc explicitly compiles MIPS code to use the static relocation
+            // model due to an issue with relocations in mclinker.
+            // see libbcc/support/CompilerConfig.cpp for details
+            reloc_model = llvm::Reloc::Static;
+            changed_module = true;
+            break;
+        case llvm::Triple::ArchType::arm:
+        case llvm::Triple::ArchType::aarch64:
+            // ARM subtargets need no fixup passes as they are the initial target as generated by the
+            // slang compiler frontend.
+            break;
+        default:
+            if (log)
+                log->Warning("Ignoring unknown renderscript target");
+            return false;
+    }
+
+    if (changed_module)
+    {
+        llvm::TargetOptions options;
+        llvm::TargetMachine *target_machine =
+            target_info->createTargetMachine(real_triple, "", "", options, reloc_model);
+        assert(target_machine && "failed to identify RenderScriptRuntime target machine");
+        // We've been using a triple and datalayout of some ARM variant all along, so
+        // we need to let the backend know that this is no longer the case.
+        if (log)
+        {
+            log->Printf("%s - Changing RS target triple to '%s'", __FUNCTION__, real_triple.str().c_str());
+            log->Printf("%s - Changing RS datalayout to '%s'", __FUNCTION__,
+                        target_machine->createDataLayout().getStringRepresentation().c_str());
+        }
+        module.setTargetTriple(real_triple);
+        module.setDataLayout(target_machine->createDataLayout());
+    }
+    return changed_module;
+}
+
+char RenderScriptRuntimeModulePass::ID = 0;
+
+namespace lldb_private
+{
+
+bool
+RenderScriptRuntime::GetOverrideExprOptions(clang::TargetOptions &proto)
+{
+    auto *process = GetProcess();
+    assert(process);
+    return registerRSDefaultTargetOpts(proto, process->GetTarget().GetArchitecture().GetMachine());
+}
+
+bool
+RenderScriptRuntime::GetIRPasses(LLVMUserExpression::IRPasses &passes)
+{
+    if (!m_ir_passes)
+        m_ir_passes = new RSIRPasses(GetProcess());
+    assert(m_ir_passes);
+
+    passes.EarlyPasses = m_ir_passes->EarlyPasses;
+    passes.LatePasses = m_ir_passes->LatePasses;
+
+    return true;
+}
+
+namespace lldb_renderscript
+{
+
+RSIRPasses::RSIRPasses(Process *process)
+{
+    IRPasses();
+    assert(process);
+
+    EarlyPasses = std::make_shared<llvm::legacy::PassManager>();
+    assert(EarlyPasses);
+    EarlyPasses->add(new RenderScriptRuntimeModulePass(process));
+}
+
+RSIRPasses::~RSIRPasses()
+{
+}
+
+} // namespace lldb_renderscript
+} // namespace lldb_private

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h

 #include <map>
 #include <memory>
 #include <string>
 #include <vector>
 
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/Module.h"
+#include "lldb/Expression/LLVMUserExpression.h"
 #include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private
 {
 namespace lldb_renderscript
 {
protected:
     struct Element;
 
     void
     InitSearchFilter(lldb::TargetSP target)
     {
         if (!m_filtersp)
             m_filtersp.reset(new SearchFilterForUnconstrainedSearches(target));
     }
-    
     void
     FixupScriptDetails(lldb_renderscript::RSModuleDescriptorSP rsmodule_sp);
 
     void
     LoadRuntimeHooks(lldb::ModuleSP module, ModuleKind kind);
 
     bool
     RefreshAllocation(AllocationDetails *allocation, StackFrame *frame_ptr);
protected:
 
     lldb::SearchFilterSP m_filtersp; // Needed to create breakpoints through Target API
 
     bool m_initiated;
     bool m_debuggerPresentFlagged;
     bool m_breakAllKernels;
     static const HookDefn s_runtimeHookDefns[];
     static const size_t s_runtimeHookCount;
+    LLVMUserExpression::IRPasses *m_ir_passes;
 
 private:
     RenderScriptRuntime(Process *process); // Call CreateInstance instead.
-    
     static bool
     HookCallback(void *baton, StoppointCallbackContext *ctx, lldb::user_id_t break_id, lldb::user_id_t break_loc_id);
 
     static bool
     KernelBreakpointHit(void *baton, StoppointCallbackContext *ctx, lldb::user_id_t break_id,
                         lldb::user_id_t break_loc_id);
 
     void
private:
     LookUpScript(lldb::addr_t address, bool create);
 
     // Search for a previously saved allocation detail object using a target address.
     // If an allocation does not exist for this address then nullptr will be returned.
     // If 'create' is true and there is no previous allocation then a new allocation
     // detail object will be created for this address and returned.
     AllocationDetails *
     LookUpAllocation(lldb::addr_t address, bool create);
+
+    bool
+    GetOverrideExprOptions(clang::TargetOptions &prototype) override;
+
+    bool
+    GetIRPasses(LLVMUserExpression::IRPasses &passes) override;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_RenderScriptRuntime_h_

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

 {
     assert(!m_initiated);
 }
 
 RenderScriptRuntime::RenderScriptRuntime(Process *process)
     : lldb_private::CPPLanguageRuntime(process),
       m_initiated(false),
       m_debuggerPresentFlagged(false),
-      m_breakAllKernels(false)
+      m_breakAllKernels(false),
+      m_ir_passes(nullptr)
 {
     ModulesDidLoad(process->GetTarget().GetImages());
 }
 
 lldb::CommandObjectSP
 RenderScriptRuntime::GetCommandObject(lldb_private::CommandInterpreter &interpreter)
 {
     return CommandObjectSP(new CommandObjectRenderScriptRuntime(interpreter));
 }
 
 RenderScriptRuntime::~RenderScriptRuntime() = default;

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.h

+//===-- x86ABIFixups.h ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_RENDERSCRIPT_X86_H
+#define LLDB_RENDERSCRIPT_X86_H
+
+#include "llvm/IR/Module.h"
+
+namespace lldb_private
+{
+namespace lldb_renderscript
+{
+
+bool
+fixupX86FunctionCalls(llvm::Module &module);
+
+bool
+fixupX86_64FunctionCalls(llvm::Module &module);
+}
+}
+#endif

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp

+//===-- x86ABIFixups.cpp ----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// C Includes
+// C++ Includes
+#include <set>
+
+// Other libraries and framework includes
+#include "llvm/ADT/StringRef.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/CallSite.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IRReader/IRReader.h"
+#include "llvm/Pass.h"
+
+// Project includes
+#include "lldb/Core/Log.h"
+#include "lldb/Target/Process.h"
+
+using namespace lldb_private;
+namespace
+{
+
+bool
+isRSAPICall(llvm::Module &module, llvm::CallInst *call_inst)
+{
+    // TODO get the list of renderscript modules from lldb and check if
+    // this llvm::Module calls into any of them.
+    (void)module;
+    const auto func_name = call_inst->getCalledFunction()->getName();
+    if (func_name.startswith("llvm") || func_name.startswith("lldb"))
+        return false;
+
+    if (call_inst->getCalledFunction()->isIntrinsic())
+        return false;
+
+    return true;
+}
+
+bool
+isRSLargeReturnCall(llvm::Module &module, llvm::CallInst *call_inst)
+{
+    // i686 and x86_64 returns for large vectors in the RenderScript API are not handled as normal
+    // register pairs, but as a hidden sret type. This is not reflected in the debug info or mangled
+    // symbol name, and the android ABI for x86 and x86_64, (as well as the emulators) specifies there is
+    // no AVX, so bcc generates an sret function because we cannot natively return 256 bit vectors.
+    // This function simply checks whether a function has a > 128bit return type. It is perhaps an
+    // unreliable heuristic, and relies on bcc not generating AVX code, so if the android ABI one day
+    // provides for AVX, this function may go out of fashion.
+    (void)module;
+    if (!call_inst || !call_inst->getCalledFunction())
+        return false;
+
+    return call_inst->getCalledFunction()->getReturnType()->getPrimitiveSizeInBits() > 128;
+}
+
+bool
+isRSAllocationPtrTy(const llvm::Type *type)
+{
+    if (!type->isPointerTy())
+        return false;
+    auto ptr_type = type->getPointerElementType();
+
+    return ptr_type->isStructTy() && ptr_type->getStructName().startswith("struct.rs_allocation");
+}
+
+bool
+isRSAllocationTyCallSite(llvm::Module &module, llvm::CallInst *call_inst)
+{
+    (void)module;
+    if (!call_inst->hasByValArgument())
+        return false;
+    for (const auto &param : call_inst->operand_values())
+        if (isRSAllocationPtrTy(param->getType()))
+            return true;
+    return false;
+}
+
+llvm::FunctionType *
+cloneToStructRetFnTy(llvm::CallInst *call_inst)
+{
+    // on x86 StructReturn functions return a pointer to the return value, rather than the return
+    // value itself [ref](http://www.agner.org/optimize/calling_conventions.pdf section 6).
+    // We create a return type by getting the pointer type of the old return type, and inserting a new
+    // initial argument of pointer type of the original return type.
+    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_LANGUAGE | LIBLLDB_LOG_EXPRESSIONS));
+
+    assert(call_inst && "no CallInst");
+    llvm::Function *orig = call_inst->getCalledFunction();
+    assert(orig && "CallInst has no called function");
+    llvm::FunctionType *orig_type = orig->getFunctionType();
+    auto name = orig->getName();
+    if (log)
+        log->Printf("%s - cloning to StructRet function for '%s'", __FUNCTION__, name.str().c_str());
+
+    std::vector<llvm::Type *> new_params{orig_type->getNumParams() + 1, nullptr};
+    unsigned num_params = orig_type->getNumParams();
+    std::vector<llvm::Type *> params{orig_type->param_begin(), orig_type->param_end()};
+
+    // This may not work if the function is somehow declared void as llvm is strongly typed
+    // and represents void* with i8*
+    assert(!orig_type->getReturnType()->isVoidTy() && "Cannot add StructRet attribute to void function");
+    llvm::PointerType *return_type_ptr_type = llvm::PointerType::getUnqual(orig->getReturnType());
+    assert(return_type_ptr_type && "failed to get function return type PointerType");
+    if (!return_type_ptr_type)
+        return nullptr;
+
+    if (log)
+        log->Printf("%s - return type pointer type for StructRet clone @ '0x%p':\n", __FUNCTION__,
+                    (void *)return_type_ptr_type);
+    // put the the sret pointer argument in place at the beginning of the argument list.
+    params.emplace(params.begin(), return_type_ptr_type);
+    assert(params.size() == num_params + 1);
+    return llvm::FunctionType::get(return_type_ptr_type, params, orig->isVarArg());
+}
+
+bool
+findRSCallSites(llvm::Module &module, std::set<llvm::CallInst *> &rs_callsites,
+                bool (*predicate)(llvm::Module &, llvm::CallInst *))
+{
+    bool found = false;
+
+    for (auto &func : module.getFunctionList())
+        for (auto &block : func.getBasicBlockList())
+            for (auto &inst : block)
+            {
+                llvm::CallInst *call_inst = llvm::dyn_cast_or_null<llvm::CallInst>(&inst);
+                if (!call_inst || !call_inst->getCalledFunction())
+                    // This is not the call-site you are looking for...
+                    continue;
+                if (isRSAPICall(module, call_inst) && predicate(module, call_inst))
+                {
+                    rs_callsites.insert(call_inst);
+                    found = true;
+                }
+            }
+    return found;
+}
+
+bool
+fixupX86StructRetCalls(llvm::Module &module)
+{
+    bool changed = false;
+    // changing a basic block while iterating over it seems to have some undefined behaviour
+    // going on so we find all RS callsites first, then fix them up after consuming
+    // the iterator.
+    std::set<llvm::CallInst *> rs_callsites;
+    if (!findRSCallSites(module, rs_callsites, isRSLargeReturnCall))
+        return false;
+
+    for (auto call_inst : rs_callsites)
+    {
+        llvm::FunctionType *new_func_type = cloneToStructRetFnTy(call_inst);
+        assert(new_func_type && "failed to clone functionType for Renderscript ABI fixup");
+
+        llvm::CallSite call_site(call_inst);
+        llvm::Function *func = call_inst->getCalledFunction();
+        assert(func && "cannot resolve function in RenderScriptRuntime");
+        // Copy the original call arguments
+        std::vector<llvm::Value *> new_call_args(call_site.arg_begin(), call_site.arg_end());
+
+        // Allocate enough space to store the return value of the original function
+        // we pass a pointer to this allocation as the StructRet param, and then copy its
+        // value into the lldb return value
+        llvm::AllocaInst *return_value_alloc =
+            new llvm::AllocaInst(func->getReturnType(), "var_vector_return_alloc", call_inst);
+        // use the new allocation as the new first argument
+        new_call_args.emplace(new_call_args.begin(), llvm::cast<llvm::Value>(return_value_alloc));
+        llvm::PointerType *new_func_ptr_type = llvm::PointerType::get(new_func_type, 0);
+        // Create the type cast from the old function type to the new one
+        llvm::Constant *new_func_cast =
+            llvm::ConstantExpr::getCast(llvm::Instruction::BitCast, func, new_func_ptr_type);
+        // create an allocation for a new function pointer
+        llvm::AllocaInst *new_func_ptr = new llvm::AllocaInst(new_func_ptr_type, "new_func_ptr", call_inst);
+        // store the new_func_cast to the newly allocated space
+        (void)new llvm::StoreInst(new_func_cast, new_func_ptr, "new_func_ptr_load_cast", call_inst);
+        // load the new function address ready for a jump
+        llvm::LoadInst *new_func_addr_load = new llvm::LoadInst(new_func_ptr, "load_func_pointer", call_inst);
+        // and create a callinstruction from it
+        llvm::CallInst *new_call_inst =
+            llvm::CallInst::Create(new_func_addr_load, new_call_args, "new_func_call", call_inst);
+        new_call_inst->setCallingConv(call_inst->getCallingConv());
+        new_call_inst->setTailCall(call_inst->isTailCall());
+        llvm::LoadInst *lldb_save_result_address = new llvm::LoadInst(return_value_alloc, "save_return_val", call_inst);
+
+        // Now remove the old broken call
+        call_inst->replaceAllUsesWith(lldb_save_result_address);
+        call_inst->eraseFromParent();
+        changed = true;
+    }
+    return changed;
+}
+
+bool
+fixupRSAllocationStructByValCalls(llvm::Module &module)
+{
+    // On x86_64, calls to functions in the RS runtime that take an `rs_allocation` type argument
+    // are actually handled as by-ref params by bcc, but appear to be passed by value by lldb (the callsite all use
+    // `struct byval`).
+    // On x86_64 Linux, struct arguments are transferred in registers if the struct size is no bigger than
+    // 128bits [ref](http://www.agner.org/optimize/calling_conventions.pdf) section 7.1 "Passing and returning objects"
+    // otherwise passed on the stack.
+    // an object of type `rs_allocation` is actually 256bits, so should be passed on the stack. However, code generated
+    // by bcc actually treats formal params of type `rs_allocation` as `rs_allocation *` so we need to convert the
+    // calling convention to pass by reference, and remove any hint of byval from formal parameters.
+    bool changed = false;
+    std::set<llvm::CallInst *> rs_callsites;
+    if (!findRSCallSites(module, rs_callsites, isRSAllocationTyCallSite))
+        return false;
+
+    std::set<llvm::Function *> rs_functions;
+
+    // for all call instructions
+    for (auto call_inst : rs_callsites)
+    {
+        // add the called function to a set so that we can strip its byval attributes in another pass
+        rs_functions.insert(call_inst->getCalledFunction());
+
+        // get the function attributes
+        llvm::AttributeSet call_attribs = call_inst->getAttributes();
+
+        // iterate over the argument attributes
+        for (size_t i = 1; i <= call_attribs.getNumSlots(); ++i)
+        {
+            // if this argument is passed by val
+            if (call_attribs.hasAttribute(i, llvm::Attribute::ByVal))
+            {
+                // strip away the byval attribute
+                call_inst->removeAttribute(i, llvm::Attribute::get(module.getContext(), llvm::Attribute::ByVal));
+                changed = true;
+            }
+        }
+    }
+
+    llvm::AttributeSet attr_byval = llvm::AttributeSet::get(module.getContext(), 1u, llvm::Attribute::ByVal);
+
+    // for all called function decls
+    for (auto func : rs_functions)
+    {
+        // inspect all of the arguments in the call
+        llvm::SymbolTableList<llvm::Argument> &argList = func->getArgumentList();
+        for (auto &arg : argList)
+        {
+            if (arg.hasByValAttr())
+            {
+                arg.removeAttr(attr_byval);
+                changed = true;
+            }
+        }
+    }
+    return changed;
+}
+} // end anonymous namespace
+
+namespace lldb_private
+{
+namespace lldb_renderscript
+{
+
+bool
+fixupX86FunctionCalls(llvm::Module &module)
+{
+    return fixupX86StructRetCalls(module);
+}
+
+bool
+fixupX86_64FunctionCalls(llvm::Module &module)
+{
+    bool changed = false;
+    changed |= fixupX86StructRetCalls(module);
+    changed |= fixupRSAllocationStructByValCalls(module);
+    return changed;
+}
+
+} // end namespace lldb_renderscript
+} // end namespace lldb_private