[Lldb-commits] [PATCH] D52376: [API][Swig] Overloaded functions for SBTarget

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This patch changes the SB API. We don't break binary compatibility with the SB API's, and especially not for something like GetInstructions/GetInstructionsWithFlavor, which are fu

[Lldb-commits] [PATCH] D52376: [Swig] Merge typemaps with same bodies

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Nice, thanks! Repository: rLLDB LLDB https://reviews.llvm.org/D52376 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.

[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process

jingham added a comment. I agree with Zachary, converting to NativeProcess is enough of a project that we should not block useful fixes to the current ProcessWindows plugin on it. OTOH, it is a good project - somebody should add it to the Projects page. https://reviews.llvm.org/D52618

[Lldb-commits] [PATCH] D52651: Add functionality to export settings

jingham added a comment. To Raphael's points: Argument completion handlers are set by the command object implementing HandleArgumentCompletion and dispatching to the CommandCompletions::eDiskFileCompletion. An example of this is in CommandObjectProcessLaunch. Note, arguments currently have a

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

jingham added a comment. The only thing that worries me about this fix is that it's addressing a problem that's a pretty easy mistake to make. Is there ever a case that you would want to resolve a DWARF expression in the context of a .o file module? If not, is there some way we can make this

[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"

jingham added a comment. Would it be possible for the exporter to notice empty settings and write "settings clear" instead? I'm worried that if you have a complicated setting, and the you do: (lldb) settings set target.some-complex-setting and decide you are wrong, you don't want to change th

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

jingham added a comment. I was thinking more like if it's possible to figure out that a given module has a debug_map parent, "SetModule" could automatically redirect to the that module. Then all clients have to do is SetModule with the module they have to hand and it would get the right thing.

[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

jingham added a comment. The StopOnCrash logic is slightly different. Because the outer if is "GetStopOnCrash()" you will only set stop on crash if it is set at this level and was set in the previously pushed flag set. That forces StopOnCrash to be set from the top all the way down. For Stop

[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"

jingham added a comment. I'm thinking of the scenario where you type: (lldb) settings set target.run-args and then decide you didn't want to change the run-args after all. Before this change hitting return used to be a way to dismiss the settings operation, and that actually seems a pretty na

[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"

jingham added a comment. And more, I just like operations to be explicit and not have overloads like "settings set property" == "settings clear property". Repository: rLLDB LLDB https://reviews.llvm.org/D52772 ___ lldb-commits mailing list lldb-

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

jingham added a comment. Just a couple of trivial requests, mostly about comments... Comment at: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:27-28 + +@add_test_categories(["libc++"]) +def test(self): +

[Lldb-commits] [PATCH] D53010: Add an alias "var" for "frame var" and "vo" for "frame var -O"

jingham created this revision. Herald added subscribers: lldb-commits, abidh. "expression" is a hugely inefficient way to get the value of a local variable. There are a few cases where "frame variable" and "expression" will produce different results on the same expression (e.g. "foo->bar" when

[Lldb-commits] [PATCH] D53010: Add an alias "var" for "frame var" and "vo" for "frame var -O"

This revision was automatically updated to reflect the committed changes. Closed by commit rL344102: Add "var" and "vo" aliases for "frame variable" and "frame variable -O". (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

jingham added a comment. IIRC, the test is disabled on Linux not because of problems with building the test executables, but because the Linux port doesn't currently load the dependencies of an binary when it loads the binary. So the test was irrelevant on Linux, as was the feature. Reposito

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Couple more lines you can delete from the test case, and I think you should make this a debug-variant insensitive test. Do that and this is good. Comment at: p

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks great, thanks for making this work, it will be SO helpful for debugging std::function uses. https://reviews.llvm.org/D52851 ___ lld

[Lldb-commits] [PATCH] D53208: [lldbsuite] Fix the mac version decorator to work on non-mac platforms

jingham added a comment. We do compose the decorators in a bunch of places (like Shafik's usage here). That will work more naturally if the categories that the decorators assert are as decoupled as possible. So the statement about macos version should only skip the test if the os is macos, an

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

jingham added a comment. I wouldn't be surprised if Jonas isn't familiar enough with the Windows port to answer the question. But if you have a running lldb you can tell easily. Just do: lldb SomeBinaryThatLoadsSharedLibraries (lldb) image list Do you get only SomeBinaryThatLoadsSharedLibrar

[Lldb-commits] [PATCH] D53208: [lldbsuite] Fix the mac version decorator to work on non-mac platforms

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That seems more convenient to me, it makes the mac version only relevant if you are on a mac. Repository: rLLDB LLDB https://reviews.llvm.org/D53208 __

[Lldb-commits] [PATCH] D53309: Return a useful "Error" for an expression that completes but produces no result

jingham created this revision. Herald added subscribers: lldb-commits, abidh, JDevlieghere. When you run an expression like: (lldb) expr int $x = 10 the expression has no result. The ValueObject (and then SBValue) you get back from the expression signals that by putting an error indicating thi

[Lldb-commits] [PATCH] D53309: Return a useful "Error" for an expression that completes but produces no result

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB344647: Return a named error in the result object of an expression with no result (authored by jingham, committed by ).

[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. And tick the checkbox... Repository: rLLDB LLDB https://reviews.llvm.org/D53361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface

jingham added a comment. Presumably you are doing this so one scripted thread plan can use another one? This is the right way to do that, so this is great. Not sure why I didn't think of it. BTW, looks like I added this without any tests. Totally my bad, this was a weekend experiment and wa

[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

jingham added a reviewer: clayborg. jingham added a comment. This seems like the sort of thing Greg should have a look at as well. Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

jingham added a reviewer: apolyakov. jingham added a comment. This looks okay to me, there's a trivial comment nit... But Alexander has been doing the most work on MI recently, he might want to give this a look-over. Repository: rL LLVM https://reviews.llvm.org/D52953 ___

[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface

jingham added a comment. This should be easy to test with the python testsuite (lldbtest.) Start with the sample_test in packages/Python/lldbsuite/test - don't use the inline one, that won't be flexible enough. Then you can just make a scripted thread plan that just pushes a "step over" or "s

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

jingham added a comment. This seems good to me, but Greg is more expert than I so I'd wait on his okay. Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1901-1907 +dwarf->GetTypeList()->Insert(type_sp); +// Cache the type if it isn't context-s

[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum

jingham added a comment. It would be great not to have to use comments to express what these values mean. OTOH, having to put in static casts whenever you want to or values together would be a pain, so if there's no way to do it without magic, I'm a little less enthused... But on the magic: I

[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum

jingham added a comment. That would be great! https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum

jingham added a comment. That looks good to me, though you should wait for Greg to weigh in. He might notice something I missed. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

jingham added a comment. This is good. The addition of the "info" command will be helpful for people trying to debug their recognizers. It's okay to add multiple -s and -n's later - though the fact that you don't allow "apply to all frames" may make us want the ability to provide more than on

[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface

jingham accepted this revision. jingham added a comment. That looks right. Thanks for adding the test! https://reviews.llvm.org/D53361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com

[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks fine. https://reviews.llvm.org/D52772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks reasonable to me. https://reviews.llvm.org/D53656 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cg

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. I worry that your patch changes the behavior when you add the type_class explicitly in the lookup (i.e. look up "struct Struct" not "Struct". That should work... Note, this doesn't currently work in type lookup: (lldb) type lookup "struct Foo" no type was found ma

[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D53677 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks good to me. Looking at the addition of Type::ConsumeTypeClass makes it really clear that both this function and Type::GetTypeScopeAndBasename need to dispatch to the CompilerSyst

[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum

jingham added subscribers: clayborg, zturner. jingham added a comment. So far as I can tell, this patch will make lookup of exact types faster for PDB, but because of the way DWARF debug_names tables are constructed, I don't think there's any way we can do the same thing for DWARF. But unless I

[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum

jingham added a comment. Ah, right... Too many patches (a good problem!) The standard as I read it says that the name entry points into the general string table, but doesn't specify which entry it points to. However, the current DWARF debug_info doesn't ever emit a string for the fully qualif

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. So the current approach relies on the ability to sniff the name to determine the context in which the user intends to find it. It does (and always did) use the presence of an initial "::" to tell whether a symbol is exact. That's obviously also inappropriate for a gen

[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables

jingham added a comment. Is there anything PDB specific about the test you've added? If not, it might be good to use this as a generic SymbolFile test. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. It seemed to me like Greg thought you were changing the behavior of lookups, which this patch doesn't do, it just makes it more efficient. I don't know if that alters his objections or not. The Module and higher layer of FindTypes calls are awkward. For instance Modu

[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables

jingham added a comment. Well, what's really going on is that I'm not familiar enough with lit to know that it doesn't have the ability to run different commands to produce the input file... But as you guessed, my point is that you have written a bunch of tests that would be valuable to test a

[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables

jingham added a comment. Could you also use Vedant's new FileCheck dotest test class? That should allow you to write the tests exactly as you are, but use the dotest mechanism to build and run the example. https://reviews.llvm.org/D53731 ___ lldb

[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables

jingham added a comment. dotest tests don't require a process. Presumably dotest knows how to build windows targeted PDB debug flavor files (to go along with dwarf/dsym/etc.). So it would be straightforward to make a test that had your input sources, built and made a target out of it and then

[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables

jingham added a comment. The first of the commands you showed presents info we definitely should add to type lookup. I actually have a bug around to do that, but it hasn't risen to the top of my queue because it's trivial to do with the SB API's so every time I've needed that info I get it fro

[Lldb-commits] [PATCH] D53761: [Target] Do not skip a stop on a breakpoint if a plan was completed

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This seems safe, but you certainly want to add a comment explaining why you are doing this. We find the expression breakpoint by calling ObjectFile::GetEntryPointAddress on the ma

[Lldb-commits] [PATCH] D53761: [Target] Do not skip a stop on a breakpoint if a plan was completed

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D53761 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/l

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. Exact matches aren't a C++ specific thing. An exact match is useful for mixed C/C++ since you might want to say Foo and not Bar::Foo. IIRC a couple of the places where exact was dialed up explicitly in FindTypes were for C types. But since C and ObjC allow multiple

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. That depends on the definition of "fully qualified name". If you can ensure that it means "name of C++ class" - or other ODR enforcing type system, then you could make that assumption. In C you are free to redefine types on a per-function basis if you so desire; and s

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. You could also get multiple matches if you had classes in anonymous namespaces with the same name in multiple compile units. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://l

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. Yes, that usage is exactly the sort of use I was talking about. When we get an ObjC object and want to find its dynamic type, we read the type name by following the object's ISA pointer to the Class object. Then we go to look up the type from that name. We know we wa

[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

jingham added a comment. Yes. You can't put them in the same CompileUnit because declaring variables of the type would be ambiguous (though all other references would be okay since ObjC method dispatch looks totally different from C++ method calling). But there's nothing keeping the same modu

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM. Thanks for working on this! https://reviews.llvm.org/D44603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.

[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32

jingham added a comment. There were a bunch of other tests testing wchar_t handling, all in: lang/cpp/wchar_t as well as some tests in: functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py Those tests weren't failed (except for the latter test, and t

[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32

jingham added a comment. So the deal is that we were relying on a summary formatter to print wchar_t before, and now you have a format option that handles them as well? Do we need both? Maybe the summary also handles wchar_t * strings? As an aside, for reasons that are not entirely clear to me

[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32

jingham added a comment. It doesn't seem unreasonable to want to build lldb for smaller systems that don't have Python available, and in fact we do that internally at Apple. Actually, it DOES seem a little unreasonable to me because after all you can just run the debugserver/lldb-server and con

[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32

jingham added a comment. Note also, the vast majority of the uses of LLDB_DISABLE_PYTHON are related to the requirement that we have Python to use any of the data formatter facilities. Those uses shouldn't be necessary. All the scripted interpreters should go through the generic ScriptInterpr

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

jingham created this revision. jingham added a reviewer: aprantl. Herald added a subscriber: lldb-commits. Sometimes you want to make sure that the target doesn't run at all when running an expression, and you'd rather the expression fail if it would have run the target. You can do this with th

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

jingham added a comment. --jit wouldn't describe what the flag actually does. Currently allow-jit and the SB Setting I added for it set the execution policy to eExecutionPolicyWhenNeeded, not eExecutionPolicyAlways. So this really does just allow the JIT to be used, it doesn't force it. If w

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

This revision was automatically updated to reflect the committed changes. Closed by commit rL346053: Add an SBExpressionOptions setting mirroring the "exec" command's --allow-jit. (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

jingham added a subscriber: aprantl. jingham added a comment. Thanks for doing this! Why do we care about the file name of the test, shouldn't we be using the test class name for everything (that one I did remember to change...) Jim Repository: rL LLVM https://reviews.llvm.org/D54056 _

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Looking at the extant API's we do seem to prefer "const CompilerType &" over "const CompilerType" so it seems more consistent to choose that. Other than that, this looks fine to

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This is pretty good, but in all the places where some plan tries to find a sub-plan to do its job, you are losing the text of the error when that job fails. So the controlling pla

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

jingham added a comment. You were probably speaking loosely, but to be clear, the code you are changing doesn't get used for expressions - i.e. the expression command - unless I'm missing something. This little mini-parser is for doing things like: (lldb) frame variable foo.bar We don't use c

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

jingham added a comment. Those seem legit things to try to capture, though a little esoteric. Since "frame variable" and "target variable" didn't support these constructs before you should certainly add some tests for that. The frame variable parser also supports: (lldb) frame variable foo[0]

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

jingham added a comment. We do also handle [], though it isn't obvious to me after a quick glance where that gets done. This is a little cheesy because the name of the child that we are finding with [0] is actually "[0]", so you just have to be careful not to consume that when you consume the

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

jingham added a comment. A dotest test can be xfailed if the debug format is not PDB, right? At least we can xfail them for all the DWARF variants so it should be possible to do that for PDB as well. So you should be able to write a test for this and then just xfail it till the DWARF parser c

[Lldb-commits] [PATCH] D54460: Don't keep a global ABI plugin per architecture

jingham created this revision. jingham added reviewers: clayborg, jasonmolenda. Herald added subscribers: lldb-commits, abidh, atanasyan, kbarton, javed.absar, nemanjai. For reasons that are unclear to me, when the ABIXXX::CreateInstance function is called to make a new ABI for a given process a

[Lldb-commits] [PATCH] D54460: Don't keep a global ABI plugin per architecture

This revision was automatically updated to reflect the committed changes. Closed by commit rL346775: Since ABI's now hold a process WP, they should be handed (authored by jingham, committed by ). Herald added subscribers: llvm-commits, jrtc27. Changed prior to commit: https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D54460: Don't keep a global ABI plugin per architecture

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB346775: Since ABI's now hold a process WP, they should be handed (authored by jingham, committed by ). Repository: rLLDB LLDB https://reviews.llvm.org/D54460 Files: source/Plugins/ABI/MacOSX-arm/

[Lldb-commits] [PATCH] D54460: Don't keep a global ABI plugin per architecture

jingham added a comment. Okay, I think I'll keep what Jason did and just make them per-process. As Greg said, they are very cheap to make, since they just take a Process SP and get the weak pointer from it. None of them have any other state. Repository: rLLDB LLDB https://reviews.llvm.org

[Lldb-commits] [PATCH] D54460: Don't keep a global ABI plugin per architecture

jingham added a comment. As for testing, Jason was careful to use a WP, so nothing crashes. The process's SP doesn't resolve and then you get the default setting of any process specific setting that uses the ABI. Since there aren't any of those yet - this currently has no observable consequen

[Lldb-commits] [PATCH] D54444: [CMake] Use extended llvm_codesign to pass entitlements for lldb-server

jingham added a comment. We do use lldb-server, though only in the platform mode, for remote testsuite running. The gdbserver mode doesn't do anything for a macOS hosted lldb-server, so lldb-server should not require any entitlements. But we do use it. https://reviews.llvm.org/D5 ___

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. The lldb API's parameters are ordered input first than output. Pretty much all the API's that take a Status as a parameter take it as the last parameter. So it looks weird to hav

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D54221 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

jingham added a comment. I didn't read the patch in detail yet, these are just meta-comments: It looks like the libOption approach (or this implementation of it) is missing the notion of option groups. That's what you see in the first section of the lldb --help printout in the current version.

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

jingham added a comment. You have to gather all the -O's and -S's and -Q's and add them to the list of code that gets sourced before the file is loaded in the order in which you find them. There can be more than one of each of these and they can be interspersed anywhere among the other command

[Lldb-commits] [PATCH] D43886: [lldb] Add GetCurrentException APIs to SBThread, add frame recognizer for objc_exception_throw for Obj-C runtimes

jingham added a comment. The exception breakpoints know that libobjc.B.dylib`objc_exception_throw is where you stop for exception breakpoints, and now the ObjCExceptionRecognizer knows the same thing. It always makes me nervous when two different places have the same hard-coded string. Can we

[Lldb-commits] [PATCH] D43886: [lldb] Add GetCurrentException APIs to SBThread, add frame recognizer for objc_exception_throw for Obj-C runtimes

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This isn't an external API, so we can generalize it when we get around to extending the Recognizers to C++ exceptions. This is fine for now. Thanks for doing this! CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D54843: [Expr] Check the language before ignoring Objective C keywords

jingham accepted this revision. jingham added a comment. I think the ideal behavior would be "if we don't have debug info for the frame, choose ObjC++". But if you are in frame with debug info, obey that frame's language (except that we have to use C++ because the expression parser uses C++ re

[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This is a little bit odd, but it does make it easy to call methods on a value you got from FindVariable without having to cons up an expression. That seems worthwhile. It would b

[Lldb-commits] [PATCH] D44072: [lldb] Retrieve currently handled Obj-C exception via __cxa_current_exception_type and add GetCurrentExceptionBacktrace SB ABI

jingham added a comment. This looks good. Can you add two more tests: 1. Can you add a step to your tests that shows fetching the current exception object from a thread that doesn't have an exception does the right thing (i.e. nothing). 2. Since C++ and ObjC exceptions use the same mechanism c

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jingham created this revision. jingham added reviewers: jasonmolenda, aprantl. Herald added subscribers: lldb-commits, abidh. The test "TestExec.py" has been on and off flakey on the GreenDragon MacOS bots for a while now. I'm trying to fix that. It adds a lot of noise to the bots. For backgro

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jingham updated this revision to Diff 177085. jingham marked an inline comment as done. jingham added a comment. Added a comment and removed an errant space. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55399/new/ https://reviews.llvm.org/D55399 Files: sour

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jingham marked 2 inline comments as done. jingham added inline comments. Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:83 + m_break_id(LLDB_INVALID_BREAK_ID), m_mutex() + , m_maybe_image_infos_address(LLDB_INVALID_ADDRESS) {}

[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

jingham added inline comments. Comment at: include/lldb/API/SBValue.h:310-312 + lldb::SBValue EvaluateExpression(const char *expr) const; + lldb::SBValue EvaluateExpression(const char *expr, + const SBExpressionOptions &options) const;

[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

jingham added inline comments. Comment at: source/API/SBValue.cpp:1367-1371 + if (log) +log->Printf("SBValue(%p)::EvaluateExpression (expr=\"%s\") => SBValue(%p) " +"(execution result=%d)", +static_cast(value_sp.get()), expr, +

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jingham marked an inline comment as done. jingham added inline comments. Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:104 +if (image_infos_address != m_maybe_image_infos_address) + did_exec = true; + } jasonmol

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jingham marked an inline comment as done. jingham added inline comments. Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:104 +if (image_infos_address != m_maybe_image_infos_address) + did_exec = true; + } jingham

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB348559: Handle detecting exec for DynamicLoaderMacOS with older debugservers (authored by jingham, committed by ). Changed prior to commit: https://reviews.llvm.org/D55399?vs=177085&id=177087#toc Re

[Lldb-commits] [PATCH] D55356: Add a method to get the "base" file address of an object file

jingham added a comment. In D55356#1327280 , @clayborg wrote: > In D55356#1327242 , @labath wrote: > > > In D55356#1327224 , @clayborg > > wrote: > > > > > In D55356#1327099

[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

jingham added a comment. This change looks good to me too. I don't think this will fix the parameter/local variable lookup inversion that we've been hacking around. The problem comes because to make an JITted object we can easily call we make a wrapper function with a simple argument list - th

[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

jingham created this revision. jingham added a reviewer: clayborg. Herald added subscribers: lldb-commits, abidh. Because of the way we use weak pointers in Process, it is not safe to just destroy a fully live Process object. For instance, because the Thread holds a ProcessWP, if the Process ge

[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

jingham added a comment. Yeah, I'll simplify the logic after this change. I've had mysterious crashes that I've never been able to reproduce for quite some time now. Things like we go to tear down a process, the thread is deleting its thread plans, a thread plan goes to remove one of it's brea

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. The frame regex will get confused if you had a binary called "foo 0x1" Then we would treat "foo" as the binary name, 0x1 as the pc, and the rest would be the function name. I don

[Lldb-commits] [PATCH] D55559: Remove the Disassembly benchmarks.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9/new/ https://reviews.llvm.org/D9 ___ lldb-commits mailing list lldb-commi

[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

jingham updated this revision to Diff 178306. jingham added a comment. This is better. Instead of just finalizing, I call DeleteCurrentProcess. All the plugins that implement DebugProcess actually call DeleteCurrentProcess, so this just makes sure it happens before we drop our (potentially las

[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

This revision was automatically updated to reflect the committed changes. Closed by commit rL349435: Call DeleteCurrentProcess before we replace the old process. (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D556

  1   2   3   4   5   6   7   8   9   10   >