> On Oct 26, 2017, at 10:24 AM, Davide Italiano via lldb-commits > <lldb-commits@lists.llvm.org> wrote: > > On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits > <lldb-commits@lists.llvm.org> wrote: >> Author: sas >> Date: Thu Oct 26 10:04:20 2017 >> New Revision: 316673 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev >> Log: >> Allow SysV-i386 ABI on everything other than Apple targets >> >> Summary: >> This matches other SysV ABIs that are different on Apple and non-Apple >> targets, >> like `ABISysV_arm.cpp` for instance. >> >> Reviewers: clayborg, emaste >> >> Subscribers: aemerson, kristof.beyls, lldb-commits >> >> Differential Revision: https://reviews.llvm.org/D39335 >> >> Modified: >> lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp >> >> Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff >> ============================================================================== >> --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original) >> +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 >> 10:04:20 2017 >> @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3 >> 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().isOSFreeBSD())) { >> - if (!g_abi_sp) >> - g_abi_sp.reset(new ABISysV_i386(process_sp)); >> - return g_abi_sp; >> + if (arch.GetTriple().getVendor() != llvm::Triple::Apple) { >> + if (arch.GetTriple().getArch() == llvm::Triple::x86) { >> + if (!g_abi_sp) >> + g_abi_sp.reset(new ABISysV_i386(process_sp)); >> + return g_abi_sp; >> + } >> } >> return ABISP(); >> } >> > > This seems to change a fairly fundamental function for lldb-i386. > I think we should have an unit-test for this. Sorry for being > pedantic, I promise I'll stop after this one.
It's a good suggestion, and not something we test today. Right now there are two i386 ABIs that lldb supports: Darwin and SysV. This patch implements that correctly -- but the obvious problem is if a third i386 ABI is added in the future. Now it's a race to see whether SysV-i386 or CrazyOtherABI-i386 gets CreateInstance'd, depending on the order they're registered or something. And I'm not sure how you write a test today that would test a new target that uses CrazyOtherABI-i386 is getting the correct plugin activated. J _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
