Re: [Lldb-commits] [PATCH] Implemented thread jump built-in command.

2013-08-29 Thread jingham
This looks great. One quibble and a suggestion. The quibble: since the meaning of force is very specific, namely don't leave the current function it would be clearer to use that wording for the command line option. I know I suggested the force but since you came up with a better name, we

Re: [Lldb-commits] [lldb] r189934 - Remove windows.h from lldb-types.h.

2013-09-04 Thread jingham
Yes, I agree with this part. It should not be necessary to have the LLVM headers around to build programs that only use the lldb SB API's. Jim On Sep 4, 2013, at 4:36 PM, Malea, Daniel daniel.ma...@intel.com wrote: This commit introduces a dependency in the LLDB API on the LLVM headers,

Re: [Lldb-commits] [lldb] r190149 - Correct logic error found by inspection.

2013-09-06 Thread jingham
I must be dense today, but I can't see any logical difference between the version in your patch and the one that was there before. Jim On Sep 6, 2013, at 2:03 PM, Malea, Daniel daniel.ma...@intel.com wrote: Hi Ed/Jim, After this fix, I'm noticing some buildbot failures in

Re: [Lldb-commits] [lldb] r190149 - Correct logic error found by inspection.

2013-09-09 Thread jingham
Daniel, We try never to call GetStackFrameCount in the ShouldStop methods of any thread plans. This can slow down stepping noticeably when you are far down on the stack. If the problem is that there is confusion when you are stepping through nested inlines, then it should be possible to look

Re: [Lldb-commits] [lldb] r193830 - Thread::SetState() is not being called upon hitting the breakpoint

2013-10-31 Thread jingham
This test should only be expected fail'ed on Darwin. It works properly on Linux. Jim On Oct 31, 2013, at 4:36 PM, Enrico Granata egran...@apple.com wrote: Author: enrico Date: Thu Oct 31 18:36:47 2013 New Revision: 193830 URL: http://llvm.org/viewvc/llvm-project?rev=193830view=rev Log:

Re: [Lldb-commits] [PATCH] Patch for fixing the handling of hardware breakpoints

2014-02-19 Thread jingham
This change bothers me on a couple of counts. First of all, hardware breakpoints are generally a pretty limited resource. So any mechanism that will use them up without the user specifically asking for them doesn't seem like a great idea. For instance, setting a hardware breakpoints will

Re: [Lldb-commits] [PATCH] Patch for fixing the handling of hardware breakpoints

2014-02-21 Thread jingham
Greg should probably take a look as well, but this seems fine to me. There are a couple of typos in the comments, e.g. // check if the error was something other then an unsupported breakpoint type the then should be than. // we will reach here when the stub gives an

Re: [Lldb-commits] [lldb] r202561 - Make sure the exe_ctx passed to ClangUserExpression::Execute has a valid thread.

2014-03-03 Thread jingham
that's a clearer fix than adding m_can_interpret to the if conditional. Thanks for catching this. Jim On Mar 3, 2014, at 7:55 AM, Ed Maste ema...@freebsd.org wrote: On 28 February 2014 19:17, Jim Ingham jing...@apple.com wrote: Author: jingham Date: Fri Feb 28 18:17:06 2014 New Revision

Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.

2014-03-12 Thread jingham
Sorry, got busy. This looks fine. Jim On Mar 12, 2014, at 2:29 AM, Andrew MacPherson andrew.m...@gmail.com wrote: Hi Jim, just pinging this again to see if you've had a chance to take a quick look at the ModulesDidLoad patch, let me know if you'd prefer I submit this as a general patch

Re: [Lldb-commits] [PATCH] Revert r205480 and rename enums defined in llvm/Support/MachO.h

2014-04-08 Thread jingham
That looks okay to me, please commit. Jim On Apr 8, 2014, at 3:20 PM, Akira Hatanaka ahata...@gmail.com wrote: This patch reverts the workaround introduced in r205480 that was required to avoid collision between enums defined in llvm/Support/MachO.h and macros defined in system header

Re: [Lldb-commits] [lldb] r206618 - Address hung tests in Linux.

2014-04-18 Thread jingham
This is okay as a workaround, but I am not sure why Linux threads are being created in the state eStateSuspended. That state is for what the user directs the thread to do (SBThread::Suspend() and SBThread::Resume() for instance.) It seems weird that this should be starting out as suspended.

Re: [Lldb-commits] [lldb] r211868 - lldb: remove adhoc implementation of array_sizeof

2014-06-27 Thread jingham
Hey, Saleem. I reverted the debugserver part of this change. Debugserver doesn't currently depend on llvm (actually it really doesn't depend on lldb either) and I'd rather not add that dependence unless there's some strong reason. Jim On Jun 26, 2014, at 10:17 PM, Saleem Abdulrasool

Re: [Lldb-commits] [PATCH] Reducing lldb Carbon footprint by replacing the only Carbon API call by Cocoa equivalent

2014-07-02 Thread jingham
Jean-Daniel, Did you actually get this to work with a recent Xcode? The last time I tried this, Xcode had broken the AS support for set the selection which rendered this whole bit of code moot. I just hadn't gotten around to deleting it yet. If you have it working, that is excellent! But

Re: [Lldb-commits] [PATCH] Adds the ability for Command options to be validated at runtime.

2014-07-03 Thread jingham
I wonder a little about the the OptionGroups here. Those options don't know a-priori what command they are attached to. It seems not unreasonable to say that if an element of an option group is invalid for one platform for command A, it will also be invalid for all other commands? But we

Re: [Lldb-commits] [PATCH] Adds the ability for Command options to be validated at runtime.

2014-07-03 Thread jingham
But anyway, otherwise I agree with Greg, this looks fine. Jim On Jul 3, 2014, at 10:45 AM, jing...@apple.com wrote: I wonder a little about the the OptionGroups here. Those options don't know a-priori what command they are attached to. It seems not unreasonable to say that if an

Re: [Lldb-commits] [PATCH] Host::GetOSVersion simplification

2014-07-03 Thread jingham
We originally decided to avoid Foundation if we could, though I don't remember why. If we don't object to using Foundation on the Mac then this is fine. Anybody else have an opinion about that? Jim On Jul 3, 2014, at 1:23 PM, Jean-Daniel Dupas devli...@shadowlab.org wrote: Hello, I

Re: [Lldb-commits] [PATCH] Invalidate process uid/gid command options on Windows

2014-07-07 Thread jingham
If there's no target, then we should consult the current platform - if nobody set a platform then that will default to the host platform. Usually you get that from the target, so it would seem odd to pass that in separately, but maybe in this case that's okay to do. Otherwise whoever would be

Re: [Lldb-commits] [lldb] r213294 - In Process::LoadImage, if dlopen returns 0x0 fetch the error with dlerror and report

2014-07-17 Thread jingham
not familiar with this codepath so I'm not sure how to exercise it and test. If I don't hear back in an hour or so I may see about reverting it, since I'm not sure if the attached fix is correct. On Thu, Jul 17, 2014 at 11:55 AM, Jim Ingham jing...@apple.com wrote: Author: jingham Date

Re: [Lldb-commits] [PATCH] Fix portability problems in lots of tests

2014-07-17 Thread jingham
Excellent!!! Jim On Jul 17, 2014, at 3:35 PM, Zachary Turner ztur...@google.com wrote: Adds the builder_win32 module. Test suite runs! http://reviews.llvm.org/D4573 Files: test/benchmarks/disassembly/TestDisassembly.py test/benchmarks/disassembly/TestDoAttachThenDisassembly.py

Re: [Lldb-commits] [PATCH] Add a .clang-format file to enhance formatting experience with clang-format

2014-07-22 Thread jingham
We don't have such a clang-format file. If somebody can come up with one that will run over the extant source base and produce no changes, then I guess I'd entertain using it for new code. I'm not really sure I see the point. The lldb coding conventions as far as braces and the like are

Re: [Lldb-commits] [PATCH] Add a .clang-format file to enhance formatting experience with clang-format

2014-07-22 Thread jingham
Well, tools are produced for use as Hildy Johnson notes... If folks want to use this as an after the fact emacs mode on code they are adding, I don't mind. But touching a file doesn't mean you should also fix up its formatting with such a tool. As long as THAT is part of our coding

Re: [Lldb-commits] [PATCH] Add a new command frame dump to dump the current stack frame displaying sp and fp registers.

2014-07-23 Thread jingham
This seems like a useful command, but I don't think the implementation is right. Obviously you need to fix the indenting, having the address after the sp and fp not line up with the other memory entries makes this hard to read. But that's pretty trivial. More importantly, it looks like you

Re: [Lldb-commits] [PATCH] For expression evaluation, a new ThreadPlanCallFunctionGDB for executing a function call on target via register manipulation.

2014-07-25 Thread jingham
I haven't looked over this patch in detail yet, but it would be much easier to read what you are doing if you would make your ThreadPlanCallFunctionGDB class derive from ThreadPlanCallFunction and only implement the bits that are actually different. You will have to do this anyway before you

Re: [Lldb-commits] [lldb] r214319 -

2014-07-30 Thread jingham
This change reworks how the -o, -O, -s and -S options to the lldb driver are handled so that they are interruptible. Jim On Jul 30, 2014, at 11:04 AM, Reid Kleckner r...@google.com wrote: On Wed, Jul 30, 2014 at 10:38 AM, Greg Clayton gclay...@apple.com wrote: Author: gclayton Date: Wed

Re: [Lldb-commits] [PATCH] [2/2] Fix Path support on Windows

2014-07-30 Thread jingham
On Jul 30, 2014, at 6:50 PM, Greg Clayton gclay...@apple.com wrote: Why does the user ever want to specify the PathSyntax? Shouldn't it just be something that can be queried and be set automagically? What if I did: FileSpec f(C:\Users\me\foo.txt, false, ePathSyntaxPosix); If I set

Re: [Lldb-commits] [lldb] r215159 - r215124 missed a few Mac OS X only uses of FileSpec::Resolve, fixing to match

2014-08-07 Thread jingham
this kind of thing won't happen anymore. On Thu, Aug 7, 2014 at 3:37 PM, Jim Ingham jing...@apple.com wrote: Author: jingham Date: Thu Aug 7 17:37:43 2014 New Revision: 215159 URL: http://llvm.org/viewvc/llvm-project?rev=215159view=rev Log: r215124 missed a few Mac OS X only uses of FileSpec

Re: [Lldb-commits] [PATCH] Check if certain stack frame exists before dereferencing it

2014-08-11 Thread jingham
Ah, my bad, thanks. Can you check this in? Otherwise I will... Jim On Aug 11, 2014, at 4:04 PM, Tong Shen endlessr...@google.com wrote: Hi, The patch fixes segfault for i386 on x86_64 when we are stepping in and only frame 0 is available. -- Best Regards, Tong Shen

Re: [Lldb-commits] [PATCH] Add process launch --enable-aslr option, tweak handling of flag

2014-08-18 Thread jingham
I don't think we have any other instances where we use two flags to express do x and don't do x. For the long options is isn't such a big deal but we try not to use up more short options than necessary, since this gets to be a crowded space. Maybe process launch --enable-aslr true/false,

Re: [Lldb-commits] [PATCH] Add process launch --enable-aslr option, tweak handling of flag

2014-08-18 Thread jingham
On Aug 18, 2014, at 12:45 PM, Todd Fiala tfi...@google.com wrote: On Mon, Aug 18, 2014 at 11:22 AM, jing...@apple.com wrote: I don't think we have any other instances where we use two flags to express do x and don't do x. For the long options is isn't such a big deal but we try not to

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-28 Thread jingham
Suspending threads in the inferior is very useful. There are some parts of the Host interface that will be used by the the Native* for that host, e.g. launching. So there might be things in the Host interface that you wouldn't naively think were necessary. IIUC however, HostThread operations

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-28 Thread jingham
On Aug 28, 2014, at 5:20 PM, Zachary Turner ztur...@google.com wrote: I toiled over this question for a while, but ultimately decided against a base class with virtual methods because there will only ever be 1 type of HostThread per host platform. For example, on MacOSX, the HostThread

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-28 Thread jingham
I guess I view this the other way round from you. The host layer has two sides to it. To me the primary one is to provide an interface that isolates generic lldb code from the underlying details of the host system that it is compiled for. Secondarily it will provide interfaces that the

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
So maybe I'm missing something here. It seems to me that there are the specific details, say for instance of how the user/group for a process are specified. The details of how this is represented might be wildly different, but for the most part generic lldb doesn't care about that. However,

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
On Aug 29, 2014, at 9:29 AM, Zachary Turner ztur...@google.com wrote: I actually think we are agreeing on many of the high-level issues. However, before I write anymore, I want to mention again that there would never be code that looks like this: #if HOST_THREAD == HostThreadMacOSX

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
On Aug 29, 2014, at 10:09 AM, Zachary Turner ztur...@google.com wrote: On Fri, Aug 29, 2014 at 9:54 AM, jing...@apple.com wrote: SystemLog ThreadDetach ThreadCancel There must be some way on Windows to tell a thread to exit at the next cancellation point? Can you really not

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
One thing I am unclear about, I had thought that HostThread in particular would be used for threads IN lldb, but not for handling threads in a program lldb is debugging when on that Host. So for instance, Suspend Resume don't seem necessary or desirable when dealing with threads lldb is using

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
Yes, I think there's enough reason to expect we'll use HostThread in ways the C++11 standards committee didn't think of that keeping a wrapper is useful. Also, Hosts mostly have to have fairly good thread libraries now-a-days, but they don't necessarily have to have good C++11 std::thread

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
On Aug 29, 2014, at 12:33 PM, Zachary Turner ztur...@google.com wrote: I'll proceed as discussed in the earlier thread where I listed the interfaces, minus the Suspend / Resume. I'll keep an eye on this thread though in case anything else comes up. Thanks for trudging through this with

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-08-29 Thread jingham
This was probably lacking because the only threads really managed by the hosts' thread_t were ones we created, so you'd get your hands on a thread_t by Host::ThreadCreate'ing it, then you could manipulate it with Join, etc. We didn't ever need to use thread_t for threads that already existed

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-09-02 Thread jingham
Yes, Mac OS X actually has 3 different thread identifiers, which are used for different purposes. There's the mach thread port, the pthread id and the universal thread ID. When working with your own threads - which is what the Host layer mostly wants to do, the pthread id is the important

Re: [Lldb-commits] [PATCH] Implements a HostThread class.

2014-09-04 Thread jingham
On Sep 4, 2014, at 5:57 PM, Zachary Turner ztur...@google.com wrote: Regarding point #1: I'm still not sold on this. Exposing only the HostThreadBase really complicates some things. Some of the issues escape my mind right now, but one in particular stands out. There are a couple of

Re: [Lldb-commits] [PATCH] [lldb] Abstract a superclass for a generic thread container

2014-09-05 Thread jingham
Great, thanks. Jim On Sep 5, 2014, at 11:50 AM, Kuba Brecka kuba.bre...@gmail.com wrote: Indenting the initializer list. http://reviews.llvm.org/D5200 Files: include/lldb/Target/ThreadCollection.h include/lldb/Target/ThreadList.h include/lldb/lldb-forward.h

Re: [Lldb-commits] [PATCH] First pass at HostThreads refactor

2014-09-09 Thread jingham
No, this seems great. Jim On Sep 8, 2014, at 5:19 PM, Zachary Turner ztur...@google.com wrote: Hi Jim, I don't expect you to read this patch line by line, but do you have any remaining high level concerns? On Fri, Sep 5, 2014 at 3:28 PM, Zachary Turner ztur...@google.com wrote:

Re: [Lldb-commits] [PATCH] use std::atomic to protect variables being accessed by multiple threads

2014-09-12 Thread jingham
That looks okay. Thanks for looking at this. Seems to me that if you're only synchronization is locking access to some flag, then you're either doing something fairly weak (like the log example) or missing some real synchronization... So it's worth viewing the necessity of the change with

Re: [Lldb-commits] [lldb] r217714 - Make ProcessLaunchInfo copyable.

2014-09-12 Thread jingham
Hey Zachary, at some point I'll finish up the lldb coding conventions document, but till I get around to the full thing, one of the bullet items it will have is not to use this form of listing the initializations: +: ProcessInfo() +, m_working_dir() +, m_plugin_name() +,

Re: [Lldb-commits] [PATCH] find global by regex in SBTarget

2014-09-19 Thread jingham
Oh, by the way, you also have to add this API to the equivalent .i file (./scripts/Python/interface/SBTarget.i) or it won't show up in Python. Jim On Sep 19, 2014, at 9:25 AM, jing...@apple.com wrote: You can't do it this way, because it changes a public API, which we promised not to do.

Re: [Lldb-commits] [PATCH] find global by regex in SBTarget

2014-09-19 Thread jingham
I think the enum you added goes in lldb-enumerations.h not in SBTarget.h. Also, I wouldn't use the regex match for the straight match, since that is much less efficient than the ConstString ==, which is just a pointer compare. In the fullness of time, we would retire the version that doesn't

Re: [Lldb-commits] [PATCH] Very minimal non-8-bit byte support for diverse kalimba architectures

2014-09-26 Thread jingham
Greg is out on vacation for a week. This looks fairly straightforward. There used to be architectures with weird byte sizes like 9 or 12 bits - gdb had some support for this. Choosing to express the number of bits per byte as a multiple of 8 makes it impossible to support this situation.

Re: [Lldb-commits] [PATCH] Make llgs build on Android. No functionality change.

2014-09-26 Thread jingham
Sorry, I must be missing something. Why can't HostInfoAndroid just derive from HostInfoLinux, and only modify the methods it actually changes? Jim On Sep 26, 2014, at 1:16 PM, Tong Shen endlessr...@google.com wrote: Actually, Todd suggested me to add Android specific files for HostInfo,

Re: [Lldb-commits] [lldb] r218650 - Add a very trivial example for scripted stepping.

2014-09-29 Thread jingham
Huh, I had edited this in /tmp, playing around to get it right, and then cut pasted that into emacs in place to finish it up. The python mode seems to have really mangled the text in the process. I'll know not to try that again! Should be better now. Thanks for noticing this. Jim On Sep

Re: [Lldb-commits] [lldb] r218834 - Allow Python commands to optionally take an SBExecutionContext argument in case they need to handle 'where they want to act' separately from the notion of 'currentl

2014-10-01 Thread jingham
Excellent!!! Thanks for adding this, Enrico! Can you also add something to the Python Tutorial describing this and why it's a good idea to do it this way? Jim On Oct 1, 2014, at 2:47 PM, Enrico Granata egran...@apple.com wrote: Author: enrico Date: Wed Oct 1 16:47:29 2014 New Revision:

Re: [Lldb-commits] [PATCH] Add target.jit-args settings

2014-10-06 Thread jingham
It's ugly to have to add quotes and then remove them, but until we write our own option parser I can't think of a better way to do it... Jim On Oct 2, 2014, at 8:33 AM, Todd Fiala tfi...@google.com wrote: I tested this with the lldb docs for 'type summary' here:

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

2014-10-20 Thread jingham
On Oct 20, 2014, at 6:09 AM, Matthew Gardiner m...@csr.com wrote: Hi clayborg, emaste, In order to write applications with the lldb C++ API which support non-8-bit bytes targets, it is necessary to extend the API somewhat. To use lldb's ReadMemory interface, which accepts a buffer

Re: [Lldb-commits] [PATCH] Add an OperatingSystem plugin to support goroutines

2014-10-20 Thread jingham
Greg should comment on way you're populating the Memory threads, etc. That was his code (used mostly by the OS X kernel folks to turn kernel thread activations into lldb threads...) At present you can only enable/disable the OS plugins when the target is created, but I haven't looked into how

Re: [Lldb-commits] [lldb] r222186 - Fix override/virtual warnings.

2014-11-17 Thread jingham
I'm sure I'm missing something obvious, but I don't understand what this warning you are fixing is supposed to be telling you about. The inheritance hierarchy is: PlatformDarwin--PlatformPOSIX--Platform Platform.h defines: virtual Error ResolveExecutable (const FileSpec

Re: [Lldb-commits] [lldb] r222186 - Fix override/virtual warnings.

2014-11-17 Thread jingham
As I understand it, mixing override virtual is a recipe for confusion... We currently use virtual, and not override. At some point we can talk about switching over to using override everywhere, but I don't think we are at that point. So in this case your patch is incorrect, and the virtual

Re: [Lldb-commits] [lldb] r222186 - Fix override/virtual warnings.

2014-11-17 Thread jingham
Override seems kind of gross to me. I can't use it on base class functions. If I just use override I get override.cpp:3:22: error: only virtual member functions can be marked 'override' int DoSomething () override; and if I put both I get: override.cpp:3:15: error: 'DoSomething' marked

Re: [Lldb-commits] [lldb] r222186 - Fix override/virtual warnings.

2014-11-17 Thread jingham
On Nov 17, 2014, at 5:54 PM, Zachary Turner ztur...@google.com wrote: It's true you have to remember whether you're in the base class, but as you deduced, that's pretty much the point. FWIW I switched all of ProcessWindows over to using it just a day ago, because I found some dead code

Re: [Lldb-commits] [PATCH] lldb-mi support for CLI commands and -s option

2014-11-18 Thread jingham
I didn't see a patch attached to this. Jim On Nov 18, 2014, at 8:32 AM, d...@burble.org wrote: Adds support for using command files in lldb-mi via option -s, and allows for non-MI commands to be entered in interpreter mode by embedding them inside -interpreter-exec commands. Please

Re: [Lldb-commits] [PATCH] fix for -Q option

2014-11-18 Thread jingham
Doh! Thanks for that, committed as: Sendingsource/Commands/CommandObjectCommands.cpp Transmitting file data . Committed revision 43. Jim On Nov 18, 2014, at 8:08 AM, d...@burble.org wrote: On Mon, Nov 17, 2014 at 07:47:48PM -0800, d...@burble.org wrote: This patch fixes the

Re: [Lldb-commits] [PATCH] Fix some posixy assumptions that were used during RunShellCommand

2014-12-05 Thread jingham
I did it (r223568.) Jim On Dec 5, 2014, at 5:39 PM, Zachary Turner ztur...@google.com wrote: If its not urgent i can revert in a couple hours, but I don't have access to a machine at the moment. REPOSITORY rL LLVM http://reviews.llvm.org/D6553

Re: [Lldb-commits] [PATCH] Close terminal after LaunchInTerminalTestCase test

2015-02-06 Thread jingham
A long-standing bug in the lldb command help system is that it doesn't print what the default value of boolean flags is. So for instance you get: (lldb) help process launch Launch the executable in the debugger. Syntax: process launch cmd-options [run-args] Command Options Usage:

Re: [Lldb-commits] [PATCH] Close terminal after LaunchInTerminalTestCase test

2015-02-06 Thread jingham
It's actually a bit of work. Right now, all the default values are set in the OptionParsingStarting method of the Options subclass in the CommandObject subclass that implements the command. So the only entity that knows the default value of an option is the state of the associated ivar in

Re: [Lldb-commits] [PATCH] Extend SBPlatform with capability to launch/terminate a process remotely. Integrate this change into test framework in order to spawn processes on a remote target.

2015-02-03 Thread jingham
BTW, I wrote up the rules for the SB API objects here: http://lldb.llvm.org/SB-api-coding-rules.html Feel free to add to this if there's anything that isn't clear. Jim On Feb 3, 2015, at 1:14 PM, Greg Clayton clayb...@gmail.com wrote: Also one thing to note: if you add anything to the

Re: [Lldb-commits] [PATCH] Add -exec-arguments command

2015-01-21 Thread jingham
, 2015, at 5:13 AM, Ilia K ki.s...@gmail.com wrote: In http://reviews.llvm.org/D6965#110597, @jingham wrote: If we want to have a way to directly set the arguments in a Target, I would prefer to have the SBTarget vend the SBLaunchInfo that it will use when it launches (if you don't give

Re: [Lldb-commits] [PATCH] Don't remove backslashes from arguments unless the following char is recognized.

2015-01-16 Thread jingham
If I had a path component that began with a space (dunno if you can do that on Windows) what would I have to do to enter that? d:\\ foo\bar ? I guess that's not too horrible... Jim On Jan 16, 2015, at 11:47 AM, Zachary Turner ztur...@google.com wrote: Hi jingham, clayborg, This fixes

Re: [Lldb-commits] [lldb] r225284 - Added a test case for launching a process in a separate terminal window to ensure we don't regress on this.

2015-01-20 Thread jingham
The cmake build apparently doesn't build this tool. The Xcode project does, and the test runs correctly. The correct fix is to get cmake to build it for Darwin. It would be a little odd to mark it expected fail on Darwin, when it actually does succeed on Darwin depending on how it's built...

Re: [Lldb-commits] [PATCH] Make TestHelloWorld.py to pass remotely on Linux.

2015-02-11 Thread jingham
It is part of the linker stage so each rebuild should produce an unique UUID. Note, the MacOS X linker actually makes the UUID by checksumming the binary - minus some obvious bits the linker knows to contain timestamps - so if you rebuild the same sources with the same compiler compiler

Re: [Lldb-commits] [PATCH] Prevent LLGS from crashing when exiting - make NativeProcessLinux to wait until ThreadStateCoordinator is fully stopped before entering ~NativeProcessLinux.

2015-02-18 Thread jingham
Be careful about this sort of thing, there are all sorts of gnarly corner cases, like a breakpoint command that does: (lldb) break command add Enter your debugger command(s). Type 'DONE' to end. settings set auto-confirm true process kill DONE The process kill gets executed while you are

Re: [Lldb-commits] [lldb] r229681 - Fix line crossing 80 column border.

2015-02-18 Thread jingham
We don't restrict lldb code to 80 characters. Jim On Feb 18, 2015, at 7:25 AM, Hafiz Abid Qadeer hafiz_a...@mentor.com wrote: Author: abidh Date: Wed Feb 18 09:25:25 2015 New Revision: 229681 URL: http://llvm.org/viewvc/llvm-project?rev=229681view=rev Log: Fix line crossing 80 column

Re: [Lldb-commits] [PATCH] Merge lldb-platform and lldb-gdbserver into a single binary.

2015-02-18 Thread jingham
Greg's out this week. I don't think it's necessary to revert, but I'll remind him to take a look when he gets back... Jim On Feb 18, 2015, at 11:30 AM, Vince Harron vhar...@google.com wrote: Let's wait for Greg to chime in. On Wed, Feb 18, 2015 at 10:56 AM, Robert Flack

Re: [Lldb-commits] [PATCH] Fix IRInterpreter guard variable replacement.

2015-01-09 Thread jingham
I don't see a problem with that. If we start seeing other issues we can always back it out. Jim On Jan 9, 2015, at 11:10 AM, Zachary Turner ztur...@google.com wrote: FWIW I ran the test suite on Linux with this patch applied and nothing regressed. Since Windows is the only platform

Re: [Lldb-commits] [PATCH] Fix IRInterpreter guard variable replacement.

2015-01-08 Thread jingham
Sean is on paternity leave, so he'll probably be slow to answer this sort of ping for a while. Value::replaceAllUsesWith does things very differently from this little function Sean wrote. From what I can tell, it looks like it is the right thing to use, but that function has been in llvm for

Re: [Lldb-commits] [PATCH] Add -p and -r options to lldb-mi command -file-exec-file-and-symbols to support iOS debugging on macOS

2015-03-11 Thread jingham
I'm not responsible for the MI code or its uses, so this is only a slight opinion: It seems to me if a client has to call interpreter-exec with lldb-specific commands, you are already adding lldb specific options to the MI interface, just in a messy way. It would be cleaner to have some way

Re: [Lldb-commits] [PATCH] Add =shlibs-added/=shlibs-removed notifications (MI)

2015-03-11 Thread jingham
The Apple gdb implementation of the MI and the FSF version are different in quite a few respects. The MI was added by Cygnus long ago for an project that died stillborn, and so its original form was never used for a real IDE. After that it languished for a while till we started using it for

Re: [Lldb-commits] [PATCH] Fix ProcessIO test failures

2015-03-11 Thread jingham
AM, Pavel Labath lab...@google.com wrote: Hi clayborg, jingham, There was a race condition regarding the output of the inferior process. The reading of the output is performed on a separate thread, and there was no guarantee that the output will get eventually consumed. Because

Re: [Lldb-commits] [PATCH] Add SBArgs to the public interface

2015-03-11 Thread jingham
My feelings in order of importance are: 1) For sure we should not be adding things to the SB API just for testing. That's going to make for a bad API. 2) We should test everything we can test through the SB API 3) Things that are impossible to test through the SB API need some other way to

Re: [Lldb-commits] [lldb] r231310 - Introduce lldbassert(x)

2015-03-05 Thread jingham
There are other use cases (e.g. for adding a backtrace to log messages) where we want to put the backtrace out in ways lldb is used to outputting information - e.g. lldb::Stream's. So I don't think it is true that all we want to do is get a backtrace to the screen. Jim On Mar 4, 2015, at

Re: [Lldb-commits] [PATCH] Fix a comment for ValueObject::GetValueDidChange after r231526

2015-03-10 Thread jingham
That's fine. Jim On Mar 10, 2015, at 10:15 AM, Ilia K ki.s...@gmail.com wrote: some changes http://reviews.llvm.org/D8206 Files: include/lldb/API/SBValue.h include/lldb/Core/ValueObject.h Index: include/lldb/API/SBValue.h

Re: [Lldb-commits] [PATCH] Fix deadlock in Listener/Broadcaster::Clear

2015-03-10 Thread jingham
What happens if somebody tries to add a listener to a broadcaster while it is calling clear? Jim On Mar 10, 2015, at 10:30 AM, Pavel Labath lab...@google.com wrote: Hi clayborg, When calling Listener::Clear() and Broadcaster::Clear() simultaneously from two threads a deadlock would

Re: [Lldb-commits] [PATCH] Use eFlagRequiresFrame instead of manually checking in CommandObjectThread.cpp.

2015-03-24 Thread jingham
The only thing that bothers me a bit about this patch is that somebody might think we need to go clean up all the places where a thread doesn't have a frame 0, and worse, actually try to do something reasonable in that case. That would be wasted effort. To that end, an assert here might in

Re: [Lldb-commits] [PATCH] Fix race condition in Target::Launch

2015-03-26 Thread jingham
, it would be better to see how to avoid whatever this delay is papering over, but I don't have the time to open that can of worms right now... Jim On Mar 26, 2015, at 10:17 AM, Zachary Turner ztur...@google.com wrote: In http://reviews.llvm.org/D8562#147401, @jingham wrote: I prefer treating

Re: [Lldb-commits] [PATCH] The great Python split

2015-02-27 Thread jingham
If it is a target, for sure, that's easy. Actually you could do it with a project but that's unnecessary. And Xcode has targets that build libraries, rather than having libraries as first-class objects... Jim On Feb 27, 2015, at 1:22 PM, Zachary Turner ztur...@google.com wrote: Can the

Re: [Lldb-commits] [PATCH] Strip ELF symbol versions from symbol names

2015-02-26 Thread jingham
On Feb 26, 2015, at 9:02 AM, Pavel Labath lab...@google.com wrote: So here is me thinking out loud about this issue... What are the current use cases for the Symbols and SymTabs in lldb? - symbolification (aka looking up a symbol by address): In this case we would probably want to

Re: [Lldb-commits] [PATCH] Add a --all command option to target delete

2015-03-25 Thread jingham
On Mar 25, 2015, at 5:18 PM, Zachary Turner ztur...@google.com wrote: Seems reasonable, I can add something to the help. What do you think about also changing the semantics of --clean to clear the module list regardless of if the modules are orphaned? I've spent the past 2 days trying

Re: [Lldb-commits] [PATCH] Initialize ObjC runtime at the right location.

2015-03-23 Thread jingham
I agree with Zachary. That wasn't what the FIXME was about, and moving more runtime specific code into target is not what is desired. This should go into the Runtime, and get called by the dynamic loader. I don't think this change is a change in the right direction. Jim On Mar 23, 2015,

Re: [Lldb-commits] [PATCH] Add a missing null pointer check in CommandObjectThread.cpp.

2015-03-23 Thread jingham
How do you get a thread that has NO frame 0? That seems weird, we always have a register context to make frame 0... Anyway, if you're going to do this, add it to the thread == NULL checks earlier on in the function so we can get out of the command with an appropriate error message. If we

Re: [Lldb-commits] [PATCH] Add a --all command option to target delete

2015-03-26 Thread jingham
On Mar 25, 2015, at 6:06 PM, Zachary Turner ztur...@google.com wrote: Ironically even though I've been staring at this problem for 2 days I think I actually figured this out shortly after my last email. TestBase.tearDown() already deletes all the targets but it was calling Base.tearDown

Re: [Lldb-commits] [PATCH] Add ModulesDidLoad to LanguageRuntime

2015-04-22 Thread jingham
If we get this in place, it might be useful to add a (lldb) language Language set-exception-breakpoint as well as: (lldb) break set -E Language so that we can pass language specific options for filtering the exception breakpoints. Right now we don't have this capability, but presumably

Re: [Lldb-commits] [lldb] r236174 - Introduce a NullLog class, which ignores all messages.

2015-04-30 Thread jingham
On Apr 29, 2015, at 7:23 PM, Zachary Turner ztur...@google.com wrote: The problem is when the log statements aren't simple, which turns out to be most of the time in my case. They log statement itself wraps a couple times, and when i have an if statement with only one statement inside, i

Re: [Lldb-commits] [lldb] r236174 - Introduce a NullLog class, which ignores all messages.

2015-04-30 Thread jingham
Sorry, mail from lldb-commits has been very flakey for me for some reason, I didn't see this part of the thread till just now... I still think this is a solution in search of a problem, and since we certainly shouldn't change all the log statements in lldb to use some macro - that is code

Re: [Lldb-commits] [lldb] r236174 - Introduce a NullLog class, which ignores all messages.

2015-04-30 Thread jingham
You are overselling the advantages, since there are very few if any functions in lldb that log to more than one set of flags. That's for good reason: any given method should be implementing some coherent piece of the functionality, and if it needs to log to different places as it goes along

Re: [Lldb-commits] [lldb] r236174 - Introduce a NullLog class, which ignores all messages.

2015-04-29 Thread jingham
More importantly, I don't think this is a good change. I want to be able to freely put complex log statements where ever I need without having to worry about the performance implications. That was always possible, because the cost of not logging was checking if log was NULL. With this change

Re: [Lldb-commits] [lldb] r236174 - Introduce a NullLog class, which ignores all messages.

2015-04-29 Thread jingham
Yes, but that call is only used a handful of times, and only from code written back before this was checked into llvm.org, so that's more an argument for removing those calls than making that pattern more common. I really don't want people to have to reason about the expense of logging. I

Re: [Lldb-commits] [lldb] r236174 - Introduce a NullLog class, which ignores all messages.

2015-04-29 Thread jingham
In general, for simple logs we do: if (log) log-Printf(); So you are only saving one line and a bit of indentation per log. I'm not sure I buy that this makes the code that much uglier - especially because it's really the noise of the characters in the log string that distract your eye and

Re: [Lldb-commits] [PATCH] Fix handling of hijacked events in synchronous mode

2015-05-08 Thread jingham
#169036, @jingham wrote: The more serious question I had with this patch was that it looks like this change means you are now generating a stop event when you do a synchronous continue. I don't think anybody is expecting synchronous continues to generate events. Why is this not a problem

Re: [Lldb-commits] [PATCH] Fix handling of hijacked events in synchronous mode

2015-05-08 Thread jingham
see. Jim On May 8, 2015, at 10:16 AM, Ilia K ki.s...@gmail.com wrote: In http://reviews.llvm.org/D9371#169086, @jingham wrote: Tell me again what problem you are trying to solve? In synchronous mode, you always know that when a command that runs the target returns either you stopped

Re: [Lldb-commits] [PATCH] Fix handling of hijacked events in synchronous mode

2015-05-08 Thread jingham
, Ilia K ki.s...@gmail.com wrote: @jingham wrote: I don't understand this. The Command line worked just fine and informed the user about stops without this extra event being generated. Why does the MI need what the command line doesn't? The CL notifies user by printing the (lldb

Re: [Lldb-commits] [PATCH] Fix handling of hijacked events in synchronous mode

2015-05-08 Thread jingham
: In http://reviews.llvm.org/D9371#169068, @jingham wrote: That seems like something you should fix on the MI side. The point of synchronous execution is that you don't have to worry about the events. Now you are introducing an extra event that everybody is going to have to deal

Re: [Lldb-commits] [PATCH] Fix handling of hijacked events in synchronous mode

2015-05-11 Thread jingham
BTW, if you are in async mode and you want to know if some command caused the process to run, you can just check the Process StopID (SBProcess::GetStopID().) You want the default value for include_expression_stops, since you don't want to say stopped if somebody ran a function call. Anyway,

  1   2   >