Make i386-*-freebsd expression work on JIT path

[karnajitw]

Summary

Expression evaluation which takes the non IRInterpreter::Interpret path fails on i386-*-freebsd. Following changes are done to enable it.

1. Enable i386 ABI creation for freebsd
2. Added an extra argument in ABISysV_i386::PrepareTrivialCall for mmap syscall
3. Unlike linux, the last argument of mmap is actually 64-bit(off_t). This requires us to push an additional word for the higher order bits.
4. Prior to this change, ktrace dump will show mmap failures due to invalid argument coming from the 6th mmap argument.

Prior Fix:
(lldb) expression printf("%d", d1)
error: Can't run the expression locally: Interpreter doesn't handle one of the expression's opcodes
(lldb) expression d1 + 5 << IRInterpreter path
(int) $0 = 10
(lldb) expression d1 + 5.0
error: Can't run the expression locally: Interpreter doesn't handle one of the expression's opcodes
(lldb) expression f1 * 5
error: Can't run the expression locally: Interpreter doesn't handle one of the expression's opcodes
(lldb) expression f1 / 5
error: Can't run the expression locally: Interpreter doesn't handle one of the expression's opcodes

With Fix:
(lldb) expression printf("%d", d1)
(int) $0 = 1
(lldb) expression printf("%2.3f", f1)
(int) $2 = 5
(lldb) expression d1 + 5
(int) $3 = 10
(lldb) expression d1 + 5.0
(double) $4 = 10
(lldb) expression f1 * 5
(float) $7 = 25.5
(lldb) expression f1 / 5
(float) $8 = 1.01999998

NOTE: Expression involving floating point arithmetic will result in NaN on i386-*-freebsd11.0. Patch https://svnweb.freebsd.org/base?view=revision&revision=320051 is required to make it work.

[karnajitw]

Adding the corresponing reference discussions with freebsd folks.
mmap - https://lists.freebsd.org/pipermail/freebsd-hackers/2017-May/051072.html
x87 tag- https://lists.freebsd.org/pipermail/freebsd-hackers/2017-June/051166.html

[labath]

source/Plugins/Process/Utility/InferiorCallPOSIX.cpp

92 ↗

(On Diff #104495)

It would be great if we could avoid OS-specific code in this file. That's what we have tried to do with the Platform::ConvertMmapFlagsToPlatform call (line 82), but it looks like it may not have been the right abstraction.

How about we replace the ConvertMmapFlagsToPlatform function (it's only used in this place) with a more generic GetMmapArguments call (returning a vector of args)? It can still do the MAP_ANON dance as before, but in the freebsd case it will do this additional append.

What do you think?

[karnajitw]

Looks like the right thing to do. I will make the changes accordingly.

[karnajitw]

Done the changes. Please verify. Tried with RefArray but looks like they don't work well with copy.

[labath]

+ed as freebsd owner

ArrayRef cannot be returned from a function as they don't own the underlying data. The more traditional way of using SmallVector is to "return" it as a by-ref SmallVectorImpl<T> argument, as that avoids the need to hardcode the size in the function prototypes, but I suppose this will work as well...

[karnajitw]

With my test setup on i386-*-freebsd-11.0 (freebsd patched).. There seems to be quite a good improvement in the number of test failures.

[BEFORE lldb patch]

Test Result Summary

Test Methods: 1276
Reruns: 2
Success: 490
Expected Failure: 57
Failure: 61
Error: 2
Exceptional Exit: 1
Unexpected Success: 5
Skip: 658
Timeout: 2
Expected Timeout: 0

[After lldb patch]

Test Result Summary

Test Methods: 1276
Reruns: 0
Success: 545
Expected Failure: 55
Failure: 6
Error: 2
Exceptional Exit: 1
Unexpected Success: 7
Skip: 658
Timeout: 2
Expected Timeout: 0

[emaste]

Sorry I missed this when first uploaded, I will look at it shortly.

For future uploads can you kindly include full context, as described in https://llvm.org/docs/Phabricator.html?

[emaste]

The change in PlatformOpenBSD.cpp failed to apply for me (although it was trivial to manually apply it).

[emaste]

With this patch i386 test results are comparable to amd64, and I'm happy with it from a FreeBSD perspective (modulo the PlatformOpenBSD patch conflict).

[emaste]

I've committed this to FreeBSD's copy of lldb in r322326. @labath if you're happy with this patch I will commit to lldb for @karnajitw. I'm not sure how the patch ended up with a conflict, but it's just a whitespace issue.

[labath]

lgtm

lldb/trunk/include/lldb/Target/Platform.h

public:
   bool GetUseModuleCache() const;
   bool SetUseModuleCache(bool use_module_cache);
 
   FileSpec GetModuleCacheDirectory() const;
   bool SetModuleCacheDirectory(const FileSpec &dir_spec);
 };
 
 typedef std::shared_ptr<PlatformProperties> PlatformPropertiesSP;
+typedef llvm::SmallVector<lldb::addr_t, 6> MmapArgList;
 
 //----------------------------------------------------------------------
 /// @class Platform Platform.h "lldb/Target/Platform.h"
 /// @brief A plug-in interface definition class for debug platform that
 /// includes many platform abilities such as:
 ///     @li getting platform information such as supported architectures,
 ///         supported binary file formats and more
 ///     @li launching new processes
public:
   virtual Status Install(const FileSpec &src, const FileSpec &dst);
 
   virtual size_t GetEnvironment(StringList &environment);
 
   virtual bool GetFileExists(const lldb_private::FileSpec &file_spec);
 
   virtual Status Unlink(const FileSpec &file_spec);
 
-  virtual uint64_t ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                              unsigned flags);
+  virtual MmapArgList GetMmapArgumentList(const ArchSpec &arch,
+                                          lldb::addr_t addr,
+                                          lldb::addr_t length,
+                                          unsigned prot, unsigned flags,
+                                          lldb::addr_t fd, lldb::addr_t offset);
 
   virtual bool GetSupportsRSync() { return m_supports_rsync; }
 
   virtual void SetSupportsRSync(bool flag) { m_supports_rsync = flag; }
 
   virtual const char *GetRSyncOpts() { return m_rsync_opts.c_str(); }
 
   virtual void SetRSyncOpts(const char *opts) { m_rsync_opts.assign(opts); }

lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp

 //------------------------------------------------------------------
 // Static Functions
 //------------------------------------------------------------------
 
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   static ABISP g_abi_sp;
   if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-      arch.GetTriple().isOSLinux()) {
+      (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
     if (!g_abi_sp)
       g_abi_sp.reset(new ABISysV_i386(process_sp));
     return g_abi_sp;
   }
   return ABISP();
 }
 
 bool ABISysV_i386::PrepareTrivialCall(Thread &thread, addr_t sp,

lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h

public:
 
   Status LaunchProcess(ProcessLaunchInfo &launch_info) override;
 
   lldb::ProcessSP Attach(ProcessAttachInfo &attach_info, Debugger &debugger,
                          Target *target, Status &error) override;
 
   void CalculateTrapHandlerSymbolNames() override;
 
-  uint64_t ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                      unsigned flags) override;
+  MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
+                                  lldb::addr_t length, unsigned prot,
+                                  unsigned flags, lldb::addr_t fd,
+                                  lldb::addr_t offset) override;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformFreeBSD);
 };
 
 } // namespace platform_freebsd
 } // namespace lldb_private
 
 #endif // liblldb_PlatformFreeBSD_h_

lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp

 bool PlatformFreeBSD::CanDebugProcess() {
   return false;
 }
 
 void PlatformFreeBSD::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
-uint64_t PlatformFreeBSD::ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                                     unsigned flags) {
+MmapArgList PlatformFreeBSD::GetMmapArgumentList(const ArchSpec &arch,
+                                                 addr_t addr, addr_t length,
+                                                 unsigned prot, unsigned flags,
+                                                 addr_t fd, addr_t offset) {
   uint64_t flags_platform = 0;
 
   if (flags & eMmapFlagsPrivate)
     flags_platform |= MAP_PRIVATE;
   if (flags & eMmapFlagsAnon)
     flags_platform |= MAP_ANON;
-  return flags_platform;
+
+  MmapArgList args({addr, length, prot, flags_platform, fd, offset});
+  if (arch.GetTriple().getArch() == llvm::Triple::x86)
+    args.push_back(0);
+  return args;
 }

lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h

public:
   bool CanDebugProcess() override;
 
   lldb::ProcessSP DebugProcess(ProcessLaunchInfo &launch_info,
                                Debugger &debugger, Target *target,
                                Status &error) override;
 
   void CalculateTrapHandlerSymbolNames() override;
 
-  uint64_t ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                      unsigned flags) override;
+  MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
+                                  lldb::addr_t length, unsigned prot,
+                                  unsigned flags, lldb::addr_t fd,
+                                  lldb::addr_t offset) override;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformLinux);
 };
 
 } // namespace platform_linux
 } // namespace lldb_private
 
 #endif // liblldb_PlatformLinux_h_

lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp

PlatformLinux::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
 
   return process_sp;
 }
 
 void PlatformLinux::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
-uint64_t PlatformLinux::ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                                   unsigned flags) {
+MmapArgList PlatformLinux::GetMmapArgumentList(const ArchSpec &arch,
+                                               addr_t addr, addr_t length,
+                                               unsigned prot, unsigned flags,
+                                               addr_t fd, addr_t offset) {
   uint64_t flags_platform = 0;
   uint64_t map_anon = MAP_ANON;
 
   // To get correct flags for MIPS Architecture
   if (arch.GetTriple().getArch() == llvm::Triple::mips64 ||
       arch.GetTriple().getArch() == llvm::Triple::mips64el ||
       arch.GetTriple().getArch() == llvm::Triple::mips ||
       arch.GetTriple().getArch() == llvm::Triple::mipsel)
     map_anon = 0x800;
 
   if (flags & eMmapFlagsPrivate)
     flags_platform |= MAP_PRIVATE;
   if (flags & eMmapFlagsAnon)
     flags_platform |= map_anon;
-  return flags_platform;
+
+  MmapArgList args({addr, length, prot, flags_platform, fd, offset});
+  return args;
 }
 

lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.h

public:
   bool CanDebugProcess() override;
 
   lldb::ProcessSP DebugProcess(ProcessLaunchInfo &launch_info,
                                Debugger &debugger, Target *target,
                                Status &error) override;
 
   void CalculateTrapHandlerSymbolNames() override;
 
-  uint64_t ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                      unsigned flags) override;
+  MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
+                                  lldb::addr_t length, unsigned prot,
+                                  unsigned flags, lldb::addr_t fd,
+                                  lldb::addr_t offset) override;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformNetBSD);
 };
 
 } // namespace platform_netbsd
 } // namespace lldb_private
 
 #endif // liblldb_PlatformNetBSD_h_

lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp

lldb::ProcessSP PlatformNetBSD::DebugProcess(
 
   return process_sp;
 }
 
 void PlatformNetBSD::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
-uint64_t PlatformNetBSD::ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                                   unsigned flags) {
+MmapArgList PlatformNetBSD::GetMmapArgumentList(const ArchSpec &arch,
+                                                addr_t addr, addr_t length,
+                                                unsigned prot, unsigned flags,
+                                                addr_t fd, addr_t offset) {
   uint64_t flags_platform = 0;
 
   if (flags & eMmapFlagsPrivate)
     flags_platform |= MAP_PRIVATE;
   if (flags & eMmapFlagsAnon)
     flags_platform |= MAP_ANON;
-  return flags_platform;
+
+  MmapArgList args({addr, length, prot, flags_platform, fd, offset});
+  return args;
 }

lldb/trunk/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.h

public:
   void GetStatus(Stream &strm) override;
 
   bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
 
   bool CanDebugProcess() override;
 
   void CalculateTrapHandlerSymbolNames() override;
 
-  uint64_t ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                      unsigned flags) override;
+  MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
+                                  lldb::addr_t length, unsigned prot,
+                                  unsigned flags, lldb::addr_t fd,
+                                  lldb::addr_t offset) override;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformOpenBSD);
 };
 
 } // namespace platform_openbsd
 } // namespace lldb_private
 
 #endif // liblldb_PlatformOpenBSD_h_

lldb/trunk/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp

 bool PlatformOpenBSD::CanDebugProcess() {
   return false;
 }
 
 void PlatformOpenBSD::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
-uint64_t PlatformOpenBSD::ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                 unsigned flags) {
+MmapArgList PlatformOpenBSD::GetMmapArgumentList(const ArchSpec &arch,
+                                                 addr_t addr, addr_t length,
+                                                 unsigned prot, unsigned flags,
+                                                 addr_t fd, addr_t offset) {
   uint64_t flags_platform = 0;
 
   if (flags & eMmapFlagsPrivate)
     flags_platform |= MAP_PRIVATE;
   if (flags & eMmapFlagsAnon)
     flags_platform |= MAP_ANON;
-  return flags_platform;
+
+  MmapArgList args({addr, length, prot, flags_platform, fd, offset});
+  return args;
 }

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

if (sc_list.GetContextAtIndex(0, sc)) {
       options.SetStopOthers(true);
       options.SetUnwindOnError(true);
       options.SetIgnoreBreakpoints(true);
       options.SetTryAllThreads(true);
       options.SetDebug(false);
       options.SetTimeout(std::chrono::milliseconds(500));
       options.SetTrapExceptions(false);
 
-      addr_t prot_arg, flags_arg = 0;
+      addr_t prot_arg;
       if (prot == eMmapProtNone)
         prot_arg = PROT_NONE;
       else {
         prot_arg = 0;
         if (prot & eMmapProtExec)
           prot_arg |= PROT_EXEC;
         if (prot & eMmapProtRead)
           prot_arg |= PROT_READ;
         if (prot & eMmapProtWrite)
           prot_arg |= PROT_WRITE;
       }
 
-      const ArchSpec arch = process->GetTarget().GetArchitecture();
-      flags_arg =
-          process->GetTarget().GetPlatform()->ConvertMmapFlagsToPlatform(arch,
-                                                                         flags);
-
       AddressRange mmap_range;
       if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
                              mmap_range)) {
         ClangASTContext *clang_ast_context =
             process->GetTarget().GetScratchClangASTContext();
         CompilerType clang_void_ptr_type =
             clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
-        lldb::addr_t args[] = {addr, length, prot_arg, flags_arg, fd, offset};
+        const ArchSpec arch = process->GetTarget().GetArchitecture();
+        MmapArgList args =
+            process->GetTarget().GetPlatform()->GetMmapArgumentList(
+                arch, addr, length, prot_arg, flags, fd, offset);
         lldb::ThreadPlanSP call_plan_sp(
             new ThreadPlanCallFunction(*thread, mmap_range.GetBaseAddress(),
                                        clang_void_ptr_type, args, options));
         if (call_plan_sp) {
           DiagnosticManager diagnostics;
 
           StackFrame *frame = thread->GetStackFrameAtIndex(0).get();
           if (frame) {

lldb/trunk/source/Target/Platform.cpp

bool Platform::GetFileExists(const lldb_private::FileSpec &file_spec) {
   return false;
 }
 
 Status Platform::Unlink(const FileSpec &path) {
   Status error("unimplemented");
   return error;
 }
 
-uint64_t Platform::ConvertMmapFlagsToPlatform(const ArchSpec &arch,
-                                              unsigned flags) {
+MmapArgList Platform::GetMmapArgumentList(const ArchSpec &arch, addr_t addr,
+                                          addr_t length, unsigned prot,
+                                          unsigned flags, addr_t fd,
+                                          addr_t offset) {
   uint64_t flags_platform = 0;
   if (flags & eMmapFlagsPrivate)
     flags_platform |= MAP_PRIVATE;
   if (flags & eMmapFlagsAnon)
     flags_platform |= MAP_ANON;
-  return flags_platform;
+
+  MmapArgList args({addr, length, prot, flags_platform, fd, offset});
+  return args;
 }
 
 lldb_private::Status Platform::RunShellCommand(
     const char *command, // Shouldn't be nullptr
     const FileSpec &
         working_dir, // Pass empty FileSpec to use the current working directory
     int *status_ptr, // Pass nullptr if you don't want the process exit status
     int *signo_ptr, // Pass nullptr if you don't want the signal that caused the