Page MenuHomePhabricator

jingham (Jim Ingham)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 5:24 PM (251 w, 5 d)

Recent Activity

Thu, Jul 18

jingham accepted D64964: [API] Remove use of ClangASTContext from SBTarget.

This looks fine to me. Makes it really clear that we need SBTarget::FindFirstTypeForLanguage, etc. But FindFirstType was always a crapshoot anyway...

Thu, Jul 18, 5:26 PM
jingham committed rGdb6cfe1337c0: Remember to sort the Xcode project!!! (authored by jingham).
Remember to sort the Xcode project!!!
Thu, Jul 18, 3:28 PM
jingham committed rL366508: Remember to sort the Xcode project!!!.
Remember to sort the Xcode project!!!
Thu, Jul 18, 3:28 PM
jingham committed rGfa6199bc5d37: Add an expectedFailure test for type finding. (authored by jingham).
Add an expectedFailure test for type finding.
Thu, Jul 18, 3:22 PM
jingham committed rL366507: Add an expectedFailure test for type finding..
Add an expectedFailure test for type finding.
Thu, Jul 18, 3:22 PM
jingham committed rGee515d3d03e9: The switch to table-genning command options broke the xcode project. This gets… (authored by jingham).
The switch to table-genning command options broke the xcode project. This gets…
Thu, Jul 18, 3:22 PM
jingham committed rL366506: The switch to table-genning command options broke.
The switch to table-genning command options broke
Thu, Jul 18, 3:22 PM

Wed, Jul 17

jingham added a comment to D64853: Fix CommandInterpreter for _regex-break with options.

This looks good in general. I also have a few nits, and you need to fix (and probably test) the behavior for files with spaces in their names.

Wed, Jul 17, 5:29 PM · Restricted Project
jingham added a comment to D64897: Move start-address finding to Target, implement fallback if main executable does not have a start address.

I wonder if we ought to allow target properties (target as an example) that are only for testing, so they don't print when you do settings list, etc. But the we could have some settings like a "target.testing.dont-read-LC_MAIN" and that would make it easy to automate your hand testing. Kind of like the "experimental" decorator, except you have to know they exist to use them...

Wed, Jul 17, 5:19 PM · Restricted Project, Restricted Project
jingham added a comment to D64844: [Target] Remove Target::GetScratchClangASTContext.

The nice thing about the way the ObjCLanguageRuntime::Get was that is was only useable where we decided it was legit to use it, in the actual ObjCLanguageRuntime plugin or its direct users. If you want to keep the convenience cast function in a header in Plugins/ExpressionParser/Clang, that would be fine.

Wed, Jul 17, 2:57 PM
jingham added a comment to D64844: [Target] Remove Target::GetScratchClangASTContext.

Seems like in most places we're just pulling out basic types or their sizes, which we should certainly be able to do with a TypeSystem.

Wed, Jul 17, 12:27 PM
jingham added a comment to D64844: [Target] Remove Target::GetScratchClangASTContext.

I think I understand your goal - a worthy one, BTW... But I think this is an unnecessary step along that path.

Wed, Jul 17, 12:14 PM
jingham requested changes to D64844: [Target] Remove Target::GetScratchClangASTContext.

This seems partly wrong to me. The point is that the Target holds keeps scratch AST contexts for all the languages it supports. They are the central repository for the accumulation of effects of expressions evaluated while that target is alive. For instance, all the user defined types and variables go there. The Target also manages its lifecycle. So all the Scratch AST Contexts are properly owned by the target. For instance, in the swift case if there's a module that we can't import, we have to fall back to making an AST Context for each module we can successfully import and dispatching expressions to the appropriate one of those.

Wed, Jul 17, 10:23 AM

Fri, Jul 12

jingham accepted D63813: Adjust variable formatting table.

That looks good.

Fri, Jul 12, 3:59 PM · Restricted Project, Restricted Project
jingham accepted D64670: [lldb][doc] Document how our LLDB table gen initialized options.

Thanks for writing this up!

Fri, Jul 12, 3:46 PM · Restricted Project, Restricted Project

Thu, Jul 11

jingham accepted D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser.

That would be cleaner.

Thu, Jul 11, 4:21 PM · Restricted Project
jingham added a comment to D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser.

Then the IRDynamicCheck part would go with LLVMUserExpression, and the C-specific checks in Clang...

Thu, Jul 11, 4:01 PM · Restricted Project
jingham accepted D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin.

Okay... Provided we can come up with not too torturous ways to get the ObjC and Swift support out of generic-code, it seems okay to do this as a first step. I just don't want to end up in the state where we have ObjCLanguageRuntime as generic but CPPLanguageRuntime is not.

Thu, Jul 11, 4:00 PM · Restricted Project
jingham added a comment to D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser.

This is a little unclear to me. LLVMUserExpression is the class that represents "anything that uses LLVM at its back end." Both the Swift & Clang user expressions are instances of this. Running through IR and inserting little callouts is an LLVMUserExpression type thing. So it seems like this move is in the right direction, but to be really clean needs to represent LLVMUserExpression in the Plugin Hierarchy.

Thu, Jul 11, 3:45 PM · Restricted Project
jingham added inline comments to D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin.
Thu, Jul 11, 3:32 PM · Restricted Project
jingham added a comment to D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin.

My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.

I think that's fine, but it does not conflict the idea of the generic parts of a cpp language runtime being a plugin also. This way the generic parts of lldb would be truly language agnostic. Then if anything wants to work with a c++ runtime, but it does not care which one, it can use CPPLanguageRuntime. And if you need a very specific runtime, you go for the Itanium thingy.

Thu, Jul 11, 3:24 PM · Restricted Project
jingham added a comment to D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin.

What is the indented relationship between CPPLanguage and CPPLanguageRuntime plugins (and generally between any Language and its LanguageRuntime)? Right now you're having the CPPLanguage depend on the CPPLanguageRuntime plugin. There is no reverse dependency, so this may be fine, if that's how we intend things to be. However, it's not clear to me whether that's the right way to organize things. Intuitively, I'd expect the LanguageRuntime to depend on Language, and not the other way around...

Thu, Jul 11, 2:48 PM · Restricted Project
jingham requested changes to D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin.

My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.

Thu, Jul 11, 2:26 PM · Restricted Project

Mon, Jul 8

jingham added a comment to D64365: [lldb] Let table gen create command option initializers..

Do you know how you are going to do enum option values?

Mon, Jul 8, 4:52 PM · Restricted Project, Restricted Project
jingham added inline comments to D64159: [Core] Generalize ValueObject::MaybeCalculateCompleteType.
Mon, Jul 8, 2:54 PM · Restricted Project
jingham added a comment to D64042: [Symbol] Improve Variable::GetLanguage.

Why did you make this a function of Variable, rather than SymbolContextScope?

Mon, Jul 8, 2:35 PM

Wed, Jul 3

jingham added inline comments to D64163: Change LaunchThread interface to return an expected..
Wed, Jul 3, 4:16 PM · Restricted Project, Restricted Project

Tue, Jul 2

jingham committed rG372cee511e27: Fix for r364686 - actually set symbol_is_missing_weak... (authored by jingham).
Fix for r364686 - actually set symbol_is_missing_weak...
Tue, Jul 2, 4:41 PM
jingham committed rL364980: Fix for r364686 - actually set symbol_is_missing_weak....
Fix for r364686 - actually set symbol_is_missing_weak...
Tue, Jul 2, 4:40 PM

Mon, Jul 1

jingham accepted D63853: [Symbol] Add DeclVendor::FindTypes.

LGTM

Mon, Jul 1, 5:25 PM · Restricted Project
jingham accepted D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

Sure, NP.

Mon, Jul 1, 11:30 AM · Restricted Project

Fri, Jun 28

jingham committed rGf2128b28cdb7: Get the expression parser to handle missing weak symbols. MachO only for this… (authored by jingham).
Get the expression parser to handle missing weak symbols. MachO only for this…
Fri, Jun 28, 2:41 PM
jingham committed rL364686: Get the expression parser to handle missing weak symbols..
Get the expression parser to handle missing weak symbols.
Fri, Jun 28, 2:40 PM
jingham closed D63914: Make the expression parser work for missing weak symbols.
Fri, Jun 28, 2:40 PM · Restricted Project, Restricted Project
jingham added inline comments to D63914: Make the expression parser work for missing weak symbols.
Fri, Jun 28, 2:37 PM · Restricted Project, Restricted Project
jingham updated the diff for D63914: Make the expression parser work for missing weak symbols.

Addressed Greg's comments

Fri, Jun 28, 2:34 PM · Restricted Project, Restricted Project
jingham committed rG8864b4360aab: Make sure the thread list is updated before you set the stop reason on a thread. (authored by jingham).
Make sure the thread list is updated before you set the stop reason on a thread.
Fri, Jun 28, 10:58 AM
jingham committed rL364666: Make sure the thread list is updated before you set the stop reason.
Make sure the thread list is updated before you set the stop reason
Fri, Jun 28, 10:58 AM
jingham closed D62887: Update the thread list before setting stop reasons with an OS plugin.
Fri, Jun 28, 10:58 AM · Restricted Project, Restricted Project
jingham added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

This looks good to me. I wonder if the SymbolContextScope -> Language calculation that you do in IsRuntimeSupportValue should really be done in SymbolContextScope? If that's going to be the policy for going from SymbolContextScope, maybe centralize it there?

Fri, Jun 28, 10:07 AM · Restricted Project

Thu, Jun 27

jingham created D63914: Make the expression parser work for missing weak symbols.
Thu, Jun 27, 6:07 PM · Restricted Project, Restricted Project
jingham added inline comments to D62887: Update the thread list before setting stop reasons with an OS plugin.
Thu, Jun 27, 6:07 PM · Restricted Project, Restricted Project
jingham updated the diff for D62887: Update the thread list before setting stop reasons with an OS plugin.

Addresses Greg's question about what happens when we load a new OS plugin. Indeed we should limit the work we do only to the case where we didn't have an OS plugin, then tried to load one and succeeded. Only then do we need to clear and update the thread list.

Thu, Jun 27, 6:07 PM · Restricted Project, Restricted Project
jingham added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

@jingham: Okay, so I tried to do what you suggested and that solution doesn't work because of ObjC++. After looking into it, it looks like Variable and Function just ask the CompileUnit for the language type instead of determining it themselves meaning that we determining the Variable's language boils down to asking the CompileUnit for its language. That can probably be improved, but I think that just relying on the SymbolContextScope alone isn't yet sufficient. I think it would be worth it to ask the SymbolContextScope before trying all the loaded language runtimes though. Would you be okay with that solution?

Thu, Jun 27, 3:09 PM · Restricted Project

Wed, Jun 26

jingham added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

To be more precise, "frame" is the wrong word to use, Variables have "scopes"... All Variables have a scope, though not every scope is contained in a Function.

But just to back up a little bit.

We're getting into problems, it seems to me, because we're asking ValueObjects questions that should be asked of their containing scope. For instance, if you are in a Function, and want to list the locals and arguments, you want to know what the rules for the Function are for which artificial variables should and should not be printed, not any of its value objects.

For instance, ObjC could decide that it wants to reserve the word "this" in methods of ObjC classes in ObjC++ files, and use it as a should-be-hidden artificial variable that will always store a pointer to C++ object for some evil purposes. Asking the runtime language of this artificial "this" variable whether the word "this" is runtime whitelisted seems like the wrong thing to do. After all, the Variable has a CompilerType that's clearly a C++ object, so you might reasonably expect its runtime language to be C++. But then you would get the wrong answer. You really need to ask the Function "what are your rules for hiding artificial variables".

Getting to the right actor to ask about "which artificial variables to show" was the reason for suggesting that the runtime language of a Variable should be that of its containing scope. But then you get into the weird situation where a C++ Variable is claiming a runtime language of ObjC++, which seems a little odd. Instead, it makes more sense to get the RuntimeLanguage of the current frame, and then iterate over the variables and ask the frame's runtime whether they should be shown or not.

I'm not sure what to do about global variables. Reasoning by analogy, that's something that the language of the CompileUnit which contains the global variables should answer.

If I understood you correctly, your conclusion is that ValueObject::IsRuntimeSupportValue shouldn't really exist and that you should be asking a frame or a compilation unit if a value should be displayed to the user. I think that this is reasonable. That being said, I think that this idea will have some interesting challenges as well. One problem that I'm aware of at the moment is that SBValue has IsRuntimeSupportValue. If I remember correctly, removing from the SBAPI is generally not permitted (something I disagree with in general, but I digress).

In the short term, I'd like to get this patch (or some variation of it) into LLDB so that we can remove ValueObject's dependence on ObjC (after this there's one reference left but I have a good idea of how to remove it). Would you mind if I committed this, or do you feel strongly that we should first refactor further?

Wed, Jun 26, 5:04 PM · Restricted Project
jingham added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

Shouldn't ValueObjectVariables figure out their runtime language from their defining frame, not their CompilerType? For a ValueObject you get from an expression, you probably can't do that. But we're always talking about hiding locals or args here - i.e. they are all ValueObjectVariables. And it seems to me that in that case getting the RuntimeLanguage from the containing frame is much more useful than from the CompilerType.

Does every variable have an associated frame? I imagine things like global variables wouldn't. I'm not sure if any language or implementation of a language has global variables as compiler/runtime helper variables, but I'm not comfortable making that assumption since one of my goals is more generalized language support.

Wed, Jun 26, 4:01 PM · Restricted Project
jingham added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

Shouldn't ValueObjectVariables figure out their runtime language from their defining frame, not their CompilerType? For a ValueObject you get from an expression, you probably can't do that. But we're always talking about hiding locals or args here - i.e. they are all ValueObjectVariables. And it seems to me that in that case getting the RuntimeLanguage from the containing frame is much more useful than from the CompilerType.

Wed, Jun 26, 2:51 PM · Restricted Project
jingham requested changes to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Wed, Jun 26, 10:28 AM · Restricted Project

Mon, Jun 24

jingham added inline comments to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Mon, Jun 24, 10:19 AM · Restricted Project

Jun 21 2019

jingham added inline comments to D63622: [Target] Hoist LanguageRuntime::GetDeclVendor.
Jun 21 2019, 3:31 PM · Restricted Project
jingham added a reviewer for D63667: Support __kernel_rt_sigreturn in frame initialization: jasonmolenda.
Jun 21 2019, 1:46 PM · Restricted Project, Restricted Project
jingham added inline comments to D63622: [Target] Hoist LanguageRuntime::GetDeclVendor.
Jun 21 2019, 1:34 PM · Restricted Project
jingham updated subscribers of D63622: [Target] Hoist LanguageRuntime::GetDeclVendor.

Seems like a reasonable thing to do, but I don't really know what this code does...

Jun 21 2019, 10:51 AM · Restricted Project
jingham added inline comments to D63622: [Target] Hoist LanguageRuntime::GetDeclVendor.
Jun 21 2019, 10:35 AM · Restricted Project
jingham added inline comments to D63622: [Target] Hoist LanguageRuntime::GetDeclVendor.
Jun 21 2019, 10:35 AM · Restricted Project

Jun 20 2019

jingham added a comment to D63622: [Target] Hoist LanguageRuntime::GetDeclVendor.

This change makes it clear that SBTarget::FindFirstType should take a language, but that is orthogonal to this change.

Jun 20 2019, 2:17 PM · Restricted Project

Jun 18 2019

jingham added a comment to D63363: [Signals] Create a plugin directory just for signals.

Although this is technically correct and pretty consistent with our other "plugins", I can't help but feel that it's incredibly wasteful. Each of the XXXSignals.cpp files is less than a 100 lines long (with the licence header and all bolierplate) and it's unlikely to ever grow beyond that. And essentially, all these files do is define a single enum. The reason they are this long is because the UnixSignals class is already over-engineered (e.g. I don't see why LinuxSignals needs to be a separate class, or why it needs to repeat the descriptions/default stop values for each of the signals). Making this a plugin would just add another chunk of boilerplate on top of that.

I don't know about others, but I'd rather us move in a direction which reduces the amount of boilerplate instead of adding more of it. In my ideal world, each of these signal definitions would just be a bunch of (number, name) pairs. This doesn't have/need to be a class or a plugin; a single constexpr variable would suffice for that. Then we'd just cross-reference this mapping with another one which gives the default stop values and descriptions for each of the signals, and that's it.

I know I am repeating myself, but each time I say this, it's because I find another reason for it: I think we should start a new library which I would roughly define as "utilities for describing and manipulating various low-level aspects of processes, but which is agnostic of any actual process class". The idea would be that we can shove here all classes which are shared between lldb-server liblldb. UnixSignals would be one candidate for it. AuxVector, MemoryRegionInfo are others. Plugins/Process/Utility (where most of the signal classes live) would be a pretty good place for it already, were it not for the "Plugins" part (it would be weird for non-plugin code to depend on something called a "plugin"). However, maybe we could just create a new top-level library called "ProcessUtil" (or whatever name we come up with) and move the relevant stuff there.

Anyway, TL;DR: I think this should be handled differently. However, if others are fine with this approach, then feel free to ignore me.

Jun 18 2019, 5:31 PM
jingham added a comment to D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime.

This looks fine to me.

Jun 18 2019, 5:12 PM · Restricted Project
jingham added a comment to D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime.

Have you considered making just AddExceptionPrecondition virtual? Wouldn't that solve the problem too, without the code duplication of making CreateExceptionBreakpoint virtual?

I don't believe so. As I understand it, you might not have a process yet when you call LanguageRuntime::CreateExceptionBreakpoint from Target, so you won't have an instance of LanguageRuntime. You could create an instance of a LanguageRuntime object with a nullptr process just to invoke it as an instance method, but that seems like the wrong choice to me. All this combined has led me to believe that AddExceptionPrecondition needs to get a callback to a static function from PluginManager.

Jun 18 2019, 5:11 PM · Restricted Project

Jun 4 2019

jingham created D62887: Update the thread list before setting stop reasons with an OS plugin.
Jun 4 2019, 6:54 PM · Restricted Project, Restricted Project
jingham added a comment to D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl.

Pretty much the only actual effect of this patch (besides its changes to the dependency graph) are introducing the possibility for ambiguity here. It seems more logical to do it all as one patch.

Jun 4 2019, 3:57 PM · Restricted Project

Jun 3 2019

jingham added a comment to D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime.

This looks fine to me.

Jun 3 2019, 6:35 PM · Restricted Project
jingham added a comment to D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl.

I have no problem with the change in general. However, you've introduced the possibility of name collision between convenience type definition in various languages. So it would be good to run through the persistent DECL's for ALL the supported languages and report collisions. Probably also need to add a -l flag to pick a language?

Jun 3 2019, 6:27 PM · Restricted Project
jingham added a comment to D62755: [Target] Remove Process::GetCPPLanguageRuntime.

Fine by me.

Jun 3 2019, 6:13 PM · Restricted Project
jingham accepted D62755: [Target] Remove Process::GetCPPLanguageRuntime.

This deals with my objections so I'm marking as accepted because that's the only way I can see to unblock. Seems like you were still discussing return types, and I don't want to preempt that, however...

Jun 3 2019, 5:03 PM · Restricted Project

May 31 2019

jingham added a comment to D62755: [Target] Remove Process::GetCPPLanguageRuntime.

I don't yet see the connection between those goals and this patch, but I might be missing something. Would CPPLanguageRuntime need to be anything but a forward declaration in Process.h?

I forgot to address this point. CPPLanguageRuntime would not be a forward declaration in Process.h, nor be referred to anywhere in Process.h

May 31 2019, 5:35 PM · Restricted Project
jingham requested changes to D62755: [Target] Remove Process::GetCPPLanguageRuntime.

This seems like carrying purity a little too far. You haven't removed the fact that the using code is explicitly dialing up the C++ language runtime, you've just made the call-site a little harder to read.

May 31 2019, 3:46 PM · Restricted Project

May 30 2019

jingham added a comment to D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register.

There should be a test to go along with this.

May 30 2019, 2:02 PM · Restricted Project, Restricted Project
jingham accepted D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough.

LGTM

May 30 2019, 1:14 PM · Restricted Project

May 29 2019

jingham accepted D62562: [Target] Introduce Process::GetLanguageRuntimes.

LGTM

May 29 2019, 10:46 AM · Restricted Project
jingham added a comment to D62472: [CMake] LLDB.framework tools handling.

The typical trick for doing argument substitution before debugging was roughly to debug:

May 29 2019, 10:41 AM · Restricted Project, Restricted Project

May 22 2019

jingham committed rGab43d1d888a5: Remove unused const version of CommandInterpreter::GetCommandHistory. (authored by jingham).
Remove unused const version of CommandInterpreter::GetCommandHistory.
May 22 2019, 6:39 PM
jingham committed rL361455: Remove unused const version of CommandInterpreter::GetCommandHistory..
Remove unused const version of CommandInterpreter::GetCommandHistory.
May 22 2019, 6:37 PM
jingham committed rLLDB361455: Remove unused const version of CommandInterpreter::GetCommandHistory..
Remove unused const version of CommandInterpreter::GetCommandHistory.
May 22 2019, 6:37 PM
jingham committed rG020d7f1abbcb: Ack, added DWARFTypeUnit to the wrong target... (authored by jingham).
Ack, added DWARFTypeUnit to the wrong target...
May 22 2019, 5:11 PM
jingham committed rL361447: Ack, added DWARFTypeUnit to the wrong target....
Ack, added DWARFTypeUnit to the wrong target...
May 22 2019, 5:10 PM
jingham committed rLLDB361447: Ack, added DWARFTypeUnit to the wrong target....
Ack, added DWARFTypeUnit to the wrong target...
May 22 2019, 5:10 PM
jingham committed rGbb7357750e7f: Add DWARFTypeUnit to the Xcode project. (authored by jingham).
Add DWARFTypeUnit to the Xcode project.
May 22 2019, 12:04 PM
jingham committed rL361420: Add DWARFTypeUnit to the Xcode project..
Add DWARFTypeUnit to the Xcode project.
May 22 2019, 12:03 PM
jingham committed rLLDB361420: Add DWARFTypeUnit to the Xcode project..
Add DWARFTypeUnit to the Xcode project.
May 22 2019, 12:03 PM

May 21 2019

jingham added a comment to D62216: [EditLine] Rewrite GetHistoryFilePath.

Then put in a comment saying something like "LLDB ONLY stores history files in .lldb and if you don't like that..." If you are instituting a policy which is not common you should at least document it...

May 21 2019, 3:55 PM · Restricted Project
jingham added a comment to D62216: [EditLine] Rewrite GetHistoryFilePath.

Most other programs write their history files in ~. So we are being a little odd in offering to put them in ~/.lldb, though I agree that is convenient.

May 21 2019, 3:36 PM · Restricted Project
jingham added a comment to D62216: [EditLine] Rewrite GetHistoryFilePath.

I don't think that's quite right. I think the behavior should be:

May 21 2019, 3:04 PM · Restricted Project
jingham added a comment to D62216: [EditLine] Rewrite GetHistoryFilePath.

Why are we calling this "widehistory" because we couldn't write into the .lldb directory? I know that's the way it was but it makes no sense. I guess it would make sense to call it widehistory if edit line was in wchar mode, that way you wouldn't ask a non wchar edit line to read a wchar history file...

May 21 2019, 1:39 PM · Restricted Project

May 15 2019

jingham added a comment to D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough.

If this iteration is going to be used a lot, I'd recommend taking a bit of time to implement an iterator abstraction over the language runtimes. It takes a bit longer to set up, but I hope we can all agree that for (runtime: process->GetLanguageRuntimes()) runtime->foo(); is more readable than process->ForEachLanguageRuntime([] (runtime) { runtime->foo(); }). This is particularly true if you need some sort of a control flow construct (continue, break, return) in the loop "body".

+1

Yeah this seems reasonable.

On macOS, there are a handful of runtimes that the Plugin Manager knows about - C++, ObjC, maybe Swift. But, for instance, we only load the ObjC language runtime into the process' m_language_runtimes array when we see libobjc.dylib get loaded. A pure C++ program might never load libobjc.dylib, and so even though the Plugin Manager knows we have support for the ObjC language runtime, that plugin wouldn't be active in the current lldb_private::Process. So there's a real difference between the "available" and the "currently loaded" runtimes. I was saying I didn't see any compelling reason to have an iterator over the available runtimes, just over the loaded ones. Not that we didn't need one or the other iterator.

Okay, that makes sense to me. Thanks for clearing that up. If I understand correctly (which I think I do), creating an iterator abstraction to iterate over the process' currently loaded runtimes instead of over every available runtime is the better option here. I'll follow up with that change later today or tomorrow unless somebody believes this is the wrong idea.

May 15 2019, 11:17 AM · Restricted Project
jingham added a comment to D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough.

Getting it from the Process's m_language_runtimes is probably fine. On reflection, I can't think of a reason why you would want to iterate over all the available LanguageRuntimes, including the ones that hadn't been recognized in this Process yet.

It's fine to do that in a separate patch. If you do that it would be good to back port it to the other iterations over the LanguageRuntimes.

Excellent, sounds good. I definitely intend to backport it to any other iterations over LanguageRuntimes that I find. As for iterating over all available LanguageRuntimes, this patch and my last few have sufficient motivation for doing so, no? Maybe I'm misunderstanding what you mean.

May 15 2019, 10:29 AM · Restricted Project
jingham added a comment to D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough.

If this iteration is going to be used a lot, I'd recommend taking a bit of time to implement an iterator abstraction over the language runtimes. It takes a bit longer to set up, but I hope we can all agree that for (runtime: process->GetLanguageRuntimes()) runtime->foo(); is more readable than process->ForEachLanguageRuntime([] (runtime) { runtime->foo(); }). This is particularly true if you need some sort of a control flow construct (continue, break, return) in the loop "body".

May 15 2019, 10:20 AM · Restricted Project

May 14 2019

jingham added a comment to D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough.

Getting it from the Process's m_language_runtimes is probably fine. On reflection, I can't think of a reason why you would want to iterate over all the available LanguageRuntimes, including the ones that hadn't been recognized in this Process yet.

May 14 2019, 6:22 PM · Restricted Project
jingham committed rG7d7b788fb186: Make SBDebugger.RunCommandInterpreter callable from Python. (authored by jingham).
Make SBDebugger.RunCommandInterpreter callable from Python.
May 14 2019, 5:08 PM
jingham committed rL360730: Make SBDebugger.RunCommandInterpreter callable from Python..
Make SBDebugger.RunCommandInterpreter callable from Python.
May 14 2019, 5:08 PM
jingham committed rLLDB360730: Make SBDebugger.RunCommandInterpreter callable from Python..
Make SBDebugger.RunCommandInterpreter callable from Python.
May 14 2019, 5:08 PM
jingham closed D61602: Handle function parameters of RunCommandInterpreter (script bridge).
May 14 2019, 5:08 PM · Restricted Project
jingham added a comment to D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough.

There is a TypeSystemEnumerateSupportedLanguages that we use so that we don't have to enumerate over all the language in the languages enums. After all the plugin manager knows which languages we have type systems for. If you're going to be doing a lot of this generalization, can you add a similar enumeration for the LanguageRuntimes? There are 40 or so languages in the language enum but we only have a couple of LanguageRuntimes...

May 14 2019, 4:48 PM · Restricted Project

May 13 2019

jingham accepted D61602: Handle function parameters of RunCommandInterpreter (script bridge).

Excellent, thanks! Can you check this in or do you need me to?

May 13 2019, 10:00 AM · Restricted Project

May 10 2019

jingham added a comment to D61737: [lldb] add -ex CLI option as alias to --one-line.

If you have simple gdb startup commands then translating them into the lldb way should be straightforward, and the burden of learning how lldb works from the command line for these purposes is just not that great and if you are going to use lldb you should just buck up and browse "lldb --help" a bit... If you have complex gdb command lines, you are going to have to rework them anyway to deal with other differences between lldb and gdb. I don't think mixing the two would add enough value to be worth the uglification.

I use GDB to debug LLDB like: gdb -q -ex 'set break pend on' -ex 'set build-id-verbose 0' -ex 'set env ASAN_OPTIONS=alloc_dealloc_mismatch=0:detect_leaks=0' -ex 'set env LD_PRELOAD=/lib64/libasan.so.5' -ex "set env PYTHONPATH=$PWD/lib64/python2.7/site-packages:$PWD/lib64/python2.7/site-packages/lldb" -ex "set env LD_LIBRARY_PATH=$PWD/lib:$PWD/lib64/python2.7/site-packages/lldb" -iex 'set breakpoint pending on' -iex 'b __sanitizer::internal__exit' -ex 'b lldb_private::Module::ReportError' -ex 'b lldb_private::Host::SystemLog' -ex r -ex 'set scheduler-locking on' --args python ../llvm-git/tools/lldb/test/dotest.py --executable $PWD/bin/lldb -C $PWD/bin/clang --log-success -t ../llvm-git/tools/lldb/packages/Python/lldbsuite/test/ -f HelloWatchpointTestCase.test_hello_watchpoint_using_watchpoint_set_dwarf

May 10 2019, 1:55 PM · Restricted Project
jingham accepted D61776: [Target] Generalize some behavior in Thread.

With appropriate comments, this seems fine.

May 10 2019, 12:03 PM · Restricted Project
jingham added a comment to D61737: [lldb] add -ex CLI option as alias to --one-line.

I really don't want to pollute the lldb driver options with gdb equivalents (or really any duplicate spellings of already present functionality). For the lldb command line, I want us to have the freedom to do the best job of making the lldb options consistent and easy to document, understand and use, and adding in random duplicate options from gdb will only make this harder.

May 10 2019, 11:17 AM · Restricted Project
jingham added a comment to D61776: [Target] Generalize some behavior in Thread.

If you really are going to support many languages you need to figure out how to tell folks what really happened with more specificity. For instance, you can use C++ to throw anything, so you could throw an ObjC object from a C++ exception throw. So you need to distinguish between the language of the exception thrown (which is captured by the ValueObject you return) and the language runtime throwing the language. So we need a way to query that. Also, there's no formula reason why you couldn't have two languages throwing an exception at the same time (for instance if a C++ exception is unwinding the stack and the destructor of some ObjC object that is getting destroyed throws an NSException, etc... So there needs to be some way to handle that.

May 10 2019, 10:15 AM · Restricted Project

May 9 2019

jingham added a comment to D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one.

The one outstanding bit of work here is that this change requires that the MSInst "IsCall" function has to mean "will return to the next instruction after call" or we might lose control of the program. It seems obvious that that SHOULD be what it means, but we need to make sure that is always going to be what it means or we risk losing control of the program. Greg is going to follow up on that.

May 9 2019, 6:03 PM · Restricted Project
jingham requested changes to D61737: [lldb] add -ex CLI option as alias to --one-line.

I would rather not clutter up the lldb command driver's options with gdb command flags. That seems like it will make lldb harder to figure out and reduce our freedom to choose reasonable short names for lldb driver options.

May 9 2019, 1:29 PM · Restricted Project