Update the code to the new world code.
These changes are needed for remote process plugin.
Sponsored by <The NetBSD Foundation>
Differential D29266
Synchronize PlatformNetBSD with Linux krytarowski on Jan 29 2017, 3:16 PM. Authored by
Details Update the code to the new world code. These changes are needed for remote process plugin. Sponsored by <The NetBSD Foundation>
Diff Detail
Event TimelineComment Actions How much of the code is now actually different between the two classes? If the only changes are of the s/linux/netbsd type, then we should just create a new base class for the two, and pass the platform name in as a parameter. Comment Actions Diff diff -u source/Plugins/Platform/Linux/PlatformLinux.cpp source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp Mostly everything is the same for now. --- source/Plugins/Platform/Linux/PlatformLinux.cpp 2016-12-19 17:48:18.156356172 +0100 +++ source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp 2017-01-31 16:48:56.313379692 +0100 @@ -1,4 +1,4 @@ -//===-- PlatformLinux.cpp ---------------------------------------*- C++ -*-===// +//===-- PlatformNetBSD.cpp -------------------------------------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// -#include "PlatformLinux.h" +#include "PlatformNetBSD.h" #include "lldb/Host/Config.h" // C Includes @@ -36,27 +36,27 @@ #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" -// Define these constants from Linux mman.h for use when targeting +// Define these constants from NetBSD mman.h for use when targeting // remote linux systems even when host has different values. -#define MAP_PRIVATE 2 -#define MAP_ANON 0x20 +#define MAP_PRIVATE 0x0002 +#define MAP_ANON 0x1000 using namespace lldb; using namespace lldb_private; -using namespace lldb_private::platform_linux; +using namespace lldb_private::platform_netbsd; static uint32_t g_initialize_count = 0; //------------------------------------------------------------------ -/// Code to handle the PlatformLinux settings +/// Code to handle the PlatformNetBSD settings //------------------------------------------------------------------ namespace { -class PlatformLinuxProperties : public Properties { +class PlatformNetBSDProperties : public Properties { public: - PlatformLinuxProperties(); + PlatformNetBSDProperties(); - ~PlatformLinuxProperties() override = default; + ~PlatformNetBSDProperties() override = default; static ConstString &GetSettingName(); @@ -64,49 +64,49 @@ static const PropertyDefinition *GetStaticPropertyDefinitions(); }; -typedef std::shared_ptr<PlatformLinuxProperties> PlatformLinuxPropertiesSP; +typedef std::shared_ptr<PlatformNetBSDProperties> PlatformNetBSDPropertiesSP; } // anonymous namespace -PlatformLinuxProperties::PlatformLinuxProperties() : Properties() { +PlatformNetBSDProperties::PlatformNetBSDProperties() : Properties() { m_collection_sp.reset(new OptionValueProperties(GetSettingName())); m_collection_sp->Initialize(GetStaticPropertyDefinitions()); } -ConstString &PlatformLinuxProperties::GetSettingName() { - static ConstString g_setting_name("linux"); +ConstString &PlatformNetBSDProperties::GetSettingName() { + static ConstString g_setting_name("netbsd"); return g_setting_name; } const PropertyDefinition * -PlatformLinuxProperties::GetStaticPropertyDefinitions() { +PlatformNetBSDProperties::GetStaticPropertyDefinitions() { static PropertyDefinition g_properties[] = { {NULL, OptionValue::eTypeInvalid, false, 0, NULL, NULL, NULL}}; return g_properties; } -static const PlatformLinuxPropertiesSP &GetGlobalProperties() { - static PlatformLinuxPropertiesSP g_settings_sp; +static const PlatformNetBSDPropertiesSP &GetGlobalProperties() { + static PlatformNetBSDPropertiesSP g_settings_sp; if (!g_settings_sp) - g_settings_sp.reset(new PlatformLinuxProperties()); + g_settings_sp.reset(new PlatformNetBSDProperties()); return g_settings_sp; } -void PlatformLinux::DebuggerInitialize(Debugger &debugger) { +void PlatformNetBSD::DebuggerInitialize(Debugger &debugger) { if (!PluginManager::GetSettingForPlatformPlugin( - debugger, PlatformLinuxProperties::GetSettingName())) { + debugger, PlatformNetBSDProperties::GetSettingName())) { const bool is_global_setting = true; PluginManager::CreateSettingForPlatformPlugin( debugger, GetGlobalProperties()->GetValueProperties(), - ConstString("Properties for the PlatformLinux plug-in."), + ConstString("Properties for the PlatformNetBSD plug-in."), is_global_setting); } } //------------------------------------------------------------------ -PlatformSP PlatformLinux::CreateInstance(bool force, const ArchSpec *arch) { +PlatformSP PlatformNetBSD::CreateInstance(bool force, const ArchSpec *arch) { Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM)); if (log) { const char *arch_name; @@ -118,7 +118,7 @@ const char *triple_cstr = arch ? arch->GetTriple().getTriple().c_str() : "<null>"; - log->Printf("PlatformLinux::%s(force=%s, arch={%s,%s})", __FUNCTION__, + log->Printf("PlatformNetBSD::%s(force=%s, arch={%s,%s})", __FUNCTION__, force ? "true" : "false", arch_name, triple_cstr); } @@ -126,18 +126,10 @@ if (create == false && arch && arch->IsValid()) { const llvm::Triple &triple = arch->GetTriple(); switch (triple.getOS()) { - case llvm::Triple::Linux: + case llvm::Triple::NetBSD: create = true; break; -#if defined(__linux__) - // Only accept "unknown" for the OS if the host is linux and - // it "unknown" wasn't specified (it was just returned because it - // was NOT specified) - case llvm::Triple::OSType::UnknownOS: - create = !arch->TripleOSWasSpecified(); - break; -#endif default: break; } @@ -145,67 +137,67 @@ if (create) { if (log) - log->Printf("PlatformLinux::%s() creating remote-linux platform", + log->Printf("PlatformNetBSD::%s() creating remote-netbsd platform", __FUNCTION__); - return PlatformSP(new PlatformLinux(false)); + return PlatformSP(new PlatformNetBSD(false)); } if (log) log->Printf( - "PlatformLinux::%s() aborting creation of remote-linux platform", + "PlatformNetBSD::%s() aborting creation of remote-netbsd platform", __FUNCTION__); return PlatformSP(); } -ConstString PlatformLinux::GetPluginNameStatic(bool is_host) { +ConstString PlatformNetBSD::GetPluginNameStatic(bool is_host) { if (is_host) { static ConstString g_host_name(Platform::GetHostPlatformName()); return g_host_name; } else { - static ConstString g_remote_name("remote-linux"); + static ConstString g_remote_name("remote-netbsd"); return g_remote_name; } } -const char *PlatformLinux::GetPluginDescriptionStatic(bool is_host) { +const char *PlatformNetBSD::GetPluginDescriptionStatic(bool is_host) { if (is_host) - return "Local Linux user platform plug-in."; + return "Local NetBSD user platform plug-in."; else - return "Remote Linux user platform plug-in."; + return "Remote NetBSD user platform plug-in."; } -ConstString PlatformLinux::GetPluginName() { +ConstString PlatformNetBSD::GetPluginName() { return GetPluginNameStatic(IsHost()); } -void PlatformLinux::Initialize() { +void PlatformNetBSD::Initialize() { PlatformPOSIX::Initialize(); if (g_initialize_count++ == 0) { -#if defined(__linux__) && !defined(__ANDROID__) - PlatformSP default_platform_sp(new PlatformLinux(true)); +#if defined(__NetBSD__) + PlatformSP default_platform_sp(new PlatformNetBSD(true)); default_platform_sp->SetSystemArchitecture(HostInfo::GetArchitecture()); Platform::SetHostPlatform(default_platform_sp); #endif PluginManager::RegisterPlugin( - PlatformLinux::GetPluginNameStatic(false), - PlatformLinux::GetPluginDescriptionStatic(false), - PlatformLinux::CreateInstance, PlatformLinux::DebuggerInitialize); + PlatformNetBSD::GetPluginNameStatic(false), + PlatformNetBSD::GetPluginDescriptionStatic(false), + PlatformNetBSD::CreateInstance, PlatformNetBSD::DebuggerInitialize); } } -void PlatformLinux::Terminate() { +void PlatformNetBSD::Terminate() { if (g_initialize_count > 0) { if (--g_initialize_count == 0) { - PluginManager::UnregisterPlugin(PlatformLinux::CreateInstance); + PluginManager::UnregisterPlugin(PlatformNetBSD::CreateInstance); } } PlatformPOSIX::Terminate(); } -Error PlatformLinux::ResolveExecutable( +Error PlatformNetBSD::ResolveExecutable( const ModuleSpec &ms, lldb::ModuleSP &exe_module_sp, const FileSpecList *module_search_paths_ptr) { Error error; @@ -330,7 +322,7 @@ return error; } -Error PlatformLinux::GetFileWithUUID(const FileSpec &platform_file, +Error PlatformNetBSD::GetFileWithUUID(const FileSpec &platform_file, const UUID *uuid_ptr, FileSpec &local_file) { if (IsRemote()) { @@ -347,7 +339,7 @@ //------------------------------------------------------------------ /// Default Constructor //------------------------------------------------------------------ -PlatformLinux::PlatformLinux(bool is_host) +PlatformNetBSD::PlatformNetBSD(bool is_host) : PlatformPOSIX(is_host) // This is the local host platform {} @@ -357,9 +349,9 @@ /// The destructor is virtual since this class is designed to be /// inherited from by the plug-in instance. //------------------------------------------------------------------ -PlatformLinux::~PlatformLinux() = default; +PlatformNetBSD::~PlatformNetBSD() = default; -bool PlatformLinux::GetProcessInfo(lldb::pid_t pid, +bool PlatformNetBSD::GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &process_info) { bool success = false; if (IsHost()) { @@ -372,8 +364,8 @@ } uint32_t -PlatformLinux::FindProcesses(const ProcessInstanceInfoMatch &match_info, - ProcessInstanceInfoList &process_infos) { +PlatformNetBSD::FindProcesses(const ProcessInstanceInfoMatch &match_info, + ProcessInstanceInfoList &process_infos) { uint32_t match_count = 0; if (IsHost()) { // Let the base class figure out the host details @@ -387,11 +379,11 @@ return match_count; } -bool PlatformLinux::GetSupportedArchitectureAtIndex(uint32_t idx, - ArchSpec &arch) { +bool PlatformNetBSD::GetSupportedArchitectureAtIndex(uint32_t idx, + ArchSpec &arch) { if (IsHost()) { ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault); - if (hostArch.GetTriple().isOSLinux()) { + if (hostArch.GetTriple().isOSNetBSD()) { if (idx == 0) { arch = hostArch; return arch.IsValid(); @@ -408,8 +400,8 @@ return m_remote_platform_sp->GetSupportedArchitectureAtIndex(idx, arch); llvm::Triple triple; - // Set the OS to linux - triple.setOS(llvm::Triple::Linux); + // Set the OS to NetBSD + triple.setOS(llvm::Triple::NetBSD); // Set the architecture switch (idx) { case 0: @@ -418,30 +410,6 @@ case 1: triple.setArchName("i386"); break; - case 2: - triple.setArchName("arm"); - break; - case 3: - triple.setArchName("aarch64"); - break; - case 4: - triple.setArchName("mips64"); - break; - case 5: - triple.setArchName("hexagon"); - break; - case 6: - triple.setArchName("mips"); - break; - case 7: - triple.setArchName("mips64el"); - break; - case 8: - triple.setArchName("mipsel"); - break; - case 9: - triple.setArchName("s390x"); - break; default: return false; } @@ -461,12 +429,12 @@ return false; } -void PlatformLinux::GetStatus(Stream &strm) { +void PlatformNetBSD::GetStatus(Stream &strm) { Platform::GetStatus(strm); #ifndef LLDB_DISABLE_POSIX // Display local kernel information only when we are running in host mode. - // Otherwise, we would end up printing non-Linux information (when running + // Otherwise, we would end up printing non-NetBSD information (when running // on Mac OS for example). if (IsHost()) { struct utsname un; @@ -482,7 +450,7 @@ } int32_t -PlatformLinux::GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) { +PlatformNetBSD::GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) { int32_t resume_count = 0; // Always resume past the initial stop when we use eLaunchFlagDebug @@ -516,7 +484,7 @@ return resume_count; } -bool PlatformLinux::CanDebugProcess() { +bool PlatformNetBSD::CanDebugProcess() { if (IsHost()) { return true; } else { @@ -525,19 +493,19 @@ } } -// For local debugging, Linux will override the debug logic to use llgs-launch +// For local debugging, NetBSD will override the debug logic to use llgs-launch // rather than // lldb-launch, llgs-attach. This differs from current lldb-launch, // debugserver-attach // approach on MacOSX. lldb::ProcessSP -PlatformLinux::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, +PlatformNetBSD::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, Target *target, // Can be NULL, if NULL create a new // target, else use existing one Error &error) { Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM)); if (log) - log->Printf("PlatformLinux::%s entered (target %p)", __FUNCTION__, + log->Printf("PlatformNetBSD::%s entered (target %p)", __FUNCTION__, static_cast<void *>(target)); // If we're a remote host, use standard behavior from parent class. @@ -562,14 +530,14 @@ // Ensure we have a target. if (target == nullptr) { if (log) - log->Printf("PlatformLinux::%s creating new target", __FUNCTION__); + log->Printf("PlatformNetBSD::%s creating new target", __FUNCTION__); TargetSP new_target_sp; error = debugger.GetTargetList().CreateTarget(debugger, "", "", false, nullptr, new_target_sp); if (error.Fail()) { if (log) - log->Printf("PlatformLinux::%s failed to create new target: %s", + log->Printf("PlatformNetBSD::%s failed to create new target: %s", __FUNCTION__, error.AsCString()); return process_sp; } @@ -578,13 +546,13 @@ if (!target) { error.SetErrorString("CreateTarget() returned nullptr"); if (log) - log->Printf("PlatformLinux::%s failed: %s", __FUNCTION__, + log->Printf("PlatformNetBSD::%s failed: %s", __FUNCTION__, error.AsCString()); return process_sp; } } else { if (log) - log->Printf("PlatformLinux::%s using provided target", __FUNCTION__); + log->Printf("PlatformNetBSD::%s using provided target", __FUNCTION__); } // Mark target as currently selected target. @@ -593,7 +561,7 @@ // Now create the gdb-remote process. if (log) log->Printf( - "PlatformLinux::%s having target create process with gdb-remote plugin", + "PlatformNetBSD::%s having target create process with gdb-remote plugin", __FUNCTION__); process_sp = target->CreateProcess( launch_info.GetListenerForProcess(debugger), "gdb-remote", nullptr); @@ -601,12 +569,12 @@ if (!process_sp) { error.SetErrorString("CreateProcess() failed for gdb-remote process"); if (log) - log->Printf("PlatformLinux::%s failed: %s", __FUNCTION__, + log->Printf("PlatformNetBSD::%s failed: %s", __FUNCTION__, error.AsCString()); return process_sp; } else { if (log) - log->Printf("PlatformLinux::%s successfully created process", + log->Printf("PlatformNetBSD::%s successfully created process", __FUNCTION__); } @@ -614,10 +582,10 @@ ListenerSP listener_sp; if (!launch_info.GetHijackListener()) { if (log) - log->Printf("PlatformLinux::%s setting up hijacker", __FUNCTION__); + log->Printf("PlatformNetBSD::%s setting up hijacker", __FUNCTION__); listener_sp = - Listener::MakeListener("lldb.PlatformLinux.DebugProcess.hijack"); + Listener::MakeListener("lldb.PlatformNetBSD.DebugProcess.hijack"); launch_info.SetHijackListener(listener_sp); process_sp->HijackProcessEvents(listener_sp); } @@ -625,7 +593,7 @@ // Log file actions. if (log) { log->Printf( - "PlatformLinux::%s launching process with the following file actions:", + "PlatformNetBSD::%s launching process with the following file actions:", __FUNCTION__); StreamString stream; @@ -648,11 +616,11 @@ if (state == eStateStopped) { if (log) - log->Printf("PlatformLinux::%s pid %" PRIu64 " state %s\n", + log->Printf("PlatformNetBSD::%s pid %" PRIu64 " state %s\n", __FUNCTION__, process_sp->GetID(), StateAsCString(state)); } else { if (log) - log->Printf("PlatformLinux::%s pid %" PRIu64 + log->Printf("PlatformNetBSD::%s pid %" PRIu64 " state is not stopped - %s\n", __FUNCTION__, process_sp->GetID(), StateAsCString(state)); } @@ -664,18 +632,18 @@ if (pty_fd != lldb_utility::PseudoTerminal::invalid_fd) { process_sp->SetSTDIOFileDescriptor(pty_fd); if (log) - log->Printf("PlatformLinux::%s pid %" PRIu64 + log->Printf("PlatformNetBSD::%s pid %" PRIu64 " hooked up STDIO pty to process", __FUNCTION__, process_sp->GetID()); } else { if (log) - log->Printf("PlatformLinux::%s pid %" PRIu64 + log->Printf("PlatformNetBSD::%s pid %" PRIu64 " not using process STDIO pty", __FUNCTION__, process_sp->GetID()); } } else { if (log) - log->Printf("PlatformLinux::%s process launch failed: %s", __FUNCTION__, + log->Printf("PlatformNetBSD::%s process launch failed: %s", __FUNCTION__, error.AsCString()); // FIXME figure out appropriate cleanup here. Do we delete the target? Do // we delete the process? Does our caller do that? @@ -684,30 +652,22 @@ return process_sp; } -void PlatformLinux::CalculateTrapHandlerSymbolNames() { +void PlatformNetBSD::CalculateTrapHandlerSymbolNames() { m_trap_handlers.push_back(ConstString("_sigtramp")); } -uint64_t PlatformLinux::ConvertMmapFlagsToPlatform(const ArchSpec &arch, +uint64_t PlatformNetBSD::ConvertMmapFlagsToPlatform(const ArchSpec &arch, unsigned flags) { 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; + flags_platform |= MAP_ANON; return flags_platform; } -ConstString PlatformLinux::GetFullNameForDylib(ConstString basename) { +ConstString PlatformNetBSD::GetFullNameForDylib(ConstString basename) { if (basename.IsEmpty()) return basename; and --- source/Plugins/Platform/Linux/PlatformLinux.h 2016-09-10 20:16:06.905683528 +0200 +++ source/Plugins/Platform/NetBSD/PlatformNetBSD.h 2017-01-31 16:48:56.332198593 +0100 @@ -1,4 +1,4 @@ -//===-- PlatformLinux.h -----------------------------------------*- C++ -*-===// +//===-- PlatformNetBSD.h ----------------------------------------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -7,8 +7,8 @@ // //===----------------------------------------------------------------------===// -#ifndef liblldb_PlatformLinux_h_ -#define liblldb_PlatformLinux_h_ +#ifndef liblldb_PlatformNetBSD_h_ +#define liblldb_PlatformNetBSD_h_ // C Includes // C++ Includes @@ -17,13 +17,13 @@ #include "Plugins/Platform/POSIX/PlatformPOSIX.h" namespace lldb_private { -namespace platform_linux { +namespace platform_netbsd { -class PlatformLinux : public PlatformPOSIX { +class PlatformNetBSD : public PlatformPOSIX { public: - PlatformLinux(bool is_host); + PlatformNetBSD(bool is_host); - ~PlatformLinux() override; + ~PlatformNetBSD() override; static void DebuggerInitialize(Debugger &debugger); @@ -83,10 +83,10 @@ ConstString GetFullNameForDylib(ConstString basename) override; private: - DISALLOW_COPY_AND_ASSIGN(PlatformLinux); + DISALLOW_COPY_AND_ASSIGN(PlatformNetBSD); }; -} // namespace platform_linux +} // namespace platform_netbsd } // namespace lldb_private -#endif // liblldb_PlatformLinux_h_ +#endif // liblldb_PlatformNetBSD_h_ How would you like to enhance it? How to name the base class, where to put it? Reuse C++ class inheritance? Comment Actions After looking at this closer, I don't think we will even need a separate class to achieve deduplication. The code is so heavily duplicated (see comments on the first three non-boilerplate functions, I did not look at the rest yet, but I expect the situation to be the same) already that we can just push the definitions down the existing class hierarchy and achieve massive savings that way. After we're done with that, I think we'll find that the PlatformLinux class will be so small that your current s/linux/netbsd/ approach will no longer be a problem. If @clayborg agrees with this approach, I can help you prepare the ground for that.
Comment Actions I'm looking forward to this approach. In the past NetBSD code was almost exact copy of the FreeBSD one. Once we will deduplicate the code, FreeBSD will catch up. Comment Actions I need to finish three patches:
and I will join here. After that, I will switch the PT_WATCHPOINT* interface to PT_GETDBREGS and PT_SETDBREGS -- and in the end, add dbregs support in NetBSD's userdata. Later on, I will need to finish few tasks on the NetBSD side (thread suspend/resume; detect simple false positives in the check-lldb target). Comment Actions Sync this patch with SVN 294071. It is to be used at least for a reference in further code sharing. Comment Actions Just sync once more please and I think we're done. We've removed enough code to offset the remaining bit of duplication here. DebugProcess is the only remaining candidate for merging, but we don't have a good class for it now. Maybe when Freebsd switches to lldb-server we can put it in processposix. Comment Actions I'm syncing the code to get the changes from SVN revision 294114 "Clean up PlatformLinux code" Comment Actions OK to land it? FreeBSD can reuse almost exact the same code. I'm not sure if it's compatible with their Process Plugin. |
PlatformLinux, PlatformFreeBSD and PlatformWindows have copies of this code already. Proposal: