[Target] Move InferiorCall to Process

[xiaobai]

Summary

InferiorCall is only ever used in Process, and it is not specific to
POSIX. By moving it to Process, we can remove all dependencies on plugins from
Process. Moving InferiorCall to Process seems to achieve this quite well.
Additionally, the name InferiorCall is a little vague now, so we rename
it something a bit more specific.

[jingham]

This seems like the right place to put it, it's clearly something you would do with a process.

I don't see any reason why this should be a private function. It's just a convenience wrapper around a bunch of other public functionality, and doesn't do anything that needs to be hidden from clients of Process. I wouldn't want somebody else to reinvent it if they needed to do the same thing.

However, InferiorCall is a pretty vague name for something that has a very specific task. Once it is part of Process, the "Inferior" bit is unnecessary, the Process IS the inferior... Maybe just CallVoidArgVoidPtrReturn? That's not too hideous.

[JDevlieghere]

I was going to say LGTM unless Jim has concerns, but looks like he cut me to the chase :-)

[clayborg]

lgtm. Jim?

[jingham]

In D67472#1668505, @clayborg wrote:

lgtm. Jim?

I already commented above.

[xiaobai]

Rename InferiorCall to CallNoArgNoReturnFunc
make CallNoArgReturnFunc public

[jingham]

I don't think CallNoArgNoReturnFunc is the right name. This routine calls a function that takes no arguments but returns a void *.

[xiaobai]

Rename function in question to CallVoidArgVoidPtrReturn

[jingham]

Excellent!

[labath]

cool

lldb/trunk/include/lldb/Target/Process.h

   /// server, which shall interpret accordingly. The thread_id can be
   /// specified in the options to obtain the configuration used by a specific
   /// thread. The thread_id specified should also match the uid otherwise an
   /// error will be returned.
   virtual Status GetTraceConfig(lldb::user_id_t uid, TraceOptions &options) {
     return Status("Not implemented");
   }
 
+  // This calls a function of the form "void * (*)(void)".
+  bool CallVoidArgVoidPtrReturn(const Address *address,
+                                lldb::addr_t &returned_func,
+                                bool trap_exceptions = false);
+
 protected:
   void SetState(lldb::EventSP &event_sp);
 
   lldb::StateType GetPrivateState();
 
   /// The "private" side of resuming a process.  This doesn't alter the state
   /// of m_run_lock, but just causes the process to resume.
   ///

lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.h

 };
 
 bool InferiorCallMmap(Process *proc, lldb::addr_t &allocated_addr,
                       lldb::addr_t addr, lldb::addr_t length, unsigned prot,
                       unsigned flags, lldb::addr_t fd, lldb::addr_t offset);
 
 bool InferiorCallMunmap(Process *proc, lldb::addr_t addr, lldb::addr_t length);
 
-bool InferiorCall(Process *proc, const Address *address,
-                  lldb::addr_t &returned_func, bool trap_exceptions = false);
-
 } // namespace lldb_private
 
 #endif // lldb_InferiorCallPOSIX_h_

lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp

if (sc_list.GetContextAtIndex(0, sc)) {
           }
         }
       }
     }
   }
 
   return false;
 }
-
-// FIXME: This has nothing to do with Posix, it is just a convenience function
-// that calls a
-// function of the form "void * (*)(void)".  We should find a better place to
-// put this.
-
-bool lldb_private::InferiorCall(Process *process, const Address *address,
-                                addr_t &returned_func, bool trap_exceptions) {
-  Thread *thread =
-      process->GetThreadList().GetExpressionExecutionThread().get();
-  if (thread == nullptr || address == nullptr)
-    return false;
-
-  EvaluateExpressionOptions options;
-  options.SetStopOthers(true);
-  options.SetUnwindOnError(true);
-  options.SetIgnoreBreakpoints(true);
-  options.SetTryAllThreads(true);
-  options.SetDebug(false);
-  options.SetTimeout(process->GetUtilityExpressionTimeout());
-  options.SetTrapExceptions(trap_exceptions);
-
-  auto type_system_or_err =
-      process->GetTarget().GetScratchTypeSystemForLanguage(eLanguageTypeC);
-  if (!type_system_or_err) {
-    llvm::consumeError(type_system_or_err.takeError());
-    return false;
-  }
-  CompilerType void_ptr_type =
-      type_system_or_err->GetBasicTypeFromAST(eBasicTypeVoid).GetPointerType();
-  lldb::ThreadPlanSP call_plan_sp(
-      new ThreadPlanCallFunction(*thread, *address, void_ptr_type,
-                                 llvm::ArrayRef<addr_t>(), options));
-  if (call_plan_sp) {
-    DiagnosticManager diagnostics;
-
-    StackFrame *frame = thread->GetStackFrameAtIndex(0).get();
-    if (frame) {
-      ExecutionContext exe_ctx;
-      frame->CalculateExecutionContext(exe_ctx);
-      ExpressionResults result =
-          process->RunThreadPlan(exe_ctx, call_plan_sp, options, diagnostics);
-      if (result == eExpressionCompleted) {
-        returned_func =
-            call_plan_sp->GetReturnValueObject()->GetValueAsUnsigned(
-                LLDB_INVALID_ADDRESS);
-
-        if (process->GetAddressByteSize() == 4) {
-          if (returned_func == UINT32_MAX)
-            return false;
-        } else if (process->GetAddressByteSize() == 8) {
-          if (returned_func == UINT64_MAX)
-            return false;
-        }
-        return true;
-      }
-    }
-  }
-
-  return false;
-}

lldb/trunk/source/Target/Process.cpp

 //===-- Process.cpp ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <atomic>
 #include <memory>
 #include <mutex>
 
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 
-#include "Plugins/Process/Utility/InferiorCallPOSIX.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"

 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/StructuredDataPlugin.h"
 #include "lldb/Target/SystemRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlan.h"
 #include "lldb/Target/ThreadPlanBase.h"
+#include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/NameMatches.h"
 #include "lldb/Utility/ProcessInfo.h"
 #include "lldb/Utility/SelectHelper.h"
 #include "lldb/Utility/State.h"
 
addr_t Process::ResolveIndirectFunction(const Address *address, Status &error) {
   addr_t function_addr = LLDB_INVALID_ADDRESS;
 
   addr_t addr = address->GetLoadAddress(&GetTarget());
   std::map<addr_t, addr_t>::const_iterator iter =
       m_resolved_indirect_addresses.find(addr);
   if (iter != m_resolved_indirect_addresses.end()) {
     function_addr = (*iter).second;
   } else {
-    if (!InferiorCall(this, address, function_addr)) {
+    if (!CallVoidArgVoidPtrReturn(address, function_addr)) {
       Symbol *symbol = address->CalculateSymbolContextSymbol();
       error.SetErrorStringWithFormat(
           "Unable to call resolver for indirect function %s",
           symbol ? symbol->GetName().AsCString() : "<UNKNOWN>");
       function_addr = LLDB_INVALID_ADDRESS;
     } else {
       m_resolved_indirect_addresses.insert(
           std::pair<addr_t, addr_t>(addr, function_addr));
UtilityFunction *Process::GetLoadImageUtilityFunction(
     Platform *platform,
     llvm::function_ref<std::unique_ptr<UtilityFunction>()> factory) {
   if (platform != GetTarget().GetPlatform().get())
     return nullptr;
   llvm::call_once(m_dlopen_utility_func_flag_once,
                   [&] { m_dlopen_utility_func_up = factory(); });
   return m_dlopen_utility_func_up.get();
 }
+
+bool Process::CallVoidArgVoidPtrReturn(const Address *address,
+                                       addr_t &returned_func,
+                                       bool trap_exceptions) {
+  Thread *thread = GetThreadList().GetExpressionExecutionThread().get();
+  if (thread == nullptr || address == nullptr)
+    return false;
+
+  EvaluateExpressionOptions options;
+  options.SetStopOthers(true);
+  options.SetUnwindOnError(true);
+  options.SetIgnoreBreakpoints(true);
+  options.SetTryAllThreads(true);
+  options.SetDebug(false);
+  options.SetTimeout(GetUtilityExpressionTimeout());
+  options.SetTrapExceptions(trap_exceptions);
+
+  auto type_system_or_err =
+      GetTarget().GetScratchTypeSystemForLanguage(eLanguageTypeC);
+  if (!type_system_or_err) {
+    llvm::consumeError(type_system_or_err.takeError());
+    return false;
+  }
+  CompilerType void_ptr_type =
+      type_system_or_err->GetBasicTypeFromAST(eBasicTypeVoid).GetPointerType();
+  lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallFunction(
+      *thread, *address, void_ptr_type, llvm::ArrayRef<addr_t>(), options));
+  if (call_plan_sp) {
+    DiagnosticManager diagnostics;
+
+    StackFrame *frame = thread->GetStackFrameAtIndex(0).get();
+    if (frame) {
+      ExecutionContext exe_ctx;
+      frame->CalculateExecutionContext(exe_ctx);
+      ExpressionResults result =
+          RunThreadPlan(exe_ctx, call_plan_sp, options, diagnostics);
+      if (result == eExpressionCompleted) {
+        returned_func =
+            call_plan_sp->GetReturnValueObject()->GetValueAsUnsigned(
+                LLDB_INVALID_ADDRESS);
+
+        if (GetAddressByteSize() == 4) {
+          if (returned_func == UINT32_MAX)
+            return false;
+        } else if (GetAddressByteSize() == 8) {
+          if (returned_func == UINT64_MAX)
+            return false;
+        }
+        return true;
+      }
+    }
+  }
+
+  return false;
+}