[Lldb-commits] Unknown vendor on Linux (was 'RE: [PATCH] 32-bit target support on x86_64 Linux')
Kaylor, Andrew
andrew.kaylor at intel.com
Thu Nov 1 11:10:43 PDT 2012
Ping.
-----Original Message-----
From: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Kaylor, Andrew
Sent: Thursday, October 11, 2012 12:16 PM
To: Greg Clayton
Cc: lldb-commits at cs.uiuc.edu
Subject: Re: [Lldb-commits] Unknown vendor on Linux (was 'RE: [PATCH] 32-bit target support on x86_64 Linux')
Thanks, Greg.
Did you see my other patch, on Tuesday, related to the host OS and vendor string?
-Andy
-----Original Message-----
From: Greg Clayton [mailto:gclayton at apple.com]
Sent: Thursday, October 11, 2012 10:42 AM
To: Kaylor, Andrew
Cc: lldb-commits at cs.uiuc.edu
Subject: Re: [Lldb-commits] Unknown vendor on Linux (was 'RE: [PATCH] 32-bit target support on x86_64 Linux')
Looks good, committed in revision 165728.
On Oct 10, 2012, at 12:21 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
> Hi Greg,
>
> Have you had a chance to look at this patch?
>
> Thanks,
> Andy
>
> -----Original Message-----
> From: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Kaylor, Andrew
> Sent: Thursday, October 04, 2012 3:07 PM
> To: Greg Clayton
> Cc: lldb-commits at cs.uiuc.edu
> Subject: [Lldb-commits] Unknown vendor on Linux (was 'RE: [PATCH] 32-bit target support on x86_64 Linux')
>
> Hi Greg,
>
> I finally got around to revisiting this issue. It turns out that the 'x86_64-unknown-linux-gnu' triple is what the LLVM configure script puts in build/include/llvm/Config/config.h for LLVM_DEFAULT_TARGET_TRIPLE if no build target is explicitly passed to the configure command invocation. (FWIW, cmake puts 'x86_64-pc-linux-gnu' in the same place if nothing is specified.) I tracked it down to a line in autoconf/config.guess. It doesn't seem right, but I'm not comfortable mucking around with the base llvm configure process at that level. It also didn't feel right having a special case in the ArchSpec::operator==() code to handle it.
>
> What I did instead was to look at the place where llvm::sys::getDefaultTargetTriple() was being called (in common/Host.cpp) and put some code there to remove the vendor string if the OS is Linux and the vendor is reported as unknown. (See attached patch.) This makes the subsequent comparison pass as expected.
>
> Does this seem like a good solution to you?
>
> -Andy
>
> -----Original Message-----
> From: Greg Clayton [mailto:gclayton at apple.com]
> Sent: Tuesday, September 11, 2012 5:40 PM
> To: Kaylor, Andrew
> Cc: lldb-commits at cs.uiuc.edu
> Subject: Re: [Lldb-commits] [PATCH] 32-bit target support on x86_64 Linux
>
>
> On Sep 11, 2012, at 5:05 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
>
>> Yeah, the Linux-specific modification is OK. I'll do a little more poking to see if I can figure out how my default triple got to be so odd.
>
> Sounds good. Fixing the comparisong sounds like the right move IMHO.
>>
>> BTW, I have llvm commit access. Does that automatically confer lldb commit-after-review access upon me?
>
> Yep!
>
>> I appreciated your having expedited the commits from my recent changes, but I'm curious as to what my status is in this regard.
>
> Yep, we all review and then you can commit will work for us.
>
>>
>> -Andy
>>
>> -----Original Message-----
>> From: Greg Clayton [mailto:gclayton at apple.com]
>> Sent: Tuesday, September 11, 2012 4:24 PM
>> To: Kaylor, Andrew
>> Cc: lldb-commits at cs.uiuc.edu
>> Subject: Re: [Lldb-commits] [PATCH] 32-bit target support on x86_64 Linux
>>
>>
>> On Sep 7, 2012, at 2:32 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
>>
>>> Hi Greg,
>>>
>>> I wasn't sure about the "TripleVendorWasSpecified" part, and I'm still not sure I understand it. Let me tell you about how I got in the position of needing that change, and maybe you can tell me what I should have done to fix it correctly.
>>>
>>> I'm not sure exactly what led to this situation, but I think it has something to do with how I built lldb. When I run lldb on my 64-bit Linux system with a 64-bit app on the command line ("lldb myapp64"), in the course of PlatformLinux::ResolveExecutable execution a call is made to lldb_private::operator==(const ArchSpec& lhs, const ArchSpec& rhs). In this case the lhs triple (derived from llvm::sys::getDefaultTargetTriple()) is "x86_64-unknown-linux-gnu" and the rhs triple (based on examination of the executable) is "x86_64-gnu-linux".
>>>
>>> It seemed to me that these two should be reported as a match. This conclusion also seemed to be supported by the code in lldb_private::operator==(const ArchSpec& lhs, const ArchSpec& rhs), which looks like this (in part):
>>>
>>> if (lhs_triple_vendor != rhs_triple_vendor)
>>> {
>>> const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
>>> const bool lhs_vendor_specified = lhs.TripleVendorWasSpecified();
>>> // Both architectures had the vendor specified, so if they aren't
>>> // equal then we return false
>>> if (rhs_vendor_specified && lhs_vendor_specified)
>>> return false;
>>>
>>> // Only fail if both vendor types are not unknown
>>> if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
>>> rhs_triple_vendor != llvm::Triple::UnknownVendor)
>>> return false;
>>> }
>>>
>>> In my case the original implementation concluded that both rhs and lhs vendors were specified and so returned false before reaching the lines where the comment says "Only fail if both vendor types are not unknown."
>>>
>>> Based on this, I thought that the intention of the code was to treat the case where both vendors are specified but one is specified as "unknown" as a match, and so I also jumped to the conclusion (apparently incorrect) that "unknown" was a variety of "unspecified" (at least in the eyes of this code).
>>>
>>> The result of this case being reported as a non-match, BTW, was that lldb reported that it didn't have a matching platform for which to load the executable.
>>>
>>> It isn't clear to me how my system default target triple got to be "x86_64-unknown-linux-gnu". I don't recall specifying anything explicitly during the configure and build process.
>>>
>>> So, what's the correct fix here?
>>
>> It sounds like we need to verify if "llvm::sys::getDefaultTargetTriple()" is correct or not. If it is correct, then this is the triple that LLVM/clang are going to expect when we run expression so we would want to chang the object file parser triple which was based on examination of the executable to match llvm::sys::getDefaultTargetTriple().
>>
>> "x86_64-unknown-linux-gnu" just seems wrong. I just spoke with Daniel Dunbar on the clang team and he said "x86_64-pc-linux" or "x86_64-pc-linux-gnu" are fine defaults. Also for clang, they don't really do much with the vendor field for linux, so I would be ok with modifying the "lldb_private::operator==(const ArchSpec& lhs, const ArchSpec& rhs)" to ignore vendor when the OS is set to "linux". That would probably solve your problem with the least number of modification in LLDB. Does that sound ok?
>>
>> Greg
>>
>>>
>>> -Andy
>>>
>>> -----Original Message-----
>>> From: Greg Clayton [mailto:gclayton at apple.com]
>>> Sent: Friday, September 07, 2012 10:51 AM
>>> To: Kaylor, Andrew
>>> Cc: lldb-commits at cs.uiuc.edu
>>> Subject: Re: [Lldb-commits] [PATCH] 32-bit target support on x86_64 Linux
>>>
>>> Andrew:
>>>
>>> This patch looks ok except this part:
>>>
>>> Index: include/lldb/Core/ArchSpec.h
>>> ===================================================================
>>> --- include/lldb/Core/ArchSpec.h (revision 163394)
>>> +++ include/lldb/Core/ArchSpec.h (working copy)
>>> @@ -210,7 +210,10 @@
>>> bool
>>> TripleVendorWasSpecified() const
>>> {
>>> - return !m_triple.getVendorName().empty();
>>> + llvm::StringRef vendor = m_triple.getVendorName();
>>> + if (vendor.empty())
>>> + return false;
>>> + return vendor != "unknown";
>>> }
>>>
>>> bool
>>>
>>>
>>> When you give an llvm::Triple just a raw architecture like "x86_64", it will always return an "unknown" enumeration when asked what its vendor and OS is, but in LLDB we want to know when the user specified "unknown" explicity since we use this when looking for the right plug-in. There currently isn't a "none" enumeration or name that is accepted by the triple. So if the user currently specifies:
>>>
>>> (lldb) target create --arch x86_64-unknown-unknown /tmp/a.out
>>>
>>> We will know that the user specified "unknown" with:
>>>
>>> bool
>>> TripleVendorWasSpecified() const
>>> {
>>> return !m_triple.getVendorName().empty();
>>> }
>>>
>>> This causes no vendor or OS to be used when debugging and it is handy when you are debugging something on a bare board. It allows us to select the DynamicLoaderStatic to be used which just loads files at the addresses that are contained in the files themselves (absolute addressing).
>>>
>>> So the "bool ArchSpec::TripleVendorWasSpecified() const" is really trying to say "did the user type something for the vendor" (which inludes "unknown" as a valid thing the user typed). Does that make sense?
>>>
>>> I committed the other changes though:
>>>
>>> % svn commit
>>> Sending source/Host/common/Host.cpp
>>> Sending source/Plugins/Platform/Linux/PlatformLinux.cpp
>>> Sending source/Plugins/Process/Linux/ProcessMonitor.cpp
>>> Transmitting file data ...
>>> Committed revision 163398.
>>>
>>> On Sep 6, 2012, at 2:45 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
>>>
>>>> The attached patch adds support for debugging 32-bit processes when running a 64-bit lldb on an x86_64 Linux system.
>>>>
>>>> Making this work required two basic changes:
>>>>
>>>> 1) Getting lldb to report that it could debug 32-bit processes
>>>> 2) Changing an assumption about how ptrace works when debugging cross-platform
>>>>
>>>> For the first change, I took a conservative approach and only enabled this for x86_64 Linux platforms. It may be that the change I made in Host.cpp could be extended to other 64-bit Linux platforms, but I'm not familiar enough with the other platforms to know for sure.
>>>>
>>>> For the second change, the Linux ProcessMonitor class was assuming that ptrace(PTRACE_[PEEK|POKE]DATA...) would read/write a "word" based on the child process word size. However, the ptrace documentation says that the "word" size read or written is "determined by the OS variant." I verified experimentally that when ptracing a 32-bit child from a 64-bit parent a 64-bit word is read or written.
>>>>
>>>> This patch also modifies ArchSpec.h to recognize that "unknown" indicates an unspecified vendor choice. Whether or not this comes into play seems to depend on how you build lldb.
>>>>
>>>> -Andy
>>>> <linux-32-bit-support.patch>_______________________________________________
>>>> lldb-commits mailing list
>>>> lldb-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>
>>
>
_______________________________________________
lldb-commits mailing list
lldb-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
More information about the lldb-commits
mailing list