Re: [Lldb-commits] [lldb] r221213 - Fixed SBTarget::ReadMemory() to work correctly and the TestTargetAPI.py test case that was reading target memory in TargetAPITestCase.test_read_memory_with_dsym and

[Matthew Gardiner]

Hi Greg,

Apologies for my sluggishness, but I've been off lldb for a bit,
contributing to an in-house project.

Thanks for clarifying that a virtual address in my world is the same
as a load address in lldb's terminology.

What puzzles me is why the invocation of:
 
a = target.ResolveFileAddress(data_section_addr)

was removed in your recent change, since in that change, I had been
attempting to associate the raw address with a file section.

Did this fail to resolve to a section/offset based address?   

Matt

On Tue, 2014-11-04 at 10:18 -0800, Greg Clayton wrote:
  On Nov 3, 2014, at 10:53 PM, Matthew Gardiner m...@csr.com wrote:
  
  Hi Greg,
  
  So what in lldb's world is the difference between a file and a load
  address?
 
 
 file address (as LLDB treats them) is the address that is found in the 
 object file (ELF, MachO, or COFF). load address is the actual address in 
 the process where the section is loaded after being slid.
 
 For example you might have a shared library that has a function foo whose 
 file address is 0x1000, but when the shared library gets loaded into memory, 
 it will be loaded at a different address because all shared libraries have 
 functions in the low file address range (say from 0 to 0x40 for example). 
 So if the shared library gets loaded with a slide of 0x100, foo will have 
 a load address of 0x100 + 0x1000.
 
 Now for most embedded debugging where there is no OS that will slide things 
 around, your file and load addresses will match. For actual OS level 
 debugging, they won't for shared libraries, and might for the main 
 executable. Sometimes the main executable doesn't get slid around, but other 
 OSs will use ASLR to slide the main executables around for security.
 
 The test that was written was trying to get a data section from the object 
 file. Then it made a section offset address that pointed to that data 
 section. Then it called SBTarget::ReadMemory(...) with that section offset 
 address. What was happening on MacOSX was:
 
 - get the data section whose file address was 0x10001000
 - Make a section + offset address from it that was represented as a.out's 
 data section + 0 bytes
 - call target read memory
 
 Prior to my fix this happened:
 
 SBTarget::ReadMemory() made a new section offset address:
 
 Address address(section_offset_addr.GetFileAddress(), NULL);
 
 Now we have an address that has no section. If such an address is passed to 
 anything that is trying to read memory from a live process, an address with 
 no section is considered to be a load address. It will try and read from 
 the load address 0x10001000. But on MacOSX, or any OS with ASLR, the data 
 section was slid by a random amount (like 0xef). So we would try to read 
 from load address 0x10001000 and it would fail. If we leave the address 
 object as a section offset address (don't make a new address like we did 
 above), we pass this address to a read memory function and it will resolve 
 the section offset address into an address in the live process, or into a 
 load address. This will be 0x10001000 + 0xef + 0. The load address of the 
 data section is 0x10001000 + 0xef and the offset was 0. And the resulting 
 memory it will read from in the process is 0x10ef1000. The old way it would 
 have tried to read from 0x10001000 which was incorrect.
 
 
  In my world I consider the file and load addresses to be the
  same thing, that is, the address (not the file offset) of the symbol in
  the object file, e.g. when I objdump symbols and grep for ones I know of
  in a kalimba ELF, I get
  
  054f g   DM|0    $_g_matt1
  ...
  
  
  0x54f as the file address of g_matt1.
  
  
  The other address terminology I hear of is virtual address. To me this
  the address of the symbol once the binary is actually running on the
  processor. So in some embedded scenarios (like kalimba where there is no
  OS) we have code addresses in the ELF (i.e. file/load address) all
  starting at 8000 e.g.
  
  8354 g F PM|0    $_main
  
  But on the device (since it's harvard architecture with a CODE and DATA
  bus), main is actually at 0x0354. So in this context I'd say 0x8354
  was the load/file address but 0x0354 was the virtual address. I see
  similar scenario with linux shared object files where in the file the
  symbol addresses are often based at 0, but at runtime are fixed-up to
  some arbitrary offset.   
  
  Can you explain what lldb means by file/load/virtual and so on
  addresses?   
 
 
 So in your terms:
 
 virtual address is what we call the load address.
 file address means address as it is found in the object file you loaded it 
 from.
 
 Many object files speak of a virtual address when they are speaking of file 
 addresses, so I didn't think virtual address made as much sense as load 
 address. For example the mach-o segments have a vmaddr and vmsize fields 
 when parsing the segments which stand for virtual address

Re: [Lldb-commits] [lldb] r221213 - Fixed SBTarget::ReadMemory() to work correctly and the TestTargetAPI.py test case that was reading target memory in TargetAPITestCase.test_read_memory_with_dsym and

[Matthew Gardiner]

On Tue, 2014-11-04 at 14:37 -0800, Jason Molenda wrote:
 FWIW the use of an Address object to represent addresses was motivated by the 
 hassle of address handling we had with gdb.  If you start the debugger and 
 give it an executable and a bunch of solibs, set an address breakpoint, and 
 then run the process, how does that address breakpoint get re-set to the 
 correct place when the executable and all the solibs land at their final 
 address?  What if multiple solibs are at the same addr (say 0x0) before we 
 start execution?  (on Mac OS X solibs don't have distinct virtual addresses 
 these days - they're all 0-based and the dynamic loader picks a spot in the 
 address space at runtime)
 

Yes, I believe that the linux OS does something very similar.


 The Address object represents addresses as a section + 
 offset-into-that-section, if it's within the bounds of a known binary (a 
 Module).  When the process starts up, lldb learns where all the solibs are 
 actually loaded from the dynamic loader -- it records where each Section is 
 loaded in memory as part of the Target.  So given an Address object, we can 
 get the actual address in real memory via the Target's section load list.
 
 This abstraction solves a lot of subtle bugs we'd hit in gdb when an objfile 
 would shift in memory to a new address -- we'd have to go through all the 
 addresses that might have that old address and make sure they're updated 
 correctly to the new one.  With lldb we have the section load list in the 
 Target which has the current address for each Section.
 

Yup. This abstraction makes a lot of sense.

 When we're getting a real memory address (a load address, or as Greg was 
 saying to me earlier, think of it as a live address if that helps) out of 
 the process (e.g. we read the pc register), usually the first thing we'll do 
 is convert that into an Address object (again, with help of the Target's 
 section load list) putting it in terms of a Section and an offset.
 

Well, live address is possibly clearer, but I'm quite happy now that I
understand what's meant by lldb's load address terminology. And I
believe that Target::ResolveLoadAddress() will convert a raw (live)
address to section:offset address, based on what the dyld has done, with
a running a process.

I had added Target::ResolveFileAddress() (to SBTarget and Target) in an
attempt to take a file_address and convert it to section:offset address.
I think that hindsight is now telling me that this was a bad idea, since
a file_address is not necessarily unique, since we may have address x in
both module A and module B

thanks for your explanation and patience
Matt


 When a real memory address in the process doesn't belong to any of the 
 binaries (Modules) -- for instance, a pointer into a heap allocation -- then 
 we can put it in an Address object but it's just an offset alone with no 
 section.  But addresses that correspond to a function or a symbol are 
 expressed in terms of the containing section and offset into that section.
 
 
 
  On Nov 4, 2014, at 10:18 AM, Greg Clayton gclay...@apple.com wrote:
  
  
  On Nov 3, 2014, at 10:53 PM, Matthew Gardiner m...@csr.com wrote:
  
  Hi Greg,
  
  So what in lldb's world is the difference between a file and a load
  address?
  
  
  file address (as LLDB treats them) is the address that is found in the 
  object file (ELF, MachO, or COFF). load address is the actual address in 
  the process where the section is loaded after being slid.
  
  For example you might have a shared library that has a function foo whose 
  file address is 0x1000, but when the shared library gets loaded into 
  memory, it will be loaded at a different address because all shared 
  libraries have functions in the low file address range (say from 0 to 
  0x40 for example). So if the shared library gets loaded with a slide of 
  0x100, foo will have a load address of 0x100 + 0x1000.
  
  Now for most embedded debugging where there is no OS that will slide things 
  around, your file and load addresses will match. For actual OS level 
  debugging, they won't for shared libraries, and might for the main 
  executable. Sometimes the main executable doesn't get slid around, but 
  other OSs will use ASLR to slide the main executables around for security.
  
  The test that was written was trying to get a data section from the object 
  file. Then it made a section offset address that pointed to that data 
  section. Then it called SBTarget::ReadMemory(...) with that section offset 
  address. What was happening on MacOSX was:
  
  - get the data section whose file address was 0x10001000
  - Make a section + offset address from it that was represented as a.out's 
  data section + 0 bytes
  - call target read memory
  
  Prior to my fix this happened:
  
  SBTarget::ReadMemory() made a new section offset address:
  
  Address address(section_offset_addr.GetFileAddress(), NULL);
  
  Now we have an address that has

Re: [Lldb-commits] [lldb] r221213 - Fixed SBTarget::ReadMemory() to work correctly and the TestTargetAPI.py test case that was reading target memory in TargetAPITestCase.test_read_memory_with_dsym and

[Matthew Gardiner]

Hi Greg,

So what in lldb's world is the difference between a file and a load
address? In my world I consider the file and load addresses to be the
same thing, that is, the address (not the file offset) of the symbol in
the object file, e.g. when I objdump symbols and grep for ones I know of
in a kalimba ELF, I get

054f g   DM|0    $_g_matt1
...


0x54f as the file address of g_matt1.


The other address terminology I hear of is virtual address. To me this
the address of the symbol once the binary is actually running on the
processor. So in some embedded scenarios (like kalimba where there is no
OS) we have code addresses in the ELF (i.e. file/load address) all
starting at 8000 e.g.

8354 g F PM|0    $_main

But on the device (since it's harvard architecture with a CODE and DATA
bus), main is actually at 0x0354. So in this context I'd say 0x8354
was the load/file address but 0x0354 was the virtual address. I see
similar scenario with linux shared object files where in the file the
symbol addresses are often based at 0, but at runtime are fixed-up to
some arbitrary offset.   

Can you explain what lldb means by file/load/virtual and so on
addresses?   

thanks
Matt



On Tue, 2014-11-04 at 00:56 +, Greg Clayton wrote:
 Author: gclayton
 Date: Mon Nov  3 18:56:30 2014
 New Revision: 221213
 
 URL: http://llvm.org/viewvc/llvm-project?rev=221213view=rev
 Log:
 Fixed SBTarget::ReadMemory() to work correctly and the TestTargetAPI.py test 
 case that was reading target memory in 
 TargetAPITestCase.test_read_memory_with_dsym and 
 TargetAPITestCase.test_read_memory_with_dwarf.
 
 The problem was that SBTarget::ReadMemory() was making a new section offset 
 lldb_private::Address by doing:
 
 
 size_t
 SBTarget::ReadMemory (const SBAddress addr,
   void *buf,
   size_t size,
   lldb::SBError error)
 {
 ...
 lldb_private::Address addr_priv(addr.GetFileAddress(), NULL);
 bytes_read = target_sp-ReadMemory(addr_priv, false, buf, size, 
 err_priv);
 
 
 This is wrong. If you get the file addresss from the addr argument and try 
 to read memory using that, it will think the file address is a load address 
 and it will try to resolve it accordingly. This will work fine if your 
 executable is loaded at the same address (no slide), but it won't work if 
 there is a slide.
 
 The fix is to just pass along the addr.ref() instead of making a new 
 addr_priv as this will pass along the lldb_private::Address that is inside 
 the SBAddress (which is what we want), and not always change it into 
 something that becomes a load address (if we are running), or abmigious file 
 address (think address zero when you have 150 shared libraries that have 
 sections that start at zero, which one would you pick). The main reason for 
 passing a section offset address to SBTarget::ReadMemory() is so you _can_ 
 read from the actual section + offset that is specified in the SBAddress. 
 
 
 
 Modified:
 lldb/trunk/source/API/SBTarget.cpp
 lldb/trunk/test/python_api/target/TestTargetAPI.py
 
 Modified: lldb/trunk/source/API/SBTarget.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBTarget.cpp?rev=221213r1=221212r2=221213view=diff
 ==
 --- lldb/trunk/source/API/SBTarget.cpp (original)
 +++ lldb/trunk/source/API/SBTarget.cpp Mon Nov  3 18:56:30 2014
 @@ -1306,13 +1306,11 @@ SBTarget::ReadMemory (const SBAddress ad
  if (target_sp)
  {
  Mutex::Locker api_locker (target_sp-GetAPIMutex());
 -lldb_private::Address addr_priv(addr.GetFileAddress(), NULL);
 -lldb_private::Error err_priv;
 -bytes_read = target_sp-ReadMemory(addr_priv, false, buf, size, 
 err_priv);
 -if(err_priv.Fail())
 -{
 -sb_error.SetError(err_priv.GetError(), err_priv.GetType());
 -}
 +bytes_read = target_sp-ReadMemory(addr.ref(), false, buf, size, 
 sb_error.ref());
 +}
 +else
 +{
 +sb_error.SetErrorString(invalid target);
  }
  
  return bytes_read;
 
 Modified: lldb/trunk/test/python_api/target/TestTargetAPI.py
 URL: 
 http://llvm.org/viewvc/llvm-project/lldb/trunk/test/python_api/target/TestTargetAPI.py?rev=221213r1=221212r2=221213view=diff
 ==
 --- lldb/trunk/test/python_api/target/TestTargetAPI.py (original)
 +++ lldb/trunk/test/python_api/target/TestTargetAPI.py Mon Nov  3 18:56:30 
 2014
 @@ -213,16 +213,20 @@ class TargetAPITestCase(TestBase):
  breakpoint = target.BreakpointCreateByLocation(main.c, 
 self.line_main)
  self.assertTrue(breakpoint, VALID_BREAKPOINT)
  
 +# Put debugger into synchronous mode so when we target.LaunchSimple 
 returns
 +# it will guaranteed to be at the breakpoint
 +

Re: [Lldb-commits] [PATCH] Minimal API support for non-8-bit byte targets

[Matthew Gardiner]

I've uploaded a new diff, which I believe addresses Jim's comments, that is, 
I've bifurcated each of the tests which I added in the first diff into a dwarf 
and dsym version.

http://reviews.llvm.org/D5867

Files:
  include/lldb/API/SBSection.h
  include/lldb/API/SBTarget.h
  include/lldb/Target/Target.h
  scripts/Python/interface/SBSection.i
  scripts/Python/interface/SBTarget.i
  source/API/SBSection.cpp
  source/API/SBTarget.cpp
  source/Target/Target.cpp
  test/lldbtest.py
  test/python_api/section/Makefile
  test/python_api/section/TestSectionAPI.py
  test/python_api/section/main.c
  test/python_api/target/TestTargetAPI.py
  test/python_api/target/main.c
Index: include/lldb/API/SBSection.h
===
--- include/lldb/API/SBSection.h
+++ include/lldb/API/SBSection.h
@@ -71,6 +71,18 @@
 SectionType
 GetSectionType ();
 
+//--
+/// Return the size of a target's byte represented by this section
+/// in numbers of host bytes. Note that certain architectures have
+/// varying minimum addressable unit (i.e. byte) size for their 
+/// CODE or DATA buses.
+///
+/// @return
+/// The number of host (8-bit) bytes needed to hold a target byte
+//--
+uint32_t
+GetTargetByteSize ();
+
 bool
 operator == (const lldb::SBSection rhs);
 
Index: include/lldb/API/SBTarget.h
===
--- include/lldb/API/SBTarget.h
+++ include/lldb/API/SBTarget.h
@@ -22,6 +22,8 @@
 
 namespace lldb {
 
+class SBPlatform;
+
 class SBLaunchInfo
 {
 public:
@@ -309,6 +311,18 @@
 GetProcess ();
 
 //--
+/// Return the platform object associated with the target.
+///
+/// After return, the platform object should be checked for
+/// validity.
+///
+/// @return
+/// A platform object.
+//--
+lldb::SBPlatform
+GetPlatform ();
+
+//--
 /// Install any binaries that need to be installed.
 ///
 /// This function does nothing when debugging on the host system.
@@ -564,6 +578,26 @@
 GetTriple ();
 
 //--
+/// Architecture data byte width accessor
+///
+/// @return
+/// The size in 8-bit (host) bytes of a minimum addressable
+/// unit from the Architecture's data bus
+//--
+uint32_t
+GetDataByteSize ();
+
+//--
+/// Architecture code byte width accessor
+///
+/// @return
+/// The size in 8-bit (host) bytes of a minimum addressable
+/// unit from the Architecture's code bus
+//--
+uint32_t
+GetCodeByteSize ();
+
+//--
 /// Set the base load address for a module section.
 ///
 /// @param[in] section
@@ -728,6 +762,17 @@
 Clear ();
 
 //--
+/// Resolve a current file address into a section offset address.
+///
+/// @param[in] file_addr
+///
+/// @return
+/// An SBAddress which will be valid if...
+//--
+lldb::SBAddress
+ResolveFileAddress (lldb::addr_t file_addr);
+
+//--
 /// Resolve a current load address into a section offset address.
 ///
 /// @param[in] vm_addr
@@ -772,6 +817,31 @@
 ResolveSymbolContextForAddress (const SBAddress addr, 
 uint32_t resolve_scope);
 
+//--
+/// Read target memory. If a target process is running then memory  
+/// is read from here. Otherwise the memory is read from the object
+/// files. For a target whose bytes are sized as a multiple of host
+/// bytes, the data read back will preserve the target's byte order.
+///
+/// @param[in] addr
+/// A target address to read from. 
+///
+/// @param[out] buf
+/// The buffer to read memory into. 
+///
+/// @param[in] size
+/// The maximum number of host bytes to read in the buffer passed
+/// into this call
+///
+/// @param[out] error
+/// Error information is written here if the memory read fails.
+///
+/// @return
+/// The amount of data read in host bytes.

Re: [Lldb-commits] [PATCH] Fix swig interface for SBTarget.Launch

[Matthew Gardiner]

Two initial comments:

1. applying the diff failed for me:

$ patch -p0 D5878.diff 
patching file scripts/Python/interface/SBTarget.i
Hunk #1 succeeded at 279 with fuzz 2 (offset -146 lines).
patching file test/api/multiple-debuggers/multi-process-driver.cpp
patching file test/python_api/event/TestEvents.py
Hunk #1 FAILED at 161.
Hunk #2 FAILED at 229.
2 out of 2 hunks FAILED -- saving rejects to file 
test/python_api/event/TestEvents.py.rej
patching file test/python_api/process/io/TestProcessIO.py
patching file test/python_api/target/TestTargetAPI.py

Could you re generate this diff please? 

2. Could you use a more descriptive name than Launch2 please?

http://reviews.llvm.org/D5878



___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r219102 - Call SBDebugger::Initialize/Terminate from within Create/Destroy.

[Matthew Gardiner]

Hi Ed,

Which test was the first to segfault in the suite. I've looked at the
stdio page from the check-lldb target from the attached link, and it
looks like it's this statement which indicates the bot failure: 

FAIL: LLDB (suite) :: TestFormattersSBAPI.py (FreeBSD
llvm-amd64.freebsd.your.org 10.1-STABLE FreeBSD 10.1-STABLE #0 r272281M:
Mon Sep 29 14:19:16 UTC 2014
r...@llvm-amd64.freebsd.your.org:/data/obj/usr/src/sys/GENERIC amd64
amd64)

ninja: build stopped: subcommand failed.
program finished with exit code 1
elapsedTime=586.288456

I'm keen to know as I'd like to set up a freebsd system to replicate this type 
of failure.

Matt


On Mon, 2014-10-06 at 09:52 -0400, Ed Maste wrote:

 On 6 October 2014 01:22, Matthew Gardiner m...@csr.com wrote:
  Author: mg11
  Date: Mon Oct  6 00:22:29 2014
  New Revision: 219102
 
  URL: http://llvm.org/viewvc/llvm-project?rev=219102view=rev
  Log:
  Call SBDebugger::Initialize/Terminate from within Create/Destroy.
 
  The above change permits developers using the lldb C++ API to
  code applications in a more logical manner.
 
 I've reverted this change because it caused a large number of tests to
 segfault in Python, like so:
 
 * thread #1: tid = 0, 0x004a0b1b python2.7`??? + 267, name =
 'python2.7', stop reason = signal SIGSEGV
   * frame #0: 0x004a0b1b python2.7`??? + 267
 frame #1: 0x004a0bbc python2.7`PyGILState_Ensure + 76
 frame #2: 0x0008063d6b79
 _lldb.so`lldb_private::ScriptInterpreterPython::Locker::DoAcquireLock()
 + 41
 frame #3: 0x0008063d6aec
 _lldb.so`lldb_private::ScriptInterpreterPython::Locker::Locker(lldb_private::ScriptInterpreterPython*,
 unsigned short, unsigned short, __sFILE*, __sFILE*, __sFILE*) + 124
 frame #4: 0x0008063e30cf
 _lldb.so`lldb_private::ScriptInterpreterPython::Clear() + 79
 frame #5: 0x000806366e28
 _lldb.so`lldb_private::CommandInterpreter::Clear() + 408
 frame #6: 0x000806186bef _lldb.so`lldb_private::Debugger::Clear() + 
 623
 frame #7: 0x0008061885b5
 _lldb.so`lldb_private::Debugger::~Debugger() + 53
 frame #8: 0x000806188569
 _lldb.so`lldb_private::Debugger::~Debugger() + 25
 frame #9: 0x0008061a179c
 _lldb.so`std::__1::__shared_ptr_pointerlldb_private::Debugger*,
 std::__1::default_deletelldb_private::Debugger,
 std::__1::allocatorlldb_private::Debugger ::__on_zero_shared() +
 156
 frame #10: 0x00080abf8bbc
 libc++.so.1`std::__1::__shared_weak_count::__release_shared()
 [inlined] std::__1::__shared_count::__release_shared(this=0x000804262ac0)
 + 44 at memory.cpp:61
 frame #11: 0x00080abf8ba8
 libc++.so.1`std::__1::__shared_weak_count::__release_shared(this=0x000804262ac0)
 + 24 at memory.cpp:86
 frame #12: 0x000806199ecf
 _lldb.so`std::__1::shared_ptrlldb_private::Debugger::~shared_ptr() +
 47
 frame #13: 0x0008061aca51
 _lldb.so`std::__1::__vector_basestd::__1::shared_ptrlldb_private::Debugger,
 std::__1::allocatorstd::__1::shared_ptrlldb_private::Debugger 
 ::~__vector_base() + 273
 frame #14: 0x0008061ac935
 _lldb.so`std::__1::vectorstd::__1::shared_ptrlldb_private::Debugger,
 std::__1::allocatorstd::__1::shared_ptrlldb_private::Debugger 
 ::~vector() + 21
 frame #15: 0x0008012d4610
 libc.so.7`__cxa_finalize(dso=0x) + 320 at atexit.c:198
 frame #16: 0x0008012760cc libc.so.7`exit(status=0) + 28 at exit.c:67
 frame #17: 0x00414025 python2.7`_start + 149
 
 Bot link: 
 http://llvm-amd64.freebsd.your.org/b/builders/lldb-amd64-freebsd/builds/2675
 
 
  To report this email as spam click 
 https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .




Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r219102 - Call SBDebugger::Initialize/Terminate from within Create/Destroy.

[Matthew Gardiner]

Ok, Thanks Ed,

I've just installed FreeBSD-10.0 (amd64) on VirtualBox. Have installed
virtualbox-ose-additions, and gnome2.

Unfortunately X (i.e. gnome) freezes on startup (i.e. won't accept kdb
or mouse input). Perhaps I should just struggle on with just running in
text mode, but having makes one so much more productive...

Matt


On Tue, 2014-10-07 at 09:21 -0400, Ed Maste wrote:
 On 7 October 2014 02:18, Matthew Gardiner m...@csr.com wrote:
  Hi Ed,
 
  Which test was the first to segfault in the suite. I've looked at the stdio
  page from the check-lldb target from the attached link, and it looks like
  it's this statement which indicates the bot failure:
 
  FAIL: LLDB (suite) :: TestFormattersSBAPI.py (FreeBSD
  llvm-amd64.freebsd.your.org 10.1-STABLE FreeBSD 10.1-STABLE #0 r272281M: Mon
  Sep 29 14:19:16 UTC 2014
  r...@llvm-amd64.freebsd.your.org:/data/obj/usr/src/sys/GENERIC amd64 amd64)
 
 Yes, that's the first failure. Unfortunately it seems the segfault is
 hidden by the test suite, so it's not that obvious from the log where
 it happens.
 
 I reproduce it with:
 python dotest.py --executable /.path.to./lldb -C /usr/bin/clang -v -t
 -p TestFormattersSBAPI
 
 The Linux and OS X thread libraries are more forgiving of invalid use
 than FreeBSD's, which may explain differences in the test results.
 For example when I started working on LLDB there were a few instances
 of calling _unlock() on an unlocked mutex, or from a thread other than
 that which called _lock().  That seemed to work on OS X and Linux, but
 crashed on FreeBSD.
 
  I'm keen to know as I'd like to set up a freebsd system to replicate this
  type of failure.
 
 Excellent, let me know if you need anything.  I have some instructions
 for setting up a FreeBSD VM and building LLDB here:
 https://wiki.freebsd.org/lldb
 
 
  To report this email as spam click 
 https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .




Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r219102 - Call SBDebugger::Initialize/Terminate from within Create/Destroy.

[Matthew Gardiner]

Author: mg11
Date: Mon Oct  6 00:22:29 2014
New Revision: 219102

URL: http://llvm.org/viewvc/llvm-project?rev=219102view=rev
Log:
Call SBDebugger::Initialize/Terminate from within Create/Destroy.

The above change permits developers using the lldb C++ API to
code applications in a more logical manner.


Modified:
lldb/trunk/source/API/SBDebugger.cpp

Modified: lldb/trunk/source/API/SBDebugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBDebugger.cpp?rev=219102r1=219101r2=219102view=diff
==
--- lldb/trunk/source/API/SBDebugger.cpp (original)
+++ lldb/trunk/source/API/SBDebugger.cpp Mon Oct  6 00:22:29 2014
@@ -159,6 +159,8 @@ SBDebugger::Create(bool source_init_file
 {
 Log *log(GetLogIfAllCategoriesSet (LIBLLDB_LOG_API));
 
+Initialize();
+
 SBDebugger debugger;
 
 // Currently we have issues if this function is called simultaneously on 
two different
@@ -210,6 +212,8 @@ SBDebugger::Destroy (SBDebugger debugge
  sstr.GetData());
 }
 
+Terminate();
+
 Debugger::Destroy (debugger.m_opaque_sp);
 
 if (debugger.m_opaque_sp.get() != NULL)


___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r218594 - Included cstdarg for compilation of va_start and va_end.

[Matthew Gardiner]

Author: mg11
Date: Mon Sep 29 02:12:47 2014
New Revision: 218594

URL: http://llvm.org/viewvc/llvm-project?rev=218594view=rev
Log:
Included cstdarg for compilation of va_start and va_end.

Modified:
lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp

Modified: lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp?rev=218594r1=218593r2=218594view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp Mon Sep 
29 02:12:47 2014
@@ -16,6 +16,7 @@
 
 #include ThreadStateCoordinator.h
 #include memory
+#include cstdarg
 
 using namespace lldb_private;
 


___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r218596 - Very minimal support 24-bit kalimbas. Vanilla memory read for data sections

[Matthew Gardiner]

Author: mg11
Date: Mon Sep 29 03:02:24 2014
New Revision: 218596

URL: http://llvm.org/viewvc/llvm-project?rev=218596view=rev
Log:
Very minimal support 24-bit kalimbas. Vanilla memory read for data sections
works, as do breakpoints, run and pause, display zeroth frame.

See
http://reviews.llvm.org/D5503

for a fuller description of the changes in this commit.


Modified:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/include/lldb/Core/DataExtractor.h
lldb/trunk/include/lldb/Core/Section.h
lldb/trunk/source/Commands/CommandObjectMemory.cpp
lldb/trunk/source/Core/ArchSpec.cpp
lldb/trunk/source/Core/DataExtractor.cpp
lldb/trunk/source/Core/Section.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
lldb/trunk/source/Symbol/ObjectFile.cpp

Modified: lldb/trunk/include/lldb/Core/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ArchSpec.h?rev=218596r1=218595r2=218596view=diff
==
--- lldb/trunk/include/lldb/Core/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Core/ArchSpec.h Mon Sep 29 03:02:24 2014
@@ -104,7 +104,6 @@ public:
 eCore_uknownMach32,
 eCore_uknownMach64,
 
-eCore_kalimba,
 eCore_kalimba3,
 eCore_kalimba4,
 eCore_kalimba5,
@@ -142,7 +141,7 @@ public:
 kCore_hexagon_first  = eCore_hexagon_generic,
 kCore_hexagon_last   = eCore_hexagon_hexagonv5,
 
-kCore_kalimba_first = eCore_kalimba,
+kCore_kalimba_first = eCore_kalimba3,
 kCore_kalimba_last = eCore_kalimba5
 };
 

Modified: lldb/trunk/include/lldb/Core/DataExtractor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/DataExtractor.h?rev=218596r1=218595r2=218596view=diff
==
--- lldb/trunk/include/lldb/Core/DataExtractor.h (original)
+++ lldb/trunk/include/lldb/Core/DataExtractor.h Mon Sep 29 03:02:24 2014
@@ -85,8 +85,11 @@ public:
 ///
 /// @param[in] addr_size
 /// A new address byte size value.
+///
+/// @param[in] target_byte_size
+/// A size of a target byte in 8-bit host bytes
 //--
-DataExtractor (const void* data, lldb::offset_t data_length, 
lldb::ByteOrder byte_order, uint32_t addr_size);
+DataExtractor (const void* data, lldb::offset_t data_length, 
lldb::ByteOrder byte_order, uint32_t addr_size, uint32_t target_byte_size = 1);
 
 //--
 /// Construct with shared data.
@@ -104,8 +107,11 @@ public:
 ///
 /// @param[in] addr_size
 /// A new address byte size value.
+///
+/// @param[in] target_byte_size
+/// A size of a target byte in 8-bit host bytes
 //--
-DataExtractor (const lldb::DataBufferSP data_sp, lldb::ByteOrder 
byte_order, uint32_t addr_size);
+DataExtractor (const lldb::DataBufferSP data_sp, lldb::ByteOrder 
byte_order, uint32_t addr_size, uint32_t target_byte_size = 1);
 
 //--
 /// Construct with a subset of \a data.
@@ -129,8 +135,11 @@ public:
 ///
 /// @param[in] length
 /// The length in bytes of the subset of data.
+///
+/// @param[in] target_byte_size
+/// A size of a target byte in 8-bit host bytes
 //--
-DataExtractor (const DataExtractor data, lldb::offset_t offset, 
lldb::offset_t length);
+DataExtractor (const DataExtractor data, lldb::offset_t offset, 
lldb::offset_t length, uint32_t target_byte_size = 1);
 
 DataExtractor (const DataExtractor rhs);
 //--
@@ -863,7 +872,7 @@ public:
 *offset_ptr += 1;
 return val;
 }
-
+
 uint16_t
 GetU16_unchecked (lldb::offset_t *offset_ptr) const;
 
@@ -1311,6 +1320,7 @@ protected:
 lldb::ByteOrder m_byte_order;   /// The byte order of the data we are 
extracting from.
 uint32_t m_addr_size;   /// The address size to use when 
extracting pointers or addresses
 mutable lldb::DataBufferSP m_data_sp; /// The shared pointer to data that 
can be shared among multilple instances
+const uint32_t m_target_byte_size;
 };
 
 } // namespace lldb_private

Modified: lldb/trunk/include/lldb/Core/Section.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Section.h?rev=218596r1=218595r2=218596view=diff
==
--- lldb/trunk/include/lldb/Core/Section.h (original)
+++ lldb/trunk/include/lldb/Core/Section.h Mon 

Re: [Lldb-commits] COMMERCIAL: [lldb] r218001 - Hex encode the triple values in case they contain special characters.

[Matthew Gardiner]

Hi Greg/Jason,

This change, r218001, reverses a change I made: r214480.

I made the change (send triple as plain text), motivated by a discussion
with Jason Molenda, which started here:

http://lists.cs.uiuc.edu/pipermail/lldb-commits/Week-of-Mon-20140721/011978.html

What's happening here?

thanks
Matt 



On Thu, 2014-09-18 at 00:18 +, Greg Clayton wrote:
 Author: gclayton
 Date: Wed Sep 17 19:18:32 2014
 New Revision: 218001
 
 URL: http://llvm.org/viewvc/llvm-project?rev=218001view=rev
 Log:
 Hex encode the triple values in case they contain special characters.
 
 Modified:
 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
 
 Modified: 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=218001r1=218000r2=218001view=diff
 ==
 --- 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
 (original)
 +++ 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
 Wed Sep 17 19:18:32 2014
 @@ -1643,7 +1643,9 @@ GDBRemoteCommunicationClient::GetHostInf
  }
  else if (name.compare(triple) == 0)
  {
 -triple.swap(value);
 +extractor.GetStringRef ().swap (value);
 +extractor.SetFilePos(0);
 +extractor.GetHexByteString (triple);
  ++num_keys_decoded;
  }
  else if (name.compare (distribution_id) == 0)
 @@ -2331,6 +2333,10 @@ GDBRemoteCommunicationClient::DecodeProc
  }
  else if (name.compare(triple) == 0)
  {
 +StringExtractor extractor;
 +extractor.GetStringRef().swap(value);
 +extractor.SetFilePos(0);
 +extractor.GetHexByteString (value);
  process_info.GetArchitecture ().SetTriple (value.c_str());
  }
  else if (name.compare(name) == 0)
 @@ -2447,7 +2453,10 @@ GDBRemoteCommunicationClient::GetCurrent
  }
  else if (name.compare(triple) == 0)
  {
 -triple = value;
 +StringExtractor extractor;
 +extractor.GetStringRef().swap(value);
 +extractor.SetFilePos(0);
 +extractor.GetHexByteString (triple);
  ++num_keys_decoded;
  }
  else if (name.compare(ostype) == 0)
 
 Modified: 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp?rev=218001r1=218000r2=218001view=diff
 ==
 --- 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp 
 (original)
 +++ 
 lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp 
 Wed Sep 17 19:18:32 2014
 @@ -1207,7 +1207,7 @@ GDBRemoteCommunicationServer::Handle_qHo
  ArchSpec host_arch(HostInfo::GetArchitecture());
  const llvm::Triple host_triple = host_arch.GetTriple();
  response.PutCString(triple:);
 -response.PutCString(host_triple.getTriple().c_str());
 +response.PutCStringAsRawHex8(host_triple.getTriple().c_str());
  response.Printf (;ptrsize:%u;,host_arch.GetAddressByteSize());
  
  const char* distribution_id = host_arch.GetDistributionId ().AsCString 
 ();
 @@ -1325,7 +1325,7 @@ CreateProcessInfoResponse (const Process
  {
  const llvm::Triple proc_triple = proc_arch.GetTriple();
  response.PutCString(triple:);
 -response.PutCString(proc_triple.getTriple().c_str());
 +response.PutCStringAsRawHex8(proc_triple.getTriple().c_str());
  response.PutChar(';');
  }
  }
 @@ -1361,7 +1361,10 @@ CreateProcessInfoResponse_DebugServerSty
  response.Printf (vendor:%s;, vendor.c_str ());
  #else
  // We'll send the triple.
 -response.Printf (triple:%s;, proc_triple.getTriple().c_str ());
 +response.PutCString(triple:);
 +response.PutCStringAsRawHex8(proc_triple.getTriple().c_str());
 +response.PutChar(';');
 +
  #endif
  std::string ostype = proc_triple.getOSName ();
  // Adjust so ostype reports ios for Apple/ARM and Apple/ARM64.
 
 
 ___
 lldb-commits mailing list
 lldb-commits@cs.uiuc.edu
 http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
 
 
  To report this email as spam click 
 

[Lldb-commits] [lldb] r216867 - Add an interface on ArchSpec to provide lldb client code

[Matthew Gardiner]

Author: mg11
Date: Mon Sep  1 04:06:03 2014
New Revision: 216867

URL: http://llvm.org/viewvc/llvm-project?rev=216867view=rev
Log:
Add an interface on ArchSpec to provide lldb client code
with a mechanism to query if the current target architecture
has non 8-bit bytes.

Modified:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/source/Core/ArchSpec.cpp

Modified: lldb/trunk/include/lldb/Core/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ArchSpec.h?rev=216867r1=216866r2=216867view=diff
==
--- lldb/trunk/include/lldb/Core/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Core/ArchSpec.h Mon Sep  1 04:06:03 2014
@@ -369,6 +369,24 @@ public:
 GetMachOCPUSubType () const;
 
 //--
+/// Architecture data byte width accessor
+///
+/// @return the size in 8-bit (host) bytes of a minimum addressable
+/// unit from the Architecture's data bus
+//--
+uint32_t
+GetDataByteSize() const;
+
+//--
+/// Architecture code byte width accessor
+///
+/// @return the size in 8-bit (host) bytes of a minimum addressable
+/// unit from the Architecture's code bus
+//--
+uint32_t
+GetCodeByteSize() const;
+ 
+//--
 /// Architecture tripple accessor.
 ///
 /// @return A triple describing this ArchSpec.

Modified: lldb/trunk/source/Core/ArchSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ArchSpec.cpp?rev=216867r1=216866r2=216867view=diff
==
--- lldb/trunk/source/Core/ArchSpec.cpp (original)
+++ lldb/trunk/source/Core/ArchSpec.cpp Mon Sep  1 04:06:03 2014
@@ -497,6 +497,40 @@ ArchSpec::GetMachOCPUSubType () const
 return LLDB_INVALID_CPUTYPE;
 }
 
+uint32_t
+ArchSpec::GetDataByteSize () const
+{
+switch (m_core)
+{
+case eCore_kalimba3:
+return 3;
+case eCore_kalimba4:
+return 1;
+case eCore_kalimba5:
+return 3;
+default:
+return 1;
+}
+return 1;
+}
+
+uint32_t
+ArchSpec::GetCodeByteSize () const
+{
+switch (m_core)
+{
+case eCore_kalimba3:
+return 4;
+case eCore_kalimba4:
+return 1;
+case eCore_kalimba5:
+return 1;
+default:
+return 1;
+}
+return 1;
+}
+
 llvm::Triple::ArchType
 ArchSpec::GetMachine () const
 {


___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r216398 - Change back all paths returns for lldb::PathType enumerations from HostInfo::GetLLDBPath() to return the directories in the FileSpec.m_directory field to match prev

[Matthew Gardiner]

I would just change it to GetLastPathComponent() and and leave GetDirectory() alone 
or change it to GetContainingDirectory().


Greg's suggestion maps very cleanly onto DWARF debug information, since 
with that we a compilation unit directory, e.g.


/home/mg11/src/projects/browser

and then the notion of paths relative to that compile directory:

main.cpp
server/socket.cpp
server/script_parse.cpp
utils/logger.cpp

if I recall correctly...

Matt


Greg Clayton wrote:

On Aug 26, 2014, at 12:39 PM, Zachary Turner ztur...@google.com wrote:

I guess I'm thinking of this as the standard unix basename / dirname paradigm.  
Everyone understands this paradigm and there's no confusion about it.  And 
given that FileSpec already internally stores its components in this format, it 
would be a natural fit for m_filename and m_dirname to correspond to basename 
and dirname, respectively.  In fact, the entire class is already implemented 
with this assumption.  See, for example, SetFile() which can be used to set 
arbitrary paths, which could refer to directories, but which still splits the 
last component and stores it in m_filename.  Or AppendPathComponent(), which 
handles the case where m_filename is not null, hence assuming that it must 
refer to a directory.

As for the places that broke as a result of my last change - certainly they need to be 
fixed, so reverting to green is reasonable until we can decide if there's a 
better fix.  However, I did run all tests, and none of them failed.  I don't think the 
new test which checks that GetFilename() is null is particularly useful, because it just 
tests an implementation detail of the class, and not a documented behavior of the class.  
It should be possible to write a test against whatever failed, which probably would be a 
better test.

There was no test and thus why I added one. But I do agree that the API is not 
consistent.


In any case, back to my original question: Is there any object to changing 
FileSpec::GetDirectory() and FileSpec::GetFilename() to be GetBaseName() and 
GetDirName()?

I would just change it to GetLastPathComponent() and and leave GetDirectory() 
alone or change it to GetContainingDirectory().


This would require changing the call sites of the places who previously broke 
as a result of my original change.  I don't actually know what broke since my 
test runs were all successful, though.

The other issue with changing things is that we have internal branches that are 
based on this code that are constantly requiring merging due to all of these 
changes that have been going on. I appreciate all the changes, and changing 
things to be better organized is nice.


  If this is not appropriate, then I think we need to change 
AppendPathComponent(), SetFile(), and various other functions to stop allowing 
situations where m_directory + m_filename refers to a directory.

A few core design things:
- no setters can require any stat calls to determine if things are directories 
unless someone explicitly asks the FileSpec for this info
- basename and directory must remain separate ConstString values

The Host calls that got the directories were optimized for the use cases where 
client asks for directory X and then sets the basename via the GetFilename() 
accessor. I would prefer to maintain this if possible to avoid contaminating 
the constant string pool and we can do this via APIs.

Greg
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits


  To report this email as spam click 
https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .




Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r216440 - Remove the hostname part from compilation directories, if supplied by

[Matthew Gardiner]

Author: mg11
Date: Tue Aug 26 01:57:23 2014
New Revision: 216440

URL: http://llvm.org/viewvc/llvm-project?rev=216440view=rev
Log:
Remove the hostname part from compilation directories, if supplied by 
DWARF2/3 compliant producers.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=216440r1=216439r2=216440view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue Aug 26 
01:57:23 2014
@@ -66,6 +66,9 @@
 
 #include map
 
+#include ctype.h
+#include string.h
+
 //#define ENABLE_DEBUG_PRINTF // COMMENT OUT THIS LINE PRIOR TO CHECKIN
 
 #ifdef ENABLE_DEBUG_PRINTF
@@ -110,6 +113,34 @@ DW_ACCESS_to_AccessType (uint32_t dwarf_
 return eAccessNone;
 }
 
+static const char*
+removeHostnameFromPathname(const char* path_from_dwarf)
+{
+if (!path_from_dwarf || !path_from_dwarf[0])
+{
+return path_from_dwarf;
+}
+
+const char *colon_pos = strchr(path_from_dwarf, ':');
+if (!colon_pos)
+{
+return path_from_dwarf;
+}
+
+// check whether we have a windows path, and so the first character
+// is a drive-letter not a hostname.
+if (
+colon_pos == path_from_dwarf + 1 
+isalpha(*path_from_dwarf) 
+strlen(path_from_dwarf)  2 
+'\\' == path_from_dwarf[2])
+{
+return path_from_dwarf;
+}
+
+return colon_pos + 1;
+}
+
 #if defined(LLDB_CONFIGURATION_DEBUG) || defined(LLDB_CONFIGURATION_RELEASE)
 
 class DIEStack
@@ -945,7 +976,11 @@ SymbolFileDWARF::ParseCompileUnit (DWARF
 }
 else
 {
+// DWARF2/3 suggests the form 
hostname:pathname for compilation directory.
+// Remove the host part if present.
+cu_comp_dir = 
removeHostnameFromPathname(cu_comp_dir);
 std::string fullpath(cu_comp_dir);
+
 if (*fullpath.rbegin() != '/')
 fullpath += '/';
 fullpath += cu_die_name;
@@ -1172,6 +1207,11 @@ SymbolFileDWARF::ParseCompileUnitSupport
 if (cu_die)
 {
 const char * cu_comp_dir = cu_die-GetAttributeValueAsString(this, 
dwarf_cu, DW_AT_comp_dir, NULL);
+
+// DWARF2/3 suggests the form hostname:pathname for compilation 
directory.
+// Remove the host part if present.
+cu_comp_dir = removeHostnameFromPathname(cu_comp_dir);
+
 dw_offset_t stmt_list = cu_die-GetAttributeValueAsUnsigned(this, 
dwarf_cu, DW_AT_stmt_list, DW_INVALID_OFFSET);
 
 // All file indexes in DWARF are one based and a file of index 
zero is


___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Matthew Gardiner]

Shawn,

To make this kind of solution robust in all scheduling scenarios, for 
example on a PC with a lot of additional load/other processes etc. I 
think we should make this timeout a lot bigger, if we know that the 
ProcessIOHandler will *always* be pushed, I'm thinking at least in terms 
of seconds. And if the timeout *does* occur then what class of error 
does this represent?


I can currently think of 2 reasons:
1. running event does not occur, presumably due to a runtime error
2. running event occurs, but a logic bug makes ShouldBroadcast return false

Matt


Shawn Best wrote:

Matt,

You asked earlier about the significance of the 2000us timeout. When I 
was testing an earlier patch:


do
{
usleep(0);// force context switch

if(state==stopped)
break;

} while(curTime  timeout)

I put some timers and counters in the loop to see how it was 
behaving.  Most of the time, a single context switch was enough time 
to fix the race condition.  Occasionally it would take 3 context 
switches ( in the neighborhood of a few 100us) for flag to be set.


The 2000us timeout number was my swag at a number that would give an 
order of magnitude more time to cover the race, but minimize the 
downside to lldb in case I had not anticipated some condition where it 
would actually hit the timeout.  I never saw it hitting the timeout, 
so it could theoretically be much larger.


Shawn.
On 8/4/2014 10:01 PM, Matthew Gardiner wrote:

Thanks for this update, Jim!

Hopefully, we'll have it all fixed for when he returns ;-)

Matt


jing...@apple.com wrote:
Note, Greg is on vacation this week. Dunno if he'll chime in from 
the beach, but he's sure to have opinions.  Just wanted to make sure 
folks didn't think his silence indicated he didn't...


Jim


On Aug 4, 2014, at 5:41 AM, Matthew Gardiner m...@csr.com wrote:

Folks,

I have been testing Shawn's patch some more today.

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf

...
(lldb) process launch -s
...
lldb) process continue
... lots of output ...

Control-c pressed
(lldb)

Above I found that if I launch the process with stop at entry, 
then I continue, then when I control-c to interrupt the prompt 
returns correctly. With the above session I found I could process 
continue and Ctrl-c repeatedly and the prompt would return as 
expected. I also found that using next to step the inferior also 
resulted in the debugger's prompt returning.


However, if I launched without -s

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf

...
(lldb) process launch
... lots of output ...

Control-c pressed

then the prompt was absent, and hence broken.

Also if I create my target, set a breakpoint, then launch:

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf

...
(lldb) br s -f forever.c -l 12
...
(lldb) process launch
Process 10292 launching
Process 10292 launched: 
'/home/mg11/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf' 
(x86_64)

Process 10292 stopped
* thread #1: tid = 10292, 0x003675a011f0, name = 'i64-hello.elf'
frame #0:
Process 10292 stopped
* thread #1: tid = 10292, 0x00400548 i64-hello.elf`f2(x=8, 
y=0) + 24 at forever.c:12, name = 'i64-hello.elf', stop reason = 
breakpoint 1.1
frame #0: 0x00400548 i64-hello.elf`f2(x=8, y=0) + 24 at 
forever.c:12

   9
   10  x += 8;
   11  y *= x;
- 12  buff[0] = y;
   13
   14  x = buff[0] + buff[1];
   15  return x;

Again, no prompt.

Attaching to a stopped process using gdb-remote, also results in me 
having no prompt:


(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf

...
(lldb) gdb-remote 
Process 10327 stopped
* thread #1: tid = 10327, 0x003675a011f0, name = 
'i64-hello.elf', stop reason = signal SIGSTOP

frame #0: 0x003675a011f0
- 0x3675a011f0:  movq   %rsp, %rdi
   0x3675a011f3:  callq  0x3675a046e0
   0x3675a011f8:  movq   %rax, %r12
   0x3675a011fb:  movl   0x21eb97(%rip), %eax

So, as I alluded to previously, there are a lot of scenarios where 
the IOHandler prompt bug manifests. Now that I have read the code, 
I'm surprised that it does not happen more often and on more 
architectures. Out of interest my test inferior basically spins 
producing lots of output. It *may* one reason why I can provoke 
these bugs so easily. I'm attaching my test code if this helps 
others witness all of the issues which I'm seeing. I'll see if 
there are any simple tweaks I can make (in the spirit of Shawn's 
patch) to fix the above bugs.


Matt


Matthew Gardiner wrote:
I think the last suggestion of moving the sync point to 
Debugger::HandleProcessEvent() would defeat the purpose of the 
patch since it is called from the EventHandler thread.  The race 
condition is the main thread returning

Re: [Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Matthew Gardiner]

Hi Shawn,

In the patch should not the call to SyncIOHandler in Target.cpp and 
CommandObjectProcess.cpp be inside the


if (error.Success())
{

?

My thinking that is, if the resume operation reports a failure then, 
presumably the running event won't be delivered and we won't 
PushIOHandler, and flag the condition to unblock the waiting thread. So 
in such a situation the main thread would be unnecessarily blocked.


What are your thoughts?

Matt



Shawn Best wrote:
Thanks for the feedback Greg.  I have attached a revised patch based 
on your suggestions.


- renamed m_pushed_IOHandler to m_iohandler_sync

- got rid of method ClearPushedIOHandlerSync() and make calls directly 
to m_iohandler_sync.SetValue(false..)


- renamed WaitForPushedIOHandlerSync() to SyncIOHandler()

- only wait in case where m_process_input_reader != NULL

I put the calls to reset the sync flag in both PrivateEventThread 
(after it has seen a public stop), and after the call to 
SyncIOHandler() is completed.  As far as I can tell it should work 
fine, but my preference is normally to reset the flag immediately 
before using it instead of relying on it set to false.  Having it 
internal does clean up the code though.


I think the last suggestion of moving the sync point to 
Debugger::HandleProcessEvent() would defeat the purpose of the patch 
since it is called from the EventHandler thread.  The race condition 
is the main thread returning up the call stack to start another round 
of commandIO handling before PushProcessIOHandler() gets called.



On Fri, Aug 1, 2014 at 10:39 AM, Greg Clayton gclay...@apple.com 
mailto:gclay...@apple.com wrote:


Comments:

Why should anyone outside of lldb_private::Process have to call
ClearPushedIOHandlerSync() manually? Can we get rid of this
function and just have Process.cpp do it at the right times by
directly calling m_pushed_IOHandler.SetValue(false, eBroadcastNever);?

If so then the following fixes apply:
1 - remove Process::ClearPushedIOHandlerSync() since it will be
done internally within process.
2 - rename m_pushed_IOHandler to m_iohandler_sync
3 - rename WaitForPushedIOHandlerSync() to SyncIOHandler

Other general fixes:
1 - the WaitForPushedIOHandlerSync should do nothing if there is
no process IOHandler (no stdio or we attached to a process. This
can be done by testing m_process_input_reader with if
(m_process_input_reader) { ... }

I would also like the fix this sync issue by not having to have
every command add a call to
process-WaitForPushedIOHandlerSync(...). Can't we sync this in
the Debugger::HandleProcessEvent()?

 On Jul 31, 2014, at 2:57 PM, Shawn Best sb...@blueshiftinc.com
mailto:sb...@blueshiftinc.com wrote:

 oops, with the attachment this time.


 On Thu, Jul 31, 2014 at 2:56 PM, Shawn Best
sb...@blueshiftinc.com mailto:sb...@blueshiftinc.com wrote:
 Thanks everyone for the feedback.  I have reworked the patch to
use Predicate bool, it reads much cleaner now.

 Shawn.

 On Wed, Jul 30, 2014 at 6:34 PM, Greg Clayton
gclay...@apple.com mailto:gclay...@apple.com wrote:
 You will want to use a Predicatebool here in stead of what you
have since it is exactly what we use a predicate for. The following:

 +bool  m_process_running_sync; // used with
WaitForProcessRunning() synchronization
 +std::condition_variable m_condition_process_running;//
used with WaitForProcessRunning() synchronization
 +std::mutex  m_mutex_process_running;// used with
WaitForProcessRunning() synchronization

 Is exactly what the Predicate class does: protect a value with a
mutex and condition.

 The above code should be replaced with:

  Predicatebool m_process_running_sync;

 The API on Predicate should do what you want. See the header
file at lldb/Host/Predicate.h and also look for other places
that use this class to wait for a value to be equal to another
value, or wait for a value to not be equal to something.

 Let me know when you have a patch that uses Predicate and we
will look at that.

 Greg

  On Jul 30, 2014, at 4:03 PM, Shawn Best
sb...@blueshiftinc.com mailto:sb...@blueshiftinc.com wrote:
 
  I have reworked the patch to use std::condition_variable.
 This particular sync mechanism was new to me, I hope I used it
correctly.  Is it portable across all target platforms/compilers?
 I tested on linux and OSX.
 
  The timeout is pretty small (1ms) but seems ample based on the
measurements I made.
 
 
  On Tue, Jul 29, 2014 at 9:58 PM, Matthew Gardiner
m...@csr.com mailto:m...@csr.com wrote:
  Cool, let us know how you get on!
  Matt
 
  Shawn Best wrote:
  Thanks for the feedback guys.
 
  Studying the code, I had figured going

Re: [Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Matthew Gardiner]

Thanks for this update, Jim!

Hopefully, we'll have it all fixed for when he returns ;-)

Matt


jing...@apple.com wrote:

Note, Greg is on vacation this week.  Dunno if he'll chime in from the beach, 
but he's sure to have opinions.  Just wanted to make sure folks didn't think 
his silence indicated he didn't...

Jim


On Aug 4, 2014, at 5:41 AM, Matthew Gardiner m...@csr.com wrote:

Folks,

I have been testing Shawn's patch some more today.

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf
...
(lldb) process launch -s
...
lldb) process continue
... lots of output ...

Control-c pressed
(lldb)

Above I found that if I launch the process with stop at entry, then I continue, then 
when I control-c to interrupt the prompt returns correctly. With the above session I found I could 
process continue and Ctrl-c repeatedly and the prompt would return as expected. I also found that 
using next to step the inferior also resulted in the debugger's prompt returning.

However, if I launched without -s

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf
...
(lldb) process launch
... lots of output ...

Control-c pressed

then the prompt was absent, and hence broken.

Also if I create my target, set a breakpoint, then launch:

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf
...
(lldb) br s -f forever.c -l 12
...
(lldb) process launch
Process 10292 launching
Process 10292 launched: 
'/home/mg11/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf'
 (x86_64)
Process 10292 stopped
* thread #1: tid = 10292, 0x003675a011f0, name = 'i64-hello.elf'
frame #0:
Process 10292 stopped
* thread #1: tid = 10292, 0x00400548 i64-hello.elf`f2(x=8, y=0) + 24 at 
forever.c:12, name = 'i64-hello.elf', stop reason = breakpoint 1.1
frame #0: 0x00400548 i64-hello.elf`f2(x=8, y=0) + 24 at forever.c:12
   9
   10  x += 8;
   11  y *= x;
- 12  buff[0] = y;
   13
   14  x = buff[0] + buff[1];
   15  return x;

Again, no prompt.

Attaching to a stopped process using gdb-remote, also results in me having no 
prompt:

(lldb) target create 
~/src/main/devtools/main/heracles/csrgdbserver/test/examples/simple/i64-hello.elf
...
(lldb) gdb-remote 
Process 10327 stopped
* thread #1: tid = 10327, 0x003675a011f0, name = 'i64-hello.elf', stop 
reason = signal SIGSTOP
frame #0: 0x003675a011f0
- 0x3675a011f0:  movq   %rsp, %rdi
   0x3675a011f3:  callq  0x3675a046e0
   0x3675a011f8:  movq   %rax, %r12
   0x3675a011fb:  movl   0x21eb97(%rip), %eax

So, as I alluded to previously, there are a lot of scenarios where the 
IOHandler prompt bug manifests. Now that I have read the code, I'm surprised 
that it does not happen more often and on more architectures. Out of interest 
my test inferior basically spins producing lots of output. It *may* one reason 
why I can provoke these bugs so easily. I'm attaching my test code if this 
helps others witness all of the issues which I'm seeing. I'll see if there are 
any simple tweaks I can make (in the spirit of Shawn's patch) to fix the above 
bugs.

Matt


Matthew Gardiner wrote:

I think the last suggestion of moving the sync point to 
Debugger::HandleProcessEvent() would defeat the purpose of the patch since it is called 
from the EventHandler thread.  The race condition is the main thread returning up the 
call stack to start another round of commandIO handling before PushProcessIOHandler() 
gets called.

I agree with this remark of Shawn's. The whole point of this issue is to block 
the main thread i.e. the one which calls IOHandlerEditline::GetLine, which is 
where the (lldb) prompt is printed, i.e.

#0  lldb_private::IOHandlerEditline::GetLine
#1  0x7770aceb in lldb_private::IOHandlerEditline::Run
#2  0x776f546a in lldb_private::Debugger::ExecuteIOHanders
#3  0x777d78bb in 
lldb_private::CommandInterpreter::RunCommandInterpreter
#4  0x77a47955 in lldb::SBDebugger::RunCommandInterpreter
#5  0x00409234 in Driver::MainLoop
#6  0x00409572 in main

I spent quite a while looking at this problem and the surrounding architecture 
this morning. The observed problem occurs when the Editline's IOHandler object 
does not have it's m_active flag cleared before it gets it's Run method called 
again (i.e. after the last user command is processed). So we have to block this 
main thread until another IOHandler can be pushed (i.e. the process one). It is 
the act of pushing an IOHandler which has the effect of clearing the m_active 
flag in the previous topmost handler.

Regarding Greg's comment:

not having to have every command add a call to 
process-WaitForPushedIOHandlerSync(...).

The problem is that it is only the individual Command object implementations that know 
whether asynchronous processing will occur

[Lldb-commits] [lldb] r214480 - Change the encoding of the Triple string exchanged across GDB-RSP

[Matthew Gardiner]

Author: mg11
Date: Fri Aug  1 00:12:23 2014
New Revision: 214480

URL: http://llvm.org/viewvc/llvm-project?rev=214480view=rev
Log:
Change the encoding of the Triple string exchanged across GDB-RSP
and update documentation to suit, as suggested by Jason Molenda and
discussed in:

http://lists.cs.uiuc.edu/pipermail/lldb-commits/Week-of-Mon-20140721/011978.html

Differential Revision: http://reviews.llvm.org/D4704


Modified:
lldb/trunk/docs/lldb-gdb-remote.txt

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

Modified: lldb/trunk/docs/lldb-gdb-remote.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/lldb-gdb-remote.txt?rev=214480r1=214479r2=214480view=diff
==
--- lldb/trunk/docs/lldb-gdb-remote.txt (original)
+++ lldb/trunk/docs/lldb-gdb-remote.txt Fri Aug  1 00:12:23 2014
@@ -526,7 +526,6 @@ Key value pairs are one of:
 cputype: is a number that is the mach-o CPU type that is being debugged (base 
10)
 cpusubtype: is a number that is the mach-o CPU subtype type that is being 
debugged (base 10)
 triple: a string for the target triple (x86_64-apple-macosx) that can be used 
to specify arch + vendor + os in one entry
-The triple string must be sent as hexadecimal bytes, for the above example 
this being 7838365f36342d6170706c652d6d61636f7378 
 vendor: a string for the vendor (apple), not needed if triple is specified
 ostype: a string for the OS being debugged (darwin, linux, freebsd), not 
needed if triple is specified
 endian: is one of little, big, or pdp
@@ -1110,7 +1109,7 @@ for this region.
 //  all_users   bool  A boolean value that specifies if processes should
 //  be listed for all users, not just the user that 
the 
 //  platform is running as
-//  triple  ascii-hex An ASCII hex target triple string (x86_64, 
+//  triple  stringAn ASCII triple string (x86_64, 
 //  x86_64-apple-macosx, armv7-apple-ios)
 //
 // The response consists of key/value pairs where the key is separated from the
@@ -1120,9 +1119,9 @@ for this region.
 //
 // Sample packet/response:
 // send packet: $qfProcessInfo#00
-// read packet: 
$pid:60001;ppid:59948;uid:7746;gid:11;euid:7746;egid:11;name:6c6c6462;triple:7838365f36342d6170706c652d6d61636f7378;#00
+// read packet: 
$pid:60001;ppid:59948;uid:7746;gid:11;euid:7746;egid:11;name:6c6c6462;triple:x86_64-apple-macosx;#00
 // send packet: $qsProcessInfo#00
-// read packet: 
$pid:59992;ppid:192;uid:7746;gid:11;euid:7746;egid:11;name:6d64776f726b6572;triple:7838365f36342d6170706c652d6d61636f7378;#00
+// read packet: 
$pid:59992;ppid:192;uid:7746;gid:11;euid:7746;egid:11;name:6d64776f726b6572;triple:x86_64-apple-macosx;#00
 // send packet: $qsProcessInfo#00
 // read packet: $E04#00
 //--
@@ -1190,11 +1189,11 @@ for this region.
 //  euidinteger   A string value containing the decimal effective 
user ID
 //  egidinteger   A string value containing the decimal effective 
group ID
 //  nameascii-hex An ASCII hex string that contains the name of the 
process
-//  triple  ascii-hex A target triple (x86_64-apple-macosx, 
armv7-apple-ios)
+//  triple  stringA target triple (x86_64-apple-macosx, 
armv7-apple-ios)
 //
 // Sample packet/response:
 // send packet: $qProcessInfoPID:60050#00
-// read packet: 
$pid:60050;ppid:59948;uid:7746;gid:11;euid:7746;egid:11;name:6c6c6462;triple:7838365f36342d6170706c652d6d61636f7378;#00
+// read packet: 
$pid:60050;ppid:59948;uid:7746;gid:11;euid:7746;egid:11;name:6c6c6462;triple:x86_64-apple-macosx;#00
 //--
 
 //--

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=214480r1=214479r2=214480view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Fri Aug  1 00:12:23 2014
@@ -1642,10 +1642,7 @@ GDBRemoteCommunicationClient::GetHostInf
 }
 else if (name.compare(triple) == 0)
 {
-// The triple comes as ASCII hex bytes since it 
contains '-' chars
-extractor.GetStringRef().swap(value);
-extractor.SetFilePos(0);
-extractor.GetHexByteString (triple);
+triple.swap(value);
   

Re: [Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Matthew Gardiner]

Hi Shawn,

Yes, the condition variable paradigm can initially be a little tricky to 
grasp. Have a look at something like 
http://pages.cs.wisc.edu/~remzi/OSTEP/threads-cv.pdf, first. I think the 
text on page on page 14 sums up the concept best. You should probably 
study it's operation in a simple test program first.


The concept, IIRC, is as follows: One (or more) threads needs to wait 
for a condition. I think in our case this is the main command thread, 
and it needs to wait till worker thread has completed and has pushed the 
IOHandler. Instead of having the main thread sleeping a fixed interval, 
you let it sleep (possibly for a infinite timeout) and wake when a 
condition occurs (e.g. a flag is set, or variable exceeds a particular 
value). The other thread (e.g. the one pushing the IOHandler) then sets 
the signal once the condition has arisen. The point is, we wait for a 
signal, *not* a timeout - the timeout is just (sometimes) there for if 
things go wrong, e.g. the condition is never met - and in that case this 
timeout/error condition would need to be dealt with (by the waiting 
thread) correctly (e.g. an error message preceding the printing of the 
prompt).


Greg Clayton's reply to you suggests that you use 
lldb/Host/Predicate.h. I've not used this myself, but I took a quick 
look and it looks like it wraps up the above idea. I *think* that using 
this, the main thread (the one waiting) will call one of the Predicate 
class's Waitxxx functions, and the other thread will call on of the 
Setxxx when it's ok for the waiter to wake.


I'd suggest you get your head round conditional variables first, then 
check out using lldb/Host/Predicate.h in the lldb code.


Hope this helps! Keep us posted.

Matt


Shawn Best wrote:
I have reworked the patch to use std::condition_variable.  This 
particular sync mechanism was new to me, I hope I used it correctly.  
Is it portable across all target platforms/compilers?  I tested on 
linux and OSX.


The timeout is pretty small (1ms) but seems ample based on the 
measurements I made.



On Tue, Jul 29, 2014 at 9:58 PM, Matthew Gardiner m...@csr.com 
mailto:m...@csr.com wrote:


Cool, let us know how you get on!
Matt

Shawn Best wrote:

Thanks for the feedback guys.

Studying the code, I had figured going with a straight int
would in practice be most efficient and not run into
multi-threaded problems, even if initially appearing a bit
risky.  I will rework it to use a std::condition_variable.
 That will be more robust and readable.

Shawn.

On 7/29/2014 10:53 AM, Zachary Turner wrote:

Even better would be an std::condition_variable


On Mon, Jul 28, 2014 at 10:30 PM, Matthew Gardiner
m...@csr.com mailto:m...@csr.com mailto:m...@csr.com
mailto:m...@csr.com wrote:

Hi Shawn,

I use 64-bit linux and I see this issue a lot. It usually
manifests itself as the prompt just not being printed
(or perhaps
it just gets overwritten) - regardless - I invoke a
command, and
I don't see an (lldb) prompt when I should. So I'm
well pleased
that you are looking at this!

Would it not be more robust to use a semaphore than
usleep to
synchronise the problematic threads?

Although I've not looked too deeply into this
particular issue,
whenever I've seen similar races, I found that it's almost
impossible to pick the right value when using a sleep
command. A
semaphore, though, should always ensure the waiting
thread will
wake precisely.

I'd be happy to help to test such a fix.

Matt


Shawn Best wrote:

Hi,

I have attached a patch which addresses 3 related race
conditions that cause the command line (lldb)
prompt to get
displayed inappropriately and make it appear it is not
working correctly.  This issue can be seen on
linux and
FreeBSD.  I can also artificailly induce the
problem on OSX.

The issue happens when the command handler (in the
main
thread) issues a command such as run, step or
continue.
 After the command finishes initiating its action,
it returns
up the call stack and goes back into the main
command loop
waiting for user input.  Simultaneously, as the
inferior
process starts up, the MonitorChildProcess thread
picks up
the change and posts

Re: [Lldb-commits] [PATCH] Improvement for lldb-gdb-remote.txt qHostInfo documentation

[Matthew Gardiner]

Guys,

I'm going to limit the scope of my current work to just fixing the 
triple (i.e. change it to plaintext string encoding, and the revert 
documentation). I'm unclear of the intended encoding of the other items 
we'd mentioned. I'll come back to them later.


Matt

Todd Fiala wrote:




On Mon, Jul 28, 2014 at 2:25 AM, Matthew Gardiner m...@csr.com 
mailto:m...@csr.com wrote:


Todd/Jason,

I'm happy to do this work, i.e. changes to
GDBRemoteCommunicationServer.cpp and
GDBRemoteCommunicationClient.cpp. (And to change the kalimba stub
is easy). As I have just acquired commit access to lldb this could
be a useful piece of work to introduce me to the Phabricator
review process.


Great, have at it!

A couple of questions remain, though.

Todd's comment:


Are there any ramifications for
your usage of lldb-platform, though? We’d need to change the
receiver code
of that, which would then differ based on which lldb-platform version
you’re talking to.


Is that meaning that there are installed lldb-platform binaries
which will now be broken against a version of lldb which expects
plain text triples? Is that a problem? (that is, __will__ people
upgrade both lldb and lldb-platform separately).


Matthew, this was my misunderstanding.  I had thought lldb-platform 
shipped with all the Apple platforms.  They fixed my understanding - 
it only gets downloaded to devices as needed but doesn't live there. 
 So this is a non-issue.  As for llgs using it, that should not be an 
issue as llgs is just getting functional now and backwards support 
isn't a concern just yet.


Todd, do you know if distribution_id, os_build, os_kernel need to
converted to plain-text?


I added distribution_id to indicate the Linux distribution, since it 
looks like on Android that might be the only way I can tell Android 
and its Androidisms apart from a stock Linux.  On Linux, it grabs the 
content from the lsb_release exe, which really could be anything.  If 
we wanted to switch that to binary encoding, that should be fine.  You 
could change that one.


Somebody else should probably comment on os_build/os_kernel.

Jason, what is the problem with hostname? Previously you wrote
that we are dealing with a user-specified string that may include
non-alphanumeric characters. So in lldb/GDB-RSP context is a
hostname different than that defined in
http://tools.ietf.org/html/rfc952?

Matt

Todd Fiala wrote:

Cool - sounds all good to me since we don't have a backwards
compatibility problem.  Now would be the time to do it :-)

-Todd



On Fri, Jul 25, 2014 at 12:40 PM, Jason Molenda
ja...@molenda.com mailto:ja...@molenda.com
mailto:ja...@molenda.com mailto:ja...@molenda.com wrote:


On Jul 25, 2014, at 7:08 AM, Todd Fiala tfi...@google.com
mailto:tfi...@google.com
mailto:tfi...@google.com mailto:tfi...@google.com wrote:

 Hey Jason,

 I don’t know about llgs or how much work it would be to
change
the kalimba gdbserver stub.

 Currently GDBRemoteCommunicationServer, used by both
llgs and
lldb-platform, does send the triple as hex encoded via
this code:

 response.PutCString(triple:);

response.PutCStringAsRawHex8(host_triple.getTriple().c_str());

 We can easily not do that as you suggest. Are there any
ramifications for your usage of lldb-platform, though?
We’d need
to change the receiver code of that, which would then
differ based
on which lldb-platform version you’re talking to.

It sounds like Matthew is open to changing the kalimba
stub.  We
should get rid of the hex-ascii strings (along with
distribution_id, os_build, os_kernel) everywhere.
 lldb-platform
is not bundled/distributed in any products so we don't have to
worry about deployed versions.

hostname is less clear-cut because there we're dealing with a
user-specified string and that may include one of # $ } *.

Personally, I wish the whole of gdb-remote protocol
required the
use of the binary packet escape protocol which says that
any of
these 4 metacharacters that is meant to be sent in the
body of a
packet is prefixed with } and is xor'ed with 0x20.  But
that's not
what the protocol says so we need to do these things..




-- 
Todd Fiala | Software Engineer | tfi...@google.com

mailto:tfi...@google.com mailto:tfi...@google.com
mailto:tfi...@google.com | 650-943-3180 tel:650-943-3180




To report this email as spam click here
https

Re: [Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Matthew Gardiner]

Cool, let us know how you get on!
Matt

Shawn Best wrote:

Thanks for the feedback guys.

Studying the code, I had figured going with a straight int would in 
practice be most efficient and not run into multi-threaded problems, 
even if initially appearing a bit risky.  I will rework it to use a 
std::condition_variable.  That will be more robust and readable.


Shawn.

On 7/29/2014 10:53 AM, Zachary Turner wrote:

Even better would be an std::condition_variable


On Mon, Jul 28, 2014 at 10:30 PM, Matthew Gardiner m...@csr.com 
mailto:m...@csr.com wrote:


Hi Shawn,

I use 64-bit linux and I see this issue a lot. It usually
manifests itself as the prompt just not being printed (or perhaps
it just gets overwritten) - regardless - I invoke a command, and
I don't see an (lldb) prompt when I should. So I'm well pleased
that you are looking at this!

Would it not be more robust to use a semaphore than usleep to
synchronise the problematic threads?

Although I've not looked too deeply into this particular issue,
whenever I've seen similar races, I found that it's almost
impossible to pick the right value when using a sleep command. A
semaphore, though, should always ensure the waiting thread will
wake precisely.

I'd be happy to help to test such a fix.

Matt


Shawn Best wrote:

Hi,

I have attached a patch which addresses 3 related race
conditions that cause the command line (lldb) prompt to get
displayed inappropriately and make it appear it is not
working correctly.  This issue can be seen on linux and
FreeBSD.  I can also artificailly induce the problem on OSX.

The issue happens when the command handler (in the main
thread) issues a command such as run, step or continue.
 After the command finishes initiating its action, it returns
up the call stack and goes back into the main command loop
waiting for user input.  Simultaneously, as the inferior
process starts up, the MonitorChildProcess thread picks up
the change and posts to the PrivateEvent thread.
 HandePrivateEvent() then calls PushProcessIOHandler() which
will disable the command IO handler and give the inferior
control of the TTY.  To observe this on OSX, put a

usleep(100);

immediately prior the PushProcessIOHandler() in
HandlePrivateEvent.


My proposed solution is that after a 'run', 'step', or
'continue' command, insert a synchronization point and wait
until HandlePrivateEvent knows the inferior process is
running and has pushed the IO handler.  One context switch
(100us) is usually all the time it takes on my machine.  As
an additional safety, I have a timeout (currently 1ms) so it
will never hang the main thread.

Any thoughts, or suggestions would be appreciated.

Regards,
Shawn.


To report this email as spam click here
https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ==.



___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu mailto:lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits




Member of the CSR plc group of companies. CSR plc registered in
England and Wales, registered number 4187346, registered office
Churchill House, Cambridge Business Park, Cowley Road, Cambridge,
CB4 0WZ, United Kingdom
More information can be found at www.csr.com
http://www.csr.com. Keep up to date with CSR on our technical
blog, www.csr.com/blog http://www.csr.com/blog, CSR people
blog, www.csr.com/people http://www.csr.com/people, YouTube,
www.youtube.com/user/CSRplc http://www.youtube.com/user/CSRplc,
Facebook, www.facebook.com/pages/CSR/191038434253534
http://www.facebook.com/pages/CSR/191038434253534, or follow us
on Twitter at www.twitter.com/CSR_plc
http://www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products
powered by aptX at www.aptx.com http://www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu mailto:lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits






___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] Improvement for lldb-gdb-remote.txt qHostInfo documentation

[Matthew Gardiner]

Hi Jason,

Indeed my stub implementation does not rely on the hex/otherwise 
encoding of the triple. I'm happy to revert the patch and change the 
implementation in the GDBRemote code (probably not till next week, 
though). My main regret is not floating my patch on lldb-dev first, but 
I assumed that the implementation was set in stone at that stage.


While we are at it, I also note that distribution_id, os_build, 
hostname, os_kernel are also currently hex encoded in qHostInfo 
response, so I'm now curious as to the encoding rationale to those datas 
too...


Matt


Jason Molenda wrote:

Hi Matthew,

If your stub implementation is not locked into hex encoding this triple string, let's 
change the lldb / stub implementation and the documentation.  It's a mistake and I'd hate 
to propagate it further.  Yeah, the - and + characters are used for ACK / NACK when a 
packet consists only of that character but normal packets are bounded by # and $ so we 
don't need to worry about + and -.  (plus lldb prefers to go into no-ack mode 
as soon as possible anyway :)  We've already got a ton of randomness in the remote 
protocol because of bugs which get deployed and we need to support them forever.

Thanks!

On Jul 24, 2014, at 10:22 PM, Matthew Gardiner m...@csr.com wrote:


Hi Jason,

Having the triple hex encoded did confuse me for a while in getting my 
architecture's stub up and working properly, when the previous documentation 
suggested a plain textual representation.

The comment in GDBRemoteCommunicationClient .cpp // The triple comes as ASCII hex 
bytes since it contains '-' chars got me thinking, and I discovered that '-' is 
used as the *first* byte in a packet to specify a NACK (in section E.11 Packet 
Acknowledgement of the GDB documentation).

I agree that the need to escape the '-' is probably unfounded since in the case 
of qHostInfo the '-' is wedged between $ and # characters anyway.

I'm happy to proceed with either presentation - so long as the documentation is 
correct.

Matt


Jason Molenda wrote:

The change to make the documentation match the implementation is fine.

But I don't see why the triple is sent hex encoded.  The comment  hex encoding 
was added in r128193 from March 23 2011 by Greg Clayton.  AFAIK '-' is not special 
in gdb-remote protocol.  Characters that can be a problem are $, #, *, }.

Greg?

J



On Jul 23, 2014, at 6:23 AM, Matthew Gardiner m...@csr.com wrote:

Folks,

When I first implemented the qHostInfo for the kalimba gdbserver stub, I sent 
the triple string as kalimba-csr-unknown. However the receiving code 
(GDBRemoteCommunicationClient) expects the string to be encoded as hex bytes, 
since '-' are a reserved GDB-RSP control character.

The documentation for qHostInfo in lldb-gdb-remote.txt does not allude to this 
at all. So I'm sending this patch which I think improves this state of affairs.

Could someone please apply this patch?

thanks Matt


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
lldb-gdb-remote.patch___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits


  To report this email as spam click 
https://www.mailcontrol.com/sr/k!++gfQa3yzGX2PQPOmvUtVTDJsKpCsgiT6Sr2CBa5VmIDKRAkOMY8LPYX0hRhQfFVYH2!a+sDSMQcgptJ50Pg==
 .

___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits


___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] Add kalimba architecture checking to TestImageListMultiArchitecture

[Matthew Gardiner]

Hi folks,

Since I have recently added CSR's kalimba as an architecture to lldb, we 
thought it would be wise to enhance TestImageListMultiArchitecture to 
check that kalimba ELFs are identified correctly to avoid any regression.


I'm uploading a patch to add this test functionality.

In addition to modifying the .py file, I'm adding a .c file since the 
kalimba compiler (kcc) does not support C++. Additionally I'm attaching 
a gzipped ELF since I'm not convinced that svn di is capable of 
building my added binary into the patch file.


Could someone please submit my patch if it seems ok?

thanks Matt

Index: test/functionalities/object-file/TestImageListMultiArchitecture.py
===
--- test/functionalities/object-file/TestImageListMultiArchitecture.py 
(revision 213650)
+++ test/functionalities/object-file/TestImageListMultiArchitecture.py 
(working copy)

@@ -27,6 +27,7 @@
 hello-netbsd-6.1-x86_64-gcc-4.5.3: 
re.compile(rx86_64-(unknown)?-netbsd x86_64),
 hello-ubuntu-14.04-x86_64-gcc-4.8.2: 
re.compile(rx86_64-(unknown)?-linux x86_64),
 hello-ubuntu-14.04-x86_64-clang-3.5pre: 
re.compile(rx86_64-(unknown)?-linux x86_64),
+hello-unknown-kalimba_arch4-kcc-36: 
re.compile(rkalimba-csr-unknown kalimba),

 }

 for image_name in images:
Index: 
test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36

===
Cannot display: file marked as a binary type.
svn:mime-type = application/x-executable
Index: 
test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36

===
--- 
test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36 
(revision 0)
+++ 
test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36 
(working copy)


Property changes on: 
test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36

___
Added: svn:executable
## -0,0 +1 ##
+*
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+application/x-executable
\ No newline at end of property
Index: test/functionalities/object-file/bin/hello.c
===
--- test/functionalities/object-file/bin/hello.c(revision 0)
+++ test/functionalities/object-file/bin/hello.c(working copy)
@@ -0,0 +1,8 @@
+#include stdio.h
+
+int main(int argc, char **argv)
+{
+   printf(Hello, world\n);
+   return 0;
+}
+



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: test/functionalities/object-file/TestImageListMultiArchitecture.py
===
--- test/functionalities/object-file/TestImageListMultiArchitecture.py	(revision 213650)
+++ test/functionalities/object-file/TestImageListMultiArchitecture.py	(working copy)
@@ -27,6 +27,7 @@
 hello-netbsd-6.1-x86_64-gcc-4.5.3: re.compile(rx86_64-(unknown)?-netbsd x86_64),
 hello-ubuntu-14.04-x86_64-gcc-4.8.2: re.compile(rx86_64-(unknown)?-linux x86_64),
 hello-ubuntu-14.04-x86_64-clang-3.5pre: re.compile(rx86_64-(unknown)?-linux x86_64),
+hello-unknown-kalimba_arch4-kcc-36: re.compile(rkalimba-csr-unknown kalimba),
 }
 
 for image_name in images:
Index: test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36
===
Cannot display: file marked as a binary type.
svn:mime-type = application/x-executable
Index: test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36
===
--- test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36	(revision 0)
+++ test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36	(working copy)

Property changes on: test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36
___
Added: svn:executable
## -0,0 +1 ##
+*
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+application/x-executable
\ No newline at end of property
Index: test/functionalities/object-file/bin/hello.c

[Lldb-commits] [PATCH] Improvement for lldb-gdb-remote.txt qHostInfo documentation

[Matthew Gardiner]

Folks,

When I first implemented the qHostInfo for the kalimba gdbserver stub, I 
sent the triple string as kalimba-csr-unknown. However the receiving 
code (GDBRemoteCommunicationClient) expects the string to be encoded as 
hex bytes, since '-' are a reserved GDB-RSP control character.


The documentation for qHostInfo in lldb-gdb-remote.txt does not allude 
to this at all. So I'm sending this patch which I think improves this 
state of affairs.


Could someone please apply this patch?

thanks Matt


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: docs/lldb-gdb-remote.txt
===
--- docs/lldb-gdb-remote.txt	(revision 213650)
+++ docs/lldb-gdb-remote.txt	(working copy)
@@ -526,6 +526,7 @@
 cputype: is a number that is the mach-o CPU type that is being debugged (base 10)
 cpusubtype: is a number that is the mach-o CPU subtype type that is being debugged (base 10)
 triple: a string for the target triple (x86_64-apple-macosx) that can be used to specify arch + vendor + os in one entry
+The triple string must be sent as hexadecimal bytes, for the above example this being 7838365f36342d6170706c652d6d61636f7378 
 vendor: a string for the vendor (apple), not needed if triple is specified
 ostype: a string for the OS being debugged (darwin, linux, freebsd), not needed if triple is specified
 endian: is one of little, big, or pdp
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] Add kalimba architecture checking to TestImageListMultiArchitecture

[Matthew Gardiner]

Todd Fiala wrote:

And meant to add submitted -

svn commit
Sending 
 test/functionalities/object-file/TestImageListMultiArchitecture.py
Adding  (bin) 
 test/functionalities/object-file/bin/hello-unknown-kalimba_arch4-kcc-36

Adding test/functionalities/object-file/bin/hello.c
Transmitting file data ...
Committed revision 213763.




Yup, thanks for getting this test in!

Matt


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] Improvement for lldb-gdb-remote.txt qHostInfo documentation

[Matthew Gardiner]

Todd Fiala wrote:

Done:
svn commit
Sendingdocs/lldb-gdb-remote.txt
Transmitting file data .
Committed revision 213756.




Thanks again!
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r213158 - Add kalimba as a platform.

[Matthew Gardiner]

Todd Fiala wrote:

Hi Matthew,

On Thu, Jul 17, 2014 at 10:30 PM, Matthew Gardiner m...@csr.com 
mailto:m...@csr.com wrote:


Todd Fiala wrote:

Hey Matthew,

I'm thrilled that you are trying to get a test Linux box up!

The easiest way to run that depends on whether you're using
cmake/ninja or configure/make.  If you don't have a strong
preference, going with cmake/ninja is definitely the faster
way to go.

In any event, you'll want to kick off the tests with either
one of these, after you've done a full build (i.e. 'ninja' in
the build dir or 'make' in the build dir):

cmake/ninja:
cd /your/build/dir
ninja check-lldb

I think I'm going to stick with make for now, since I've never had
any luck with cmake/ninja.


Ok.  I'm eventually going to document our setup, at which point you 
can maybe try out cmake again.  Using ninja as the builder is 
significantly faster (I think it was 20%+ faster for us over make).


configure/make:
cd /your/llvm/dir
cd tools/lldb/test
make


Ok... From what you've just said. Does that mean I'll running the
make in that tools/lldb/test runs the swig and python required to
get lldb.py built?


Assuming I have the typical llvm, llvm/tools/clang and llvm/tools/lldb 
directory structure for the source, I make a sibling directory to llvm 
called build.  I cd into that, type something like:

../llvm/configure --prefix=`pwd`/../install

Then I build with:
make -j32

(You'll want a j value that is something appropriate for your # cores 
- I typically use n, where n is the number of cores I have, 32 in this 
case).


That will build everything, including the lldb python module, 
liblldb.so (referenced by the lldb python module), and all the lldb exes.


Then I run make in the  tools/lldb/test directory with this:
make -C tools/lldb/test

That should run all the tests for you.

If I got any of the details wrong, I think it is covered here under 
'To build with autoconf' (including the autoconf-based testing):


http://lldb.llvm.org/build.html

Hope that helps!


Both those incantations will get you a test run that does the
tests without you needing to set up anything else (e.g.
lldb/python paths, or architecture settings for the test run).
 They'll also run the tests faster if you have multiple cores
on your dev box/VM.

Let me know if you hit any trouble with that.


Yeah - will do. I'm going to try to focus this today.


Good luck!


Particularly the cmake configuration line - the basic cmake
configuration line with minimal options has never worked well
for me on Linux, so I call it with a bunch of flags to specify
the llvm build type and a few other details.

No. I've never had much luck with cmake :-(


Yeah, it took me several days to get my cmake setup working back on 
Ubuntu 12.04.  It's a bit easier on Ubuntu 14.04.


Anyway, I'm kicking off a fresh lldb build (i.e. configure in the
out-of-tree build dir, then a make) now. And I'll postback later
on today regarding my progress.

Later

Matt


Let me know how it goes!


I just got this:


Well, so my regular llvm/lldb build completed:

So I cd to the tools/lldb/test in my *build* directory, then run make

~/src/main/build/tools/lldb/test

[0][][main][]  make
rm -rf lldb-test-traces
python /home/mg11/src/main/llvm/tools/lldb/test/dosep.ty -o 
--executable /home/mg11/src/main/build/Debug+Asserts/bin/lldb -q -s 
lldb-test-traces -u CXXFLAGS -u CFLAGS -C gcc
This script requires lldb.py to be in either 
/home/mg11/src/main/llvm/tools/lldb/build/Debug/LLDB.framework/Resources/Python, 
/home/mg11/src/main/llvm/tools/lldb/build/Release/LLDB.framework/Resources/Python, 
or 
/home/mg11/src/main/llvm/tools/lldb/build/BuildAndIntegration/LLDB.framework/Resources/Python

...
This script requires lldb.py to be (this line being repeated lot)
...
...
...
Ran 299 tests.
Failing Tests (299)
FAIL: LLDB (suite) :: TestSTTYBeforeAndAfter.py (Linux 
mg11-fc20.root.pri 3.13.7-200.fc20.x86_64 #1 SMP Mon Mar 24 22:01:49 UTC 
2014 x86_64 x86_64)
FAIL: LLDB (suite) :: TestLogging.py (Linux mg11-fc20.root.pri 
3.13.7-200.fc20.x86_64 #1 SMP Mon Mar 24 22:01:49 UTC 2014 x86_64 x86_64)
FAIL: LLDB (suite) :: TestSourceManager.py (Linux mg11-fc20.root.pri 
3.13.7-200.fc20.x86_64 #1 SMP Mon Mar 24 22:01:49 UTC 2014 x86_64 x86_64)

...
presumably all the failed tests are now being listed?
FAIL: LLDB (suite) :: TestPluginCommands.py (Linux mg11-fc20.root.pri 
3.13.7-200.fc20.x86_64 #1 SMP Mon Mar 24 22:01:49 UTC 2014 x86_64 x86_64)

make: *** [check-local] Error 1

[0][][main][] 

So it looks as if all my tests failed. Does lldb i.e. 
~/src/main/build/Debug+Asserts/bin need to be in the path when running 
make in the test dir?


Matt



Member of the CSR plc group of companies. CSR plc registered in England

[Lldb-commits] [PATCH] Submission request for the PlatformKalimba plugin

[Matthew Gardiner]

Could somebody please submit this attached patch for me? It comprises of 
additions and some minor changes in order that kalimba is listed as a 
supported platform and that debugging any kalimbas results in 
PlatformKalimba being associated with the target. They are as follows:


* The PlatformKalimba implementation itself
* A tweak to ArchSpec
* .note parsing for Kalimba in ObjectFileELF.cpp
* Plugin registration
* Makefile additions

Information regarding the proposed patch has been on lldb-dev for a 
couple of days, and I believe that my latest amendments have not 
generated any controversial feedback.


thanks
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: lib/Makefile
===
--- lib/Makefile	(revision 212924)
+++ lib/Makefile	(working copy)
@@ -76,7 +76,8 @@
 	lldbPluginPlatformLinux.a \
 	lldbPluginPlatformWindows.a \
 	lldbPluginPlatformFreeBSD.a \
-	lldbPluginPlatformPOSIX.a
+	lldbPluginPlatformPOSIX.a \
+	lldbPluginPlatformKalimba.a
 
 # Because GCC requires RTTI enabled for lldbCore (see source/Core/Makefile) it is
 # necessary to also link the clang rewriter libraries so vtable references can
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp	(revision 212924)
+++ source/Core/ArchSpec.cpp	(working copy)
@@ -115,7 +115,7 @@
 { eByteOrderLittle, 4, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach32  , unknown-mach-32 },
 { eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach64  , unknown-mach-64 },
 
-{ eByteOrderLittle, 4, 1, 1 , llvm::Triple::UnknownArch , ArchSpec::eCore_kalimba  , kalimba }
+{ eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba  , kalimba }
 };
 
 struct ArchDefinitionEntry
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(revision 212924)
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(working copy)
@@ -46,6 +46,7 @@
 const char *const LLDB_NT_OWNER_FREEBSD = FreeBSD;
 const char *const LLDB_NT_OWNER_GNU = GNU;
 const char *const LLDB_NT_OWNER_NETBSD  = NetBSD;
+const char *const LLDB_NT_OWNER_CSR = csr;
 
 // ELF note type definitions
 const elf_word LLDB_NT_FREEBSD_ABI_TAG  = 0x01;
@@ -1221,7 +1222,25 @@
 if (log)
 log-Printf (ObjectFileELF::%s detected NetBSD, min version constant % PRIu32, __FUNCTION__, version_info);
 }
+// Process CSR kalimba notes
+else if ((note.n_type == LLDB_NT_GNU_ABI_TAG) 
+(note.n_name == LLDB_NT_OWNER_CSR))
+{
+// We'll consume the payload below.
+processed = true;
+arch_spec.GetTriple().setOS(llvm::Triple::OSType::UnknownOS);
+arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::CSR);
 
+// TODO At some point the description string could be processed.
+// It could provide a steer towards the kalimba variant which
+// this ELF targets.
+if(note.n_descsz)
+{
+const char *cstr = data.GetCStr(offset, llvm::RoundUpToAlignment (note.n_descsz, 4));
+(void)cstr;
+}
+}
+
 if (!processed)
 offset += llvm::RoundUpToAlignment(note.n_descsz, 4);
 }
Index: source/Plugins/Platform/CMakeLists.txt
===
--- source/Plugins/Platform/CMakeLists.txt	(revision 212924)
+++ source/Plugins/Platform/CMakeLists.txt	(working copy)
@@ -10,3 +10,4 @@
 
 add_subdirectory(POSIX)
 add_subdirectory(gdb-server)
+add_subdirectory(Kalimba)
Index: source/Plugins/Platform/Kalimba/CMakeLists.txt
===
--- source/Plugins/Platform/Kalimba/CMakeLists.txt	(revision 0)
+++ source/Plugins/Platform/Kalimba/CMakeLists.txt	(working copy)
@@ -0,0 +1,5 @@
+set(LLVM_NO_RTTI 1)
+
+add_lldb_library(lldbPluginPlatformKalimba
+  PlatformKalimba.cpp
+  )
Index: source/Plugins/Platform/Kalimba/Makefile
===
--- source/Plugins/Platform/Kalimba/Makefile	(revision 0)
+++ source/Plugins/Platform/Kalimba/Makefile	(working copy)
@@ -0,0 

Re: [Lldb-commits] [PATCH] Submission request for the PlatformKalimba plugin

[Matthew Gardiner]

Abid, Hafiz wrote:

Sorry I meant source/CMakeLists.txt.


-Original Message-
From: lldb-commits-boun...@cs.uiuc.edu [mailto:lldb-commits-
boun...@cs.uiuc.edu] On Behalf Of Abid, Hafiz
Sent: 16 July 2014 13:34
To: Matthew Gardiner; lldb-commits@cs.uiuc.edu
Subject: Re: [Lldb-commits] [PATCH] Submission request for the
PlatformKalimba plugin

Hi Matthew,
You also need to add  lldbPluginPlatformKalimba to source/CMakefileList.txt.
Otherwise cmake build will fail. After doing that your patch build OK for me
on Linux with cmake. I have not tried autotools build though.


Sorry, I only built with Make and applied best efforts to get the cmake 
stuff right.


I will commit your patch later if there are no other comments.


That would be great if you could, Abid.

Thanks
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] Addition of CSR Kalimba DSP definitions to ArchSpec

[Matthew Gardiner]

Hi folks

Would someone be able to submit this patch for me please? I have been 
trying over the past few months to extend lldb to debug CSRs Kalimba 
DSPs. I could really do with getting at least these changes upstream 
before I start pushing more features.


Index: include/lldb/Core/ArchSpec.h
===
--- include/lldb/Core/ArchSpec.h(revision 212092)
+++ include/lldb/Core/ArchSpec.h(working copy)
@@ -101,6 +101,9 @@

 eCore_uknownMach32,
 eCore_uknownMach64,
+
+eCore_kalimba,
+
 kNumCores,

 kCore_invalid,
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp(revision 212092)
+++ source/Core/ArchSpec.cpp(working copy)
@@ -113,7 +113,9 @@
 { eByteOrderLittle, 4, 4, 4, llvm::Triple::hexagon , 
ArchSpec::eCore_hexagon_hexagonv5,  hexagonv5 },


 { eByteOrderLittle, 4, 4, 4 , llvm::Triple::UnknownArch , 
ArchSpec::eCore_uknownMach32  , unknown-mach-32 },
-{ eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , 
ArchSpec::eCore_uknownMach64  , unknown-mach-64 }
+{ eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , 
ArchSpec::eCore_uknownMach64  , unknown-mach-64 },

+
+{ eByteOrderLittle, 4, 1, 1 , llvm::Triple::UnknownArch , 
ArchSpec::eCore_kalimba  , kalimba }

 };

 struct ArchDefinitionEntry
@@ -249,7 +251,9 @@
 { ArchSpec::eCore_sparc9_generic  , llvm::ELF::EM_SPARCV9, 
LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // SPARC V9
 { ArchSpec::eCore_x86_64_x86_64   , llvm::ELF::EM_X86_64 , 
LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // AMD64
 { ArchSpec::eCore_mips64  , llvm::ELF::EM_MIPS   , 
LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // MIPS
-{ ArchSpec::eCore_hexagon_generic , llvm::ELF::EM_HEXAGON, 
LLDB_INVALID_CPUTYPE, 0xu, 0xu }  // HEXAGON
+{ ArchSpec::eCore_hexagon_generic , llvm::ELF::EM_HEXAGON, 
LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // HEXAGON
+{ ArchSpec::eCore_kalimba , llvm::ELF::EM_CSR_KALIMBA, 
LLDB_INVALID_CPUTYPE, 0xu, 0xu }  // KALIMBA

+
 };

 static const ArchDefinition g_elf_arch_def = {


Please note that I have left the field llvm::Triple::ArchType as 
UnknownArch for now, as I'm unsure as to how to proceed with changes 
to the Triple.h/.cpp stuff as yet.


Patch file attached.

thanks,
Matthew Gardiner


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: include/lldb/Core/ArchSpec.h
===
--- include/lldb/Core/ArchSpec.h	(revision 212092)
+++ include/lldb/Core/ArchSpec.h	(working copy)
@@ -101,6 +101,9 @@
 
 eCore_uknownMach32,
 eCore_uknownMach64,
+
+eCore_kalimba,
+
 kNumCores,
 
 kCore_invalid,
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp	(revision 212092)
+++ source/Core/ArchSpec.cpp	(working copy)
@@ -113,7 +113,9 @@
 { eByteOrderLittle, 4, 4, 4, llvm::Triple::hexagon , ArchSpec::eCore_hexagon_hexagonv5,  hexagonv5 },
 
 { eByteOrderLittle, 4, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach32  , unknown-mach-32 },
-{ eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach64  , unknown-mach-64 }
+{ eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach64  , unknown-mach-64 },
+
+{ eByteOrderLittle, 4, 1, 1 , llvm::Triple::UnknownArch , ArchSpec::eCore_kalimba  , kalimba }
 };
 
 struct ArchDefinitionEntry
@@ -249,7 +251,9 @@
 { ArchSpec::eCore_sparc9_generic  , llvm::ELF::EM_SPARCV9, LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // SPARC V9
 { ArchSpec::eCore_x86_64_x86_64   , llvm::ELF::EM_X86_64 , LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // AMD64
 { ArchSpec::eCore_mips64  , llvm::ELF::EM_MIPS   , LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // MIPS
-{ ArchSpec::eCore_hexagon_generic , llvm::ELF::EM_HEXAGON, LLDB_INVALID_CPUTYPE, 0xu, 0xu }  // HEXAGON
+{ ArchSpec::eCore_hexagon_generic , llvm::ELF::EM_HEXAGON, LLDB_INVALID_CPUTYPE, 0xu, 0xu }, // HEXAGON
+{ ArchSpec::eCore_kalimba , llvm::ELF::EM_CSR_KALIMBA, LLDB_INVALID_CPUTYPE

Re: [Lldb-commits] [PATCH] Addition of CSR Kalimba DSP definitions to ArchSpec

[Matthew Gardiner]

Todd Fiala wrote:

Just went in:
Sendinginclude/lldb/Core/ArchSpec.h
Sendingsource/Core/ArchSpec.cpp
Transmitting file data ..
Committed revision 212145.


On Tue, Jul 1, 2014 at 4:34 PM, Greg Clayton gclay...@apple.com 
mailto:gclay...@apple.com wrote:


Looks good.

 On Jul 1, 2014, at 2:45 PM, Todd Fiala todd.fi...@gmail.com
mailto:todd.fi...@gmail.com wrote:

 LGTM from my end.  If Greg is okay with it, I'll check it in.



Thanks for submitting this guys.

In order that I can make changes to the llvm::Triple stuff, to add 
Kalimba as an Architecture, and CSR as a vendor would I be right in 
thinking that I need to make a request to the llvm-dev group?


thanks again
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] Corrections for docs/lldb-gdb-remote.txt

[Matthew Gardiner]

Todd Fiala wrote:

Hey Matthew,

That 0 parameter to strtoul doesn't quite work like that.  The 0 
indicates that the input character string determines how the base is 
interpreted.  So a 0x{hex} will get interpreted base 16, a standard 
non-zero-leading set of decimal numbers is base 10, and a 0{octal} is 
octal.



Sorry, my bad. I think I jumped the gun here! However, the documentation 
in lldb-gdb-remote.txt does state that for qHostInfo:


cputype: is a number that is the mach-o CPU type that is being debugged 
(base 10)


but for qProcessInfo:
cputype: the Mach-O CPU type of the process  (base 16)

So I wondered whether bases of either message were intended to be 
different. That's why I think something is a bit wrong in the docs. Then 
I looked at the parsing code, and unfortunately misinterpreted the 0 
specification :-(


However, what do you (and others!) think of the discrepancy between the 
qHostInfo and qProcessInfo docs?


Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] Corrections for docs/lldb-gdb-remote.txt

[Matthew Gardiner]

Hi,

I've made a couple of corrections to the documentation of the qHostInfo 
packet.


1. I've changed the base of cputype and cpusubtype to reflect the fact 
that the code assumes base-16, and furthermore to be consistent with 
qProcessInfo.


2. I've also added a description for arch which is parsed from the 
response, but was missing from the document.


Could someone please apply my patch?

thanks
Matt

(Patch attached)

Index: docs/lldb-gdb-remote.txt
===
--- docs/lldb-gdb-remote.txt(revision 210650)
+++ docs/lldb-gdb-remote.txt(working copy)
@@ -523,10 +523,11 @@

 Key value pairs are one of:

-cputype: is a number that is the mach-o CPU type that is being debugged 
(base 10)
-cpusubtype: is a number that is the mach-o CPU subtype type that is 
being debugged (base 10)
+cputype: is a number that is the mach-o CPU type that is being debugged 
(base 16)
+cpusubtype: is a number that is the mach-o CPU subtype type that is 
being debugged (base 16)
 triple: a string for the target triple (x86_64-apple-macosx) that can 
be used to specify arch + vendor + os in one entry
 vendor: a string for the vendor (apple), not needed if triple is 
specified
+arch: a string for the architecture being debugged (i386, x86_64, 
unknown), not needed if triple is specified
 ostype: a string for the OS being debugged (darwin, linux, freebsd), 
not needed if triple is specified

 endian: is one of little, big, or pdp
 ptrsize: an unsigned number that represents how big pointers are in 
bytes on the debug target




Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: docs/lldb-gdb-remote.txt
===
--- docs/lldb-gdb-remote.txt	(revision 210650)
+++ docs/lldb-gdb-remote.txt	(working copy)
@@ -523,10 +523,11 @@
 
 Key value pairs are one of:
 
-cputype: is a number that is the mach-o CPU type that is being debugged (base 10)
-cpusubtype: is a number that is the mach-o CPU subtype type that is being debugged (base 10)
+cputype: is a number that is the mach-o CPU type that is being debugged (base 16)
+cpusubtype: is a number that is the mach-o CPU subtype type that is being debugged (base 16)
 triple: a string for the target triple (x86_64-apple-macosx) that can be used to specify arch + vendor + os in one entry
 vendor: a string for the vendor (apple), not needed if triple is specified
+arch: a string for the architecture being debugged (i386, x86_64, unknown), not needed if triple is specified
 ostype: a string for the OS being debugged (darwin, linux, freebsd), not needed if triple is specified
 endian: is one of little, big, or pdp
 ptrsize: an unsigned number that represents how big pointers are in bytes on the debug target
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] Corrections for docs/lldb-gdb-remote.txt

[Matthew Gardiner]

Todd Fiala wrote:

Hey Matthew,

 1. I've changed the base of cputype and cpusubtype to reflect the 
fact that the code assumes base-16, and furthermore to be consistent 
with qProcessInfo.


I'm looking at the debugserver code (RNBRemote.cpp) and 
lldb-platform/llgs code (GDBRemoteCommunicationServer.cpp) that 
produce those values.  The cputype and cpusubtype all are being 
written decimal as far as a I read it.  What context are you seeing 
hexadecimal?


Thanks!

-Todd


Hi Todd,

I'm seeing the context as being assumed as hex in the parsing of the 
*inbound* qHostInfo in lldb.


Check out: GDBRemoteCommunicationClient::GetHostInfo in 
GDBRemoteCommunicationClient.cpp


see this code:

if (name.compare(cputype) == 0)
{
   // exception type in big endian hex
   cpu = Args::StringToUInt32 (value.c_str(), LLDB_INVALID_CPUTYPE, 0);
  ...
}
else if (name.compare(cpusubtype) == 0)
{
 // exception count in big endian hex
  sub = Args::StringToUInt32 (value.c_str(), 0, 0);
..
}

the third argument to StringToUInt32  is 0, which is passed to strtoul, 
which is interpreted as base-16. Thus the parsing side in lldb assumes 
these are base-16.


A side issue is that the documentation to qProcessInfo in 
lldb-gdb-remote.txt which says base-16 for these same datas.


So we have inconsistency in the docs between qProcessInfo and qHostInfo, 
and a bug between the sender and receiver of qHostInfo.


thanks
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r208423 - Documented our attach extension packets.

[Matthew Gardiner]

Todd Fiala wrote:
It's not clear in the gdb protocol docs, but the stub does launch a 
new inferior process with the $A command.  (From the gdb remote docs, 
it looks to me like it's just supposed to set the args, but it really 
is supposed to launch it too).  That's how lldb launches an exe when 
talking to a debugserver (and lldb-gdbserver).



Thanks for this Todd, I guess I need to play with this $A stuff sometime 
later to understand it.

Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r208423 - Documented our attach extension packets.

[Matthew Gardiner]

Thanks for providing the documentation, Jim.

So in the case of vAttachOrWait, if the process is not already there, 
what entity is responsible for the launching of it? Would that be the 
stub itself, or is this where lldb-platform steps in?


thanks
Matt


Jim Ingham wrote:

Author: jingham
Date: Fri May  9 11:17:24 2014
New Revision: 208423

URL: http://llvm.org/viewvc/llvm-project?rev=208423view=rev
Log:
Documented our attach extension packets.

Modified:
 lldb/trunk/docs/lldb-gdb-remote.txt

Modified: lldb/trunk/docs/lldb-gdb-remote.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/lldb-gdb-remote.txt?rev=208423r1=208422r2=208423view=diff
==
--- lldb/trunk/docs/lldb-gdb-remote.txt (original)
+++ lldb/trunk/docs/lldb-gdb-remote.txt Fri May  9 11:17:24 2014
@@ -1195,3 +1195,58 @@ for this region.
  // send packet: $qProcessInfoPID:60050#00
  // read packet: 
$pid:60050;ppid:59948;uid:7746;gid:11;euid:7746;egid:11;name:6c6c6462;triple:7838365f36342d6170706c652d6d61636f7378;#00
  //--
+
+//--
+// vAttachName
+//
+// BRIEF
+//  Same as vAttach, except instead of a pid you send a process name.
+//
+// PRIORITY TO IMPLEMENT
+//  Low. Only needed for process attach -n.  If the packet isn't supported
+//  then process attach -n will fail gracefully.  So you need only to support
+//  it if attaching to a process by name makes sense for your environment.
+//--
+
+//--
+// vAttachWait
+//
+// BRIEF
+//  Same as vAttachName, except that the stub should wait for the next instance
+//  of a process by that name to be launched and attach to that.
+//
+// PRIORITY TO IMPLEMENT
+//  Low. Only needed to support process attach -w -n which will fail
+//  gracefully if the packet is not supported.
+//--
+
+//--
+// qAttachOrWaitSupported
+//
+// BRIEF
+//  This is a binary is it supported query.  Return OK if you support
+//  vAttachOrWait
+//
+// PRIORITY TO IMPLEMENT
+//  Low. This is required if you support vAttachOrWait, otherwise no support
+//  is needed since the standard I don't recognize this packet response
+//  will do the right thing.
+//--
+
+//--
+// vAttachOrWait
+//
+// BRIEF
+//  Same as vAttachWait, except that the stub will attach to a process
+//  by name if it exists, and if it does not, it will wait for a process
+//  of that name to appear and attach to it.
+//
+// PRIORITY TO IMPLEMENT
+//  Low. Only needed to implement process attach -w -i false -n.  If
+//  you don't implement it but do implement -n AND lldb can somehow get
+//  a process list from your device, it will fall back on scanning the
+//  process list, and sending vAttach or vAttachWait depending on
+//  whether the requested process exists already.  This is racy,
+//  however, so if you want to support this behavior it is better to
+//  support this packet.
+//--


___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits


  To report this email as spam click 
https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .




Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] Let lldb dump line tables for DWARF v3

[Matthew Gardiner]

Hi people,

I have been looking into the support for ARM binary images by lldb. One 
of the ARM tool chains I use produces .debug_line tables as specified by 
DWARF v3. With lldb and llvm-dwarfdump built from the top of the tree, I 
observed that the dwarfdump tool dumps the line table, but lldb does 
not. After some investigation I discovered that the dump tool is built 
against llvm/lib/DebugInfo/DWARFDebugLine.cpp, whereas lldb uses 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp.


It turned out that the source used by lldb has a subtle difference in 
flow-control with regards the treatment of the line table's prologue 
version number compared to that of the llvm source. After duplication of 
this flow-control, it is possible for lldb to print the line-table built 
according to DWARF standards above and beyond v2.


Could somebody submit the following patch to the lldb tree which fixes 
this issue?


Index: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp  (revision 205611)
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp  (working copy)
@@ -418,7 +418,7 @@
 const char * s;
 prologue-total_length  = 
debug_line_data.GetDWARFInitialLength(offset_ptr);

 prologue-version   = debug_line_data.GetU16(offset_ptr);
-if (prologue-version != 2)
+if (prologue-version  2)
   return false;

 prologue-prologue_length   = 
debug_line_data.GetDWARFOffset(offset_ptr);

@@ -486,7 +486,7 @@
 (void)debug_line_data.GetDWARFInitialLength(offset);
 const char * s;
 uint32_t version = debug_line_data.GetU16(offset);
-if (version != 2)
+if (version  2)
   return false;

 const dw_offset_t end_prologue_offset = 
debug_line_data.GetDWARFOffset(offset) + offset;



thank you
Matthew Gardiner


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp	(revision 205611)
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp	(working copy)
@@ -418,7 +418,7 @@
 const char * s;
 prologue-total_length  = debug_line_data.GetDWARFInitialLength(offset_ptr);
 prologue-version   = debug_line_data.GetU16(offset_ptr);
-if (prologue-version != 2)
+if (prologue-version  2)
   return false;
 
 prologue-prologue_length   = debug_line_data.GetDWARFOffset(offset_ptr);
@@ -486,7 +486,7 @@
 (void)debug_line_data.GetDWARFInitialLength(offset);
 const char * s;
 uint32_t version = debug_line_data.GetU16(offset);
-if (version != 2)
+if (version  2)
   return false;
 
 const dw_offset_t end_prologue_offset = debug_line_data.GetDWARFOffset(offset) + offset;
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] 32-bit linux Remove WriteRegOperation's explicit GetAsUInt32() call

[Matthew Gardiner]

Hi folks

Even with the register map fixed (see UserArea in RegisterContextLinux_i386.cpp)
an assertion failure occurs:

$ lldb hello
Current executable set to 'hello' (i386).
(lldb) log enable linux ptrace
(lldb) run
operation ptrace(PTRACE_SETOPTIONS, 2667, (nil), 0x58, 0)=0 called from file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 1456
Process 2667 launching
operation ptrace(PTRACE_TRACEME, 0, (nil), (nil), 0)=0 called from file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 1196
operation ptrace(PTRACE_PEEKDATA, 2667, 0x8048340, (nil), 0)=895EED31 called 
from file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 245
operation ptrace(PTRACE_PEEKDATA, 2667, 0x8048340, (nil), 0)=895EED31 called 
from file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 245
operation ptrace(PTRACE_POKEDATA, 2667, 0x8048340, 0x895eedcc, 0)=0 called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 319
operation ptrace(PTRACE_PEEKDATA, 2667, 0x8048340, (nil), 0)=895EEDCC called 
from file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 245
operation ptrace(PTRACE_POKEUSER, 2667, 0x114, 0x, 0)=0 called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 543
operation ptrace(PTRACE_POKEUSER, 2667, 0x118, 0x, 0)=0 called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 543
operation ptrace(PTRACE_PEEKUSER, 2667, 0x114, (nil), 0)= called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 494
operation ptrace(PTRACE_PEEKUSER, 2667, 0x114, (nil), 0)= called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 494
operation ptrace(PTRACE_POKEUSER, 2667, 0x114, 0x, 0)=0 called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 543
operation ptrace(PTRACE_PEEKUSER, 2667, 0x118, (nil), 0)= called from 
file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 494
operation ptrace(PTRACE_PEEKUSER, 2667, 0xfc, (nil), 0)=0 called from file 
/home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
 line 494
lldb: /home/mg11/src/heracles/llvm/tools/lldb/source/Plugins/Process/POSIX/POSIXThread.cpp:530: void POSIXThread::WatchNotify(const ProcessMessage): Assertion `wp_sp.get()  No watchpoint found' 
failed.

Aborted (core dumped)

The root cause of this is that 0x is written to dr6/7 originally from
RegisterContextPOSIXProcessMonitor_x86.cpp, whereas the programmer intended
0 to be written:

RegisterContextPOSIXProcessMonitor_x86_64::IsWatchpointHit(uint32_t hw_index)
{
snip
RegisterValue zero_bits = RegisterValue(uint64_t(0));
if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || 
!WriteRegister(m_reg_info.first_dr + 7, zero_bits))

Construction of the RegisterValue as uint64_t and the subsequent conditional
compilation of 32-bit code within ProcessMonitor.cpp

void
WriteRegOperation::Execute(ProcessMonitor *monitor)
{
snip
#if __WORDSIZE == 32
buf = (void*) m_value.GetAsUInt32();
#else

combined with RegisterValue's implementation returning fail_value for
wrapped 64-bit data accessed as 32-bits.

Removal of the preprocessing step, relies on the compiler forcing truncation
to 32-bit, when compiled on 32-bit platform, more faithfully than the
explicit GetAsUInt32().

Please could someone apply the attached patch which fixes the fail_value
return. I tested this on 32-bit by launching a program, stopping, setting
a break, then resuming it. It was fine. I can't test 64-bit yet, but clearly
my proposed patch results in the 64-bit code being unchanged.

Index: source/Plugins/Process/Linux/ProcessMonitor.cpp
===
--- source/Plugins/Process/Linux/ProcessMonitor.cpp (revision 202675)
+++ source/Plugins/Process/Linux/ProcessMonitor.cpp (working copy)
@@ -532,11 +532,7 @@
 void* buf;
 Log *log (ProcessPOSIXLog::GetLogIfAllCategoriesSet (POSIX_LOG_REGISTERS));

-#if __WORDSIZE == 32
-buf = (void*) m_value.GetAsUInt32();
-#else
 buf = (void*) m_value.GetAsUInt64();
-#endif

 if (log)
 log-Printf (ProcessMonitor::%s() reg %s: %p, __FUNCTION__, 
m_reg_name, buf);

thanks
Matt

PS

(If this patch is applied *and* the debug register offset problem fixed -
I'm uploading separate patch - then 32-bit linux debug experience is
sane).





Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, 

Re: [Lldb-commits] [PATCH] Fixes for 32-bit linux ptrace and RegisterContext issues

[Matthew Gardiner]

Todd Fiala wrote:


 +case eTypeUInt64:   return m_data.uint32;
On the surface this line looks a little suspect but I need to see it in context.

Agreed. It's not a really good fix. And it probably breaks a design goal in the RegisterValue. My motivation for making the change was merely that the initialization zero_bits writes to dr6/7 of 
IsWatchpointHit/Vacant were assigned as 64 bits, as in:


RegisterValue zero_bits = RegisterValue(uint64_t(0));

but in the business end (ProcessMonitor)

#if __WORDSIZE == 32
buf = (void*) m_value.GetAsUInt32();
#else
buf = (void*) m_value.GetAsUInt64();
#endif

these returned as  (fail_value) due (what I perceive to be) the design 
goal of RegisterValue (i.e. that switch/case statement).

Better fix is possibly somewhere in RegisterContextPOSIXProcessMonitor?

Anyway, I'll look at the r202428 and see if the issue persists.

thanks
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] Use 32-bit RegisterValue object on IsWatchpoint functions for 32-bit linux

[Matthew Gardiner]

Folks,

An issue currently exists for 32-bit linux such that when lldb receives the
first SIGTRAP, an assertion failure occurs in POSIXThread::WatchNotify. This
failure is due to 0x118 being read back from dr6, but with no watchpoints set.

In trying to trace this I have discovered that IsWatchpointHit/Vacant both
create a RegiaterValue object thusly:

RegisterValue zero_bits = RegisterValue(uint64_t(0));

and then issue a write request. When this write is handled
(Linux/ProcessMonitor.cpp) we coerce this value into a 32-bit int as follows:

#if __WORDSIZE == 32
buf = (void*) m_value.GetAsUInt32();

This is problematic since RegisterValue (deliberately?) returns fail_value
for this call if m_type==eTypeUInt64.

Without really trying to analyse the design of RegisterValue, I think an
expedient fix is to conditionally compile 32-bit lldb to use a 32-bit type
for it's RegisterValue zero_bits. Yes, the Watchpoint assert failure still
happens, however, at least now we are actually writing what we expect.

Would somebody please apply the following patch which implements the
proposed fix?

Index: source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp
===
--- source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp 
(revision 201779)
+++ source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp 
(working copy)
@@ -506,7 +506,12 @@
 if (m_watchpoints_initialized == false)
 {
 // Reset the debug status and debug control registers
+#ifdef __x86_64__
 RegisterValue zero_bits = RegisterValue(uint64_t(0));
+#else
+RegisterValue zero_bits = RegisterValue(uint32_t(0));
+#endif
+
 if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || 
!WriteRegister(m_reg_info.first_dr + 7, zero_bits))
 assert(false  Could not initialize watchpoint registers);
 m_watchpoints_initialized = true;
@@ -562,7 +567,11 @@
 if (m_watchpoints_initialized == false)
 {
 // Reset the debug status and debug control registers
+#ifdef __x86_64__
 RegisterValue zero_bits = RegisterValue(uint64_t(0));
+#else
+RegisterValue zero_bits = RegisterValue(uint32_t(0));
+#endif
 if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || 
!WriteRegister(m_reg_info.first_dr + 7, zero_bits))
 assert(false  Could not initialize watchpoint registers);
 m_watchpoints_initialized = true;

(Possibly a better fix, would be determining at run-time the target application
architecture, therefore permitting 64-bit lldb to debug 32-bit apps.)

thanks
Matt



Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our 
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, 
www.youtube.com/user/CSRplc, Facebook, 
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at 
www.aptx.com.
Index: source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp
===
--- source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp	(revision 201779)
+++ source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp	(working copy)
@@ -506,7 +506,12 @@
 if (m_watchpoints_initialized == false)
 {
 // Reset the debug status and debug control registers
+#ifdef __x86_64__
 RegisterValue zero_bits = RegisterValue(uint64_t(0));
+#else
+RegisterValue zero_bits = RegisterValue(uint32_t(0));
+#endif
+
 if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || !WriteRegister(m_reg_info.first_dr + 7, zero_bits))
 assert(false  Could not initialize watchpoint registers);
 m_watchpoints_initialized = true;
@@ -562,7 +567,11 @@
 if (m_watchpoints_initialized == false)
 {
 // Reset the debug status and debug control registers
+#ifdef __x86_64__
 RegisterValue zero_bits = RegisterValue(uint64_t(0));
+#else
+RegisterValue zero_bits = RegisterValue(uint32_t(0));
+#endif
 if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || !WriteRegister(m_reg_info.first_dr + 7, zero_bits))
 assert(false  Could not initialize watchpoint registers);
 m_watchpoints_initialized = true;
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits