[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

clayborg added a comment. Another idea for the SymbolServers: be able to specify a source repository (git, svn etc) and hash or revision ID. The symbol server can grab the source from the repo and cache is locally for display. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D76964: Fix an issue where the IgnoreName function was not allowing "Class" to be looked up inside a namespace or other class.

clayborg updated this revision to Diff 254352. clayborg added a comment. Fixed patch reviewer suggestions and added a test for "id". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76964/new/ https://reviews.llvm.org/D76964 Files: lldb/source/Plug

[Lldb-commits] [PATCH] D77295: [lldb/Core] Fix a race in the Communication class

clayborg added inline comments. Comment at: lldb/source/Core/Communication.cpp:318 bool done = false; + bool disconnect = false; while (!done && comm->m_read_thread_enabled) { Can we just init "disconnect" right here?: ``` const bool disconnect = comm->Ge

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

clayborg created this revision. clayborg added reviewers: labath, aadsm, wallace, JDevlieghere. Herald added a project: LLDB. wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. I agree with the implementation. I also find no way notify the IDE

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

clayborg updated this revision to Diff 254670. clayborg added a comment. Fix typos. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77347/new/ https://reviews.llvm.org/D77347 Files: lldb/test/API/tools/lldb-vscode/console/Makefile lldb/test/API/

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

clayborg added a comment. commit 5998aceda9fdca04db4d9ee390cec660896bf0bf (HEAD -> master, origin/master, origin/HEAD) Author: Greg Clayton Date: Thu Apr 2 16:30:33 2020 -0700 Have lldb-vscode update the currently select

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

clayborg added a comment. Does anyone know why harbormaster is useless? Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2014 -if (!method_die_offsets.empty()) { - DWARFDebugInfo &debug_info = dwarf->DebugInfo(); kwk w

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

clayborg added a comment. LGTM. Pavel? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

clayborg added a comment. I was thinking if auto repeat was enabled that the DoExecute would just get called again with nothing and then the plug-in could maintain any repeat commands without needing to add anything... Glad you chimed in! Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

clayborg added a comment. The biggest issue is maintaining the API. We can't add anything to SBCommandPluginInterface Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77444/new/ https://reviews.llvm.org/D77444 _

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

clayborg added a comment. In D77444#1963354 , @labath wrote: > In D77444#1962952 , @clayborg wrote: > > > The biggest issue is maintaining the API. We can't add anything to > > SBCommandPluginInterface > > > If we'

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

clayborg added a comment. I am not a pro at the gtest stuff, but this was very hard to follow and figure out what these tests are doing. Comment at: lldb/source/Target/Memory.cpp:23-24 // MemoryCache constructor -MemoryCache::MemoryCache(Process &process) +MemoryCache::Memory

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

clayborg added a comment. Mostly good, but it seems weird to supply the cache line size when calling the MemoryCache::Clear() function. We also don't seem to be catching updates to the cache line size property and invalidating the cache when and only when the property is modified, but we seem t

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

clayborg added a comment. In D77790#1974047 , @jarin wrote: > Regarding the callback idea, I have bad experience with callbacks because > they break if the code is not crafted for re-entrancy and they are much > harder to understand. That change feels ou

[Lldb-commits] [PATCH] D78077: Fix bug in UnwindAssemblyInstEmulation with fp-using codegen and mid-function epilogues

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. If you test worked, then there is something wrong with this test? See inline comment for copy and paste error Comment at: lldb/source/Plugins/UnwindAssembly/I

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I see plenty of examples daily, from clang version 7, where this happens. The first binary I tried to verify after seeing this patch, I found this error. Dump is below. This comp

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. And also it is worth noting that just because new compilers might generate this information correctly, it doesn't mean LLDB won't be used to try and load older debug info from compilers that don't. LTO can also greatly affect the reliability of this information. And ma

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. And this one binary had 115 examples of this: $ llvm-dwarfdump --verify libIGL.so | grep "DIE address ranges are not contained in its parent" | wc -l 115 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78489/new/ h

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. So if llvm is relying on this, we should change llvm. Symbolication will fail using LLVM's DWARF parser if it is using this information for anything real. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78489/new/ https://r

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. Sorry I might have not understood this patch's actual implementation. I thought we were switching to trusting DW_AT_ranges, which seems we are already doing. Let me read the patch code a bit more carefully, not just quickly reading the description... Repository: rG

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. So it looks like there are bugs in the "llvm-dwarfdump --verify". The blurb I posted above clearly has the function's address range contained in it, sorry for the false alarm. I have been quite a few problems with DWARF with LTO and other post production linkers. The l

[Lldb-commits] [PATCH] D78588: [lldb/Core] Check that ArchSpec is valid.

clayborg added a comment. None of this is exposed through SBPlatform right? No way to test this? Comment at: lldb/source/Target/Platform.cpp:1825-1826 ArchSpec arch = target.GetArchitecture(); + if (!arch.IsValid()) +return 0; + would be nice to log som

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. Sounds like a win then, as long as we don't get slower I am fine with this. I was guessing that line tables might be faster because it is generally very compressed and compact compared to debug info, but thanks for verifying. Might be worth checking the memory consumpt

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. In D78489#1996834 , @labath wrote: > In D78489#1995837 , @clayborg wrote: > > > Sounds like a win then, as

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. So a few things here. It doesn't seem like it is necessary to create the WasmProcessGDBRemote and IWasmProcess. It would be fine to extend the current ProcessGDBRemote and ThreadGDBRemote classes. The whole reason seems to be that the variables (globals, locals, etc) a

[Lldb-commits] [PATCH] D78839: [lldb-vscode] Add an option for loading core files

clayborg added a comment. You might look around for a core file in another test location and be able to use that. Functionality looks good to me. This could have been done with attachCommands before, but it is nice to have a built in and supported way to do this. Repository: rG LLVM Github

[Lldb-commits] [PATCH] D78825: [lldb/Driver] Exit with a non-zero exit code in batch mode when stopping because of an error.

clayborg added a comment. In D78825#2002598 , @vsk wrote: > Can we delete an override of SBDebugger::RunCommandInterpreter, or are they > all part of the stable API? If we can, it'd be nice to get rid of the one > with 6 args. We can't remove any exist

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. In D78801#2004501 , @paolosev wrote: > I am adding all the pieces to this patch to make the whole picture clearer; I > thought to add a piece at the time to simplify reviews, but probably it ended > up making things more obscure

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. In D78801#2007795 , @labath wrote: > In D78801#2007083 , @clayborg wrote: > > > It would be fine to ask the lldb_private::Process class to evaluate any > > unknown DWARF expression opcodes

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. Thanks for the explanations if everything WASM related. I now understand much better what you have. > Read Wasm memory. > IN : $qWasmMem:frame_index;addr;len > OUT: $xx..xx frame index seems weird in a memory read packet. Seems like the module ID should be passed in

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. Could we just always use memory reading and have the address contain more info? Right now you have the top 32 bits for the module ID. Could it be something like: struct WasmAddress { uint64_t module_id:16; uint64_t space:4; // 0 == code, 1 == data, 2 == globa

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. In D78801#2009917 , @labath wrote: > In D78801#2009048 , @clayborg wrote: > > > In D78801#2007795 , @labath wrote: > > > > > While that idea has occ

[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

clayborg added a comment. Sounds like we have an approach to try! I would like to see solutions that are not WASM specific when possible. One other way of doing the memory read/write with segments: maybe the "m" and "M" packets can be overloaded include the memory segment identifier if the rem

[Lldb-commits] [PATCH] D79120: [lldb/API] Add RunCommandInterpreter overload that returns SBCommandInterpreterRunResults (NFC)

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Sorry, missed the fact that the new class had ivars. For ABI stability, we can't modify the number of ivars since we need the class size of SBCommandInterpreterRunResults to neve

[Lldb-commits] [PATCH] D79120: [lldb/API] Add RunCommandInterpreter overload that returns SBCommandInterpreterRunResults (NFC)

clayborg added inline comments. Comment at: lldb/include/lldb/API/SBCommandInterpreterRunOptions.h:93 + bool m_stopped_for_crash; +}; + like how SBCommandInterpreterRunOptions does it above with its ivar "m_opaque_up" that never changes size Repository: r

[Lldb-commits] [PATCH] D79120: [lldb/API] Add RunCommandInterpreter overload that returns SBCommandInterpreterRunResults (NFC)

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Yeah, anytime you see API calls that have many arguments it is a good idea to switch to this kind of API as adding accessors doesn't require us to change the API. We did this with SBTarget

[Lldb-commits] [PATCH] D79108: [lldb/CommandInterpreter] Move AutoHandleEvents and SpawnThread into CommandInterpreterRunOptions (NFC)

clayborg added inline comments. Comment at: lldb/source/API/SBDebugger.cpp:1190 +options.SetAutoHandleEvents(auto_handle_events); +options.SetSpawnThread(spawn_thread); CommandInterpreter &interp = m_opaque_sp->GetCommandInterpreter(); I agree this i

[Lldb-commits] [PATCH] D79209: [lldb/CommandInterpreter] Add CommandInterpreterRunResult (NFC)

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: lldb/include/lldb/lldb-enumerations.h:1084 }; + +enum CommandInterpreterResult { Add some header doc for these to explain a bit maybe si

[Lldb-commits] [PATCH] D70885: [lldb] Use explicit lldb commands on tests

clayborg added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py:353-354 'target create "%s"' % (program), -'br s -f main.c -l %d' % first_line, -'br s -f main.c -l %d' % second_lin

[Lldb-commits] [PATCH] D78825: [lldb/Driver] Exit with a non-zero exit code in batch mode when stopping because of an error.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM now. Pavel? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78825/new/ https://reviews.llvm.org/D78825 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

clayborg added a comment. Interesting approach to DWARF expression evaluation, though it might be simpler to leave the DWARFExpression as it is, and have the plug-in part only handle unknown opcodes. Right now if you want to add just one opcode, you must subclass and make a plug-in instance whe

[Lldb-commits] [PATCH] D79614: Fix error reporting for qLaunchSuccess, check for fix/enable it via qSupported

clayborg added a comment. Why not just fix qLaunchSuccess by passing the string through a "escape_string(...)" function? Any packet that can return a string with unknown content should be escaped correctly. Adding a new packet doesn't really fix the problem in older debugserver/lldb-server bina

[Lldb-commits] [PATCH] D79614: Fix error reporting for qLaunchSuccess, check for fix/enable it via qSupported

clayborg added a comment. In D79614#2027869 , @jasonmolenda wrote: > In D79614#2027861 , @clayborg wrote: > > > Why not just fix qLaunchSuccess by passing the string through a > > "escape_string(...)" function? An

[Lldb-commits] [PATCH] D79614: Fix error reporting for qLaunchSuccess, check for fix/enable it via qSupported

clayborg added a comment. In D79614#2030692 , @jasonmolenda wrote: > Thanks for the feedback. > > In D79614#2029157 , @labath wrote: > > > I think that "piggy-backing" on the `qSupported` packet for communicating

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

clayborg added a comment. The error where two of the same classes conflicted usually only happened in complex cases that were hard to reduce down. If I understand this correctly, the compiler should be able to auto synthesize these functions at will since the original class definition should h

[Lldb-commits] [PATCH] D79726: Add terminateCommands to lldb-vscode protocol

clayborg added a comment. might be nice to have "disconnectCommands" at some point if we ever need them too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79726/new/ https://reviews.llvm.org/D79726 __

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

clayborg added a comment. In D79811#2035078 , @shafik wrote: > In D79811#2034842 , @clayborg wrote: > > > The error where two of the same classes conflicted usually only happened > > in complex cases that were har

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

clayborg added a comment. nice catch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79757/new/ https://reviews.llvm.org/D79757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.ll

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

clayborg added a comment. There was a patch previous to this that enabled IPv6. We need IPv6 on some computers here at Facebook. Can you check this file's log and look at the IPv6 patch and make sure this doesn't just undo that patch prior to committing this? If it does undo that patch, maybe w

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

clayborg added a comment. In D79757#2036618 , @aadsm wrote: > @clayborg the support we added was for the lldb-server. ok, phew! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79757/new/ https://reviews.llv

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

clayborg added a comment. I really worry enabling this will make the expression parser start to emit errors if we change this. The best fix would be to ensure that the AST importer can correctly ignore implicit functions when comparing types. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D79726: Add terminateCommands to lldb-vscode protocol

clayborg added inline comments. Comment at: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py:447 +] +terminateCommands = ['expr 4+2'] +self.launch(program=program, The expression parser can be quite involved even for simple thin

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

clayborg added a comment. So when talking about "prior to libOption", are we talking getopt()? I believe that getopt would have an issue with: $ lldb inferior --inferior-arg As it would not understand the argument. Or was there an intermediate llvm option parser we were using after getopt()

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

clayborg accepted this revision. clayborg added a comment. Much better! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80165/new/ https://reviews.llvm.org/D80165 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.or

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

clayborg added a comment. This looks good, thanks for subscribing me. We need to have GetNumChildren and GetChildAtIndex agreeing on things and we definitely shouldn't be walking more than on pointer recursively. My only question is do we need helper functions added to TypeSystemClang to avoid

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

clayborg accepted this revision. clayborg added a comment. Major changes not required. Thanks for finding and fixing this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

clayborg added a comment. Only issue now is documentation and why are we requiring addresses to be in compile unit. See inlined comments. Comment at: include/lldb/API/SBTarget.h:667 + lldb::SBBreakpoint BreakpointCreateFromScript( + const char *class_name, + SBStruc

[Lldb-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

clayborg added a comment. We should obey the --tty option here and set CREATE_NEW_CONSOLE if it is set no? Repository: rLLDB LLDB https://reviews.llvm.org/D51966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-

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

clayborg added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:143-151 +static OptionEnumValueElement g_dependents_enumaration[4] = { +{eLoadDependentsDefault, "default", + "Only load dependents when the target is an executables."}, +{eLoadDepende

[Lldb-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

clayborg added a comment. The other option is to only remove this flag if "--no-stdio" is set. The best solution IMHO is: - find a way to get stdio from a process and feed to to LLDB so it shows up when neither --tty nor --no-stdio is set - when --tty is specified, set the CREATE_NEW_CONSOLE bi

[Lldb-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

clayborg added a comment. In https://reviews.llvm.org/D51966#1232057, @xbolva00 wrote: > So basically the hardest problem is to "find a way to get stdio from a > process and feed to to LLDB so it shows up when neither --tty nor --no-stdio > is set" .. yes. The easier solution is to just creat

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg requested changes to this revision. clayborg added a comment. Patch looks good. A few fixes mentioned in inlined comments. And I am unsure how the test suite will run with various compilers if they don't support the -gdwarf-5 flag. We might need to run obj2yaml on a binary and then yaml

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg added a comment. In https://reviews.llvm.org/D51935#1232203, @grimar wrote: > In https://reviews.llvm.org/D51935#1232195, @clayborg wrote: > > > Patch looks good. A few fixes mentioned in inlined comments. And I am > > unsure how the test suite will run with various compilers if they do

[Lldb-commits] [PATCH] D51730: [DWARFExpression] Read literars as unsigned values.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. The test really should encode actual DWARF that contains a DW_OP_litXXX opcode (using obj2yaml/yaml2obj or llvm-mc) as compiling the code for the test won't always produce the needed DWARF

[Lldb-commits] [PATCH] D51578: DWARFConcatenatingDataExtractor for D32167

clayborg added a comment. Overall I am ok with minimal regression in speed if a few percent is all that it is costing us. I am generally ok with this patch. A few questions below. Is there a reason we need DWARFConcatenatingDataExtractor? Can we just put all functionality into DWARFDataExtracto

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for doing all requested changes! Looks great. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp:442 +ReadDescriptors(debug_line_data, offset_p

[Lldb-commits] [PATCH] D52111: Make eSearchDepthFunction work for scripted resolvers

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Very nice. You got it right. Repository: rLLDB LLDB https://reviews.llvm.org/D52111 ___ lldb-commits mailing list lldb-commits@lists.llvm.

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

clayborg added a comment. See inlined comments. Comment at: www/python-reference.html:324 + +Another use of the Python API's in lldb is to create a custom breakpoint resolver. This + allows you to implement your own logic to search the space

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

clayborg added inline comments. Comment at: www/python-reference.html:325 +Another use of the Python API's in lldb is to create a custom breakpoint resolver. This facility + was added in r51967. + Is that SVN r

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for doing all the updates. Pretty clear and concise now. Repository: rLLDB LLDB https://reviews.llvm.org/D52065 ___ lldb-commits ma

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: include/lldb/Target/StackFrame.h:506 + lldb::ValueObjectSP FindVariable(const char *name); + davide wrote: > Also, I think y

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

clayborg added a comment. I am fine with this, Jim or Jason should ok this too just to be sure https://reviews.llvm.org/D51934 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just a documentation suggestion, but looks good. Comment at: include/lldb/Target/StackFrame.h:508 + /// Attempt to reconstruct the ValueObject for a variable with a give

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

clayborg accepted this revision. clayborg added a comment. Looks good https://reviews.llvm.org/D52247 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a comment. In https://reviews.llvm.org/D50365#1241149, @lemo wrote: > Hi Greg, looking at request_evaluate() I noticed that it will evaluate the > string as a lldb command if prefixed by ` . > > This is a great feature (it allows building REPL consoles on top of DAP), but > I'm c

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

clayborg added a comment. So I would suggest not removing any functions from lldb_private::Target, just change the old ones to call through the arch plug-ins if there is one. Then many changes in this diff just go away and we still get the clean implementation where things are delegated to the

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. much better! Thanks for making the changes Repository: rLLDB LLDB https://reviews.llvm.org/D48623 ___ lldb-commits mailing list lldb-commi

[Lldb-commits] [PATCH] D52403: [LLDB] - Support the single file split DWARF.

clayborg added a comment. So the main questions is: do we need a new section enum called eSectionTypeDWARFDebugInfoDWO? If so, then this patch changes. I think I would prefer adding a new enum. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:68 -if (aut

[Lldb-commits] [PATCH] D52406: Make DIE_IS_BEING_PARSED local to the current thread.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. A little background might help here: The lldb_private::Module lock is used to prevent multiple queries into the DWARF from stomping on each other. Multi-threaded DWARF parsing wa

[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

clayborg added a comment. A little background might help here: The lldb_private::Module lock is used to prevent multiple queries into the DWARF from stomping on each other. Multi-threaded DWARF parsing was primarily added to speed up indexing and the only place it is used. Is that not true? Ind

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:298-306 +static uint32_t GetOSOSymbolFlags() { + // When a mach-o symbol is encoded, the n_type field is encoded in bits + // 23:16, and the n_desc field is encoded in bits 1

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:431 const size_t num_ranges = -die->GetAttributeAddressRanges(dwarf, this, ranges, false); +die->GetAttributeAddressRanges(dwarf, this, ranges, check_hi_lo_pc); if

[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

clayborg added a comment. In https://reviews.llvm.org/D48393#1244426, @JDevlieghere wrote: > Thanks for the information, Greg! > > In https://reviews.llvm.org/D48393#1243588, @clayborg wrote: > > > A little background might help here: The lldb_private::Module lock is used > > to prevent multiple

[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

clayborg added a comment. In https://reviews.llvm.org/D48393#1244649, @labath wrote: > I agree with Greg that it would be best to restrict things such that there is > only one instance of parsing going on at any given time for a single module. > I think this was pretty much the state we reached

[Lldb-commits] [PATCH] D52461: [PDB] Introduce `PDBNameParser`

clayborg added a comment. In https://reviews.llvm.org/D52461#1244813, @labath wrote: > I think you should look at CPlusPlusLanguage::MethodName. It already contains > a parser (in fact, two of them) of c++ names, and I think it should be easy > to extend it to do what you want. I agree with P

[Lldb-commits] [PATCH] D52501: [PDB] Restore the calling convention from PDB

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good as long as all the test suite bots are happy. Repository: rLLDB LLDB https://reviews.llvm.org/D52501 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

clayborg added a comment. In https://reviews.llvm.org/D48393#1245342, @friss wrote: > In https://reviews.llvm.org/D48393#1245049, @clayborg wrote: > > > In https://reviews.llvm.org/D48393#1244649, @labath wrote: > > > > > I agree with Greg that it would be best to restrict things such that > > >

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:425 const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly(); const dw_offset_t cu_offset = GetOffset(); Following from inline comment below, you can abort if this CU is

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

clayborg added a comment. It is the SymbolVendor's job to do the locking. And in many cases it already is. I stopped with inlined comments after a few comments as it would apply to this entire patch. It would be great if we can replace all of the locations where you added lock guards with lldb

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Good stuff! Repository: rLLDB LLDB https://reviews.llvm.org/D52572 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.

[Lldb-commits] [PATCH] D52604: Clean-up usage of OptionDefinition arrays

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: tools/driver/Driver.cpp:71 typedef struct { uint32_t usage_mask; // Used to mark options that can be used together. If (1 tatyana

[Lldb-commits] [PATCH] D52403: [LLDB] - Support the single file split DWARF.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just a few fixes. Looking good. Comment at: include/lldb/lldb-enumerations.h:643-660 + eSectionTypeDWARFDebugAbbrevDwo, eSectionTypeDWARFDebugAddr, eSect

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

clayborg added inline comments. Comment at: source/Symbol/Symtab.cpp:520 + return false; +} + This function is still pretty specific. Any comments on my above inlined comment? https://reviews.llvm.org/D52375 ___

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

clayborg added a comment. We really should be making a lldb-server that works on windows instead of making a native windows process plug-in that only works on windows. That will allow remote debugging to windows machines instead of requiring a local connection. It will also allows debug session

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good then as long as all tests pass on Darwin. https://reviews.llvm.org/D52375 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

[Lldb-commits] [PATCH] D52403: [LLDB] - Support the single file split DWARF.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good! https://reviews.llvm.org/D52403 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Any idea why the module isn't set correctly in certain dwarf expressions? Any variable that is created from debug info that pertains to a module should have its DWARFExpression c

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

clayborg added a comment. See SymbolFileDWARF::ParseVariableDIE. It has code that links up the DW_OP_addr: if (is_static_lifetime) { if (is_external) scope = eValueTypeVariableGlobal; else scope = eValueTypeVariableStatic; if (debug_map_symfile) { // When leavin

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

clayborg added a comment. If this isn't fixing up cases where the global does have an address (not just set to zero), we need to fix the code so it relinks the global address correctly. https://reviews.llvm.org/D52678 ___ lldb-commits mailing list

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Jim: we are already linking the address for the DW_OP_addr using the debug map and no .o files are currently expected to be able to link an unlinked address into a file address as nothing

<    1   2   3   4   5   6   7   8   9   10   >