[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:651-652 return; + if (name_unique_cstr[0] == '_' && name_unique_cstr[1] == '$') +

[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

clayborg added a comment. Looks good. Repository: rL LLVM https://reviews.llvm.org/D38153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D38323: Enable breakpoints and read/write GPRs for ppc64le

clayborg added inline comments. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h:71 + RegInfo m_reg_info; + Reg m_gpr_ppc64le[48]; // 64-bit general purpose registers. + Could we share the structure that backs all registers from the

[Lldb-commits] [PATCH] D38323: Enable breakpoints and read/write GPRs for ppc64le

clayborg added a comment. Looks fine. One main questions for new linux archs in particular: is linux using the lldb-server to debug these days even when debugging locally? If so, then this patch only needs to implement both a native register content and not the lldb_private::RegisterContext sub

[Lldb-commits] [PATCH] D38394: Fix dumping of characters with non-standard sizes

clayborg added a comment. Looks good. Would be nice to add support for byte sizes of 3, 5 and 7 to the unchecked version as noted in inline comments, or remove the function if no one is using this function. Just a few quick fixes and this will be good to go. Comment at: sourc

[Lldb-commits] [PATCH] D38568: [lldb] Enable using out-of-tree dwps

clayborg added inline comments. Comment at: source/Host/common/Symbols.cpp:288-294 + // FIXME: at the moment llvm-dwp doesn't output build ids, + // nor does binutils dwp. Thus in the case of DWPs + // we skip uuids check. This needs to be f

[Lldb-commits] [PATCH] D38323: Enable breakpoints and read/write GPRs for ppc64le

clayborg added a comment. Sorry for the delay, I was out on vacation for a week. Repository: rL LLVM https://reviews.llvm.org/D38323 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com

[Lldb-commits] [PATCH] D38568: [lldb] Enable using out-of-tree dwps

clayborg added a comment. (sorry for the delay, I was out on vacation for a week) Repository: rL LLVM https://reviews.llvm.org/D38568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-co

[Lldb-commits] [PATCH] D38568: [lldb] Enable using out-of-tree dwps

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. Repository: rL LLVM https://reviews.llvm.org/D38568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llv

[Lldb-commits] [PATCH] D38394: Fix dumping of characters with non-standard sizes

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

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg added a comment. I agree with all of Zach's suggestions. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h:98-99 +uint32_t refcount; // Serves as enable/disable and reference counter. +long slot; // Saves the value

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg added a comment. Feel free to add me as a reviewer. Zach as well. https://reviews.llvm.org/D38897 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

clayborg added a comment. Who is holding the lock and then fork'ing? Seems like we should fix that? I thought all log calls should be "lock, log, unlock". Why is this causing problems? https://reviews.llvm.org/D38938 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

clayborg added a comment. Never mind, re-reading your original comment made it clear. https://reviews.llvm.org/D38938 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

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

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg added inline comments. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:284 + switch (watch_flags) { + case write_mode: +rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE; We should use the lldb::WatchpointKind from lldb-enume

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

clayborg added a comment. Please do convert to python. Just know that you can use "lldb -P" to get the python path that is needed in order to do "import lldb" in the python script. So you can try doing a "import lldb", and if that fails, catch the exception, run "lldb -P", add that path to the

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

clayborg added a comment. If you want to run the script from the command line, then it is necessary. If it is run from within LLDB it will just work. I like to have my LLDB python scripts work both ways. This might be better implemented as a new command that gets installed and can be used with

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg added inline comments. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:284 + switch (watch_flags) { + case write_mode: +rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE; anajuliapc wrote: > clayborg wrote: > > We should use t

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks fine. Thanks for doing the changes. https://reviews.llvm.org/D38897 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://li

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg added a comment. Yeah, I meant to suggest to remove that. Remove and submit as needed. Thanks again. https://reviews.llvm.org/D38897 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/l

[Lldb-commits] [PATCH] D39199: [lldbtests] Handle errors instead of crashing

clayborg added inline comments. Comment at: packages/Python/lldbsuite/test/dotest.py:52 def is_exe(fpath): +"""Returns true if fpath is an executable. We could add a default parameter here like: ``` def is_exe(fpath, fatal=False): if fatal and not os.pa

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

clayborg accepted this revision. clayborg added a comment. Looks fine. We can iterate on this. https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

clayborg added inline comments. Comment at: include/lldb/Target/Target.h:1217 +const ArchSpec &GetSpec() const { return m_spec; } +Architecture *GetPlugin() const { return m_plugin_up.get(); } + zturner wrote: > Can this return a reference instead of a po

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

clayborg added inline comments. Comment at: source/Core/PluginManager.cpp:282 + ConstString name; + std::string description; + PluginManager::ArchitectureCreateInstance create_callback; zturner wrote: > clayborg wrote: > > We need "std::string" since it owns t

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

clayborg added inline comments. Comment at: source/Plugins/Architecture/Arm/ArchitectureArm.cpp:23 + return ConstString("arm"); +} + zturner wrote: > clayborg wrote: > > zturner wrote: > > > clayborg wrote: > > > > One time at startup. No threads contending yet.

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added a comment. Looks good. A little bit of cleanup. Let me know what you think of the comments. Comment at: utils/clangdiag.py:17 +import os +from subprocess import check_output as qx; + I would rather just do: ``` import subprocess ``` Then later

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added inline comments. Comment at: utils/clangdiag.py:62 + +def enable(debugger, args): +# Always disable existing breakpoints Pass exe_ctx or target into this instead of the debugger (see Jim's comment on execution contexts below. ===

[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

clayborg added a comment. It is a nice idea. I would still rather fix this in clang so it doesn't ask us about a variable it already knows about. Though this might be a good solution until we can fix it for real though. Sean or Jim? https://reviews.llvm.org/D39307 __

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Wow. I didn't realize windows support didn't do any of this. Looks good. Zach will want to ok this as he is one of the main windows contributors. https://reviews.llvm.org/D39314 __

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:651-659 + ArchSpec header_arch; + GetArchitecture(header_arch); +

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close. Thanks for making the changes. Just a few nits. Comment at: utils/clangdiag.py:66 +def setDiagBreakpoint(frame, bp_loc, dict): +id = frame.FindV

[Lldb-commits] [PATCH] D19603: Fix entry point lookup for ObjectFilePECOFF

clayborg added a comment. Does this need some ARM support where we strip bit zero in case the entry point is Thumb? https://reviews.llvm.org/D19603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. ok, just move the ARM detection out of the loop and use a THUMB_ADDRESS_BIT_MASK for ARM symbols and this is good to go. https://reviews.llvm.org/D39315 _

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

clayborg added a comment. Testing would include a test that dynamically loads a shared library and sets a breakpoint it in. We have these tests, but I am guessing they are disabled on windows probably because they might use "dlopen(...)" which is probably not available on windows? I know we hav

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg accepted this revision. clayborg added inline comments. Comment at: utils/clangdiag.py:117 +disable(exe_ctx) +else: +getDiagtool(exe_ctx.GetTarget(), args.path[0]) hintonda wrote: > clayborg wrote: > > should probably verify that this

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

clayborg added a comment. The test suite can be run remotely if ds2 supports the "lldb-server platform" packets. I'll be happy to help you get this going Stephane. Ping me internally if interested. https://reviews.llvm.org/D39315 ___ lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added a comment. Each lldb.SBValue has accessors for the stuff in an execution context: `` lldb::SBTarget GetTarget(); lldb::SBProcess GetProcess(); lldb::SBThread GetThread(); lldb::SBFrame GetFrame(); You could keep a global map of process ID to diagtool if you want? W

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. A few general things: don't modify FileSpec, we have many of these objects and we can't increase their size without directly affecting memory usage. FileSpec objects represent on

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

clayborg added a comment. I was unhappy when we went over two pointers for a FileSpec when m_syntax was added due to the extra size. Anything we can do to make this smaller would be great, so the type on the enum would work, but as you say the alignment will nullify that. The two ConstString me

[Lldb-commits] [PATCH] D39487: Add float/vector registers for ppc64le

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

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

clayborg added a comment. In https://reviews.llvm.org/D39436#912828, @zturner wrote: > In https://reviews.llvm.org/D39436#912810, @clayborg wrote: > > > I was unhappy when we went over two pointers for a FileSpec when m_syntax > > was added due to the extra size. Anything we can do to make this

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

clayborg added a comment. The main reason for two strings is for searching efficiency. Most people don't set breakpoints using full paths, they give the basename: (lldb) b main.c:12 When setting breakpoints is it very easy to search for matches by basename since this is what users usually ty

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

clayborg added a comment. And yes, the memory savings are quite large as well when sharing directories. https://reviews.llvm.org/D39436 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com

[Lldb-commits] [PATCH] D39515: Remove TestMyFirstWatchpoint and TestStepOverWatchpoint from basic_process category

clayborg added a comment. No objections from me as I don't use these categories. https://reviews.llvm.org/D39515 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase

clayborg added a comment. Another option would be to add a URL version of these functions: class SBDebugger { void SetInputURL(const char *url); void SetOutputURL(const char *url); void SetErrorURL(const char *url); }; Then we allow this to trickle down to the IOBase in that way.

[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase

clayborg added a comment. In https://reviews.llvm.org/D38829#914300, @zturner wrote: > Ok, I wasn't aware of the libedit problem. > > I still don't like this approach, because it causes the design of the `File` > class to be influenced by a limitation of a 3rd party library. > > What about addin

[Lldb-commits] [PATCH] D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit

clayborg added a comment. So I like the ability to specify any symbol context to parse all types for. If you pass in a symbol context with only a module filled in, we parse all types. If we pass in a symbol context with only the compile unit filled in (no function), we parse all types in the co

[Lldb-commits] [PATCH] D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit

clayborg added a comment. So there really aren't that many things: - module only filled out, parse all types - compile unit only filled out, parse all type from a compile unit - function filled out, parse all types in a function - block filled out, parse all types in block, or we can skip this on

[Lldb-commits] [PATCH] D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit

clayborg added a comment. But if everyone else thinks differently, I'll go with the majority. Just my initial thoughts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56462/new/ https://reviews.llvm.org/D56462 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. DW_AT_comp_dir isn't enough. See inline suggestions. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:775-787 +void DWARFUnit::ComputePathStyle() { +

lldb-commits@lists.llvm.org

clayborg added a comment. All the change to the symbol vendor make sense, but it seems like all of the call sites should be: cu->GetLanguage(); cu->ParseFunctions(); cu->GetLineTable(); cu->ParseDebugMacros(); cu->GetSupportFiles(); cu->ParseTypes(); Some of these calls might alread

lldb-commits@lists.llvm.org

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. This is fine to start. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56564/new/ https://reviews.llvm.org/D56564 ___ lldb-commits mai

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

clayborg added inline comments. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4707 +else + // The register info is incorrect, just clear it. + m_register_info.Clear(); Is this a GDB server that you can modify? Or is

[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

clayborg added inline comments. Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:858 +Status ProcessWindows::GetFileLoadAddress(const FileSpec &file, bool &is_loaded, + lldb::addr_t &load_addr) { asm

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:769-774 +FileSpec::Style DWARFUnit::GetPathStyle() { + if (!m_comp_dir) +ComputeCompDirAndGuessPathStyle(); + return m_comp_dir->GetPathStyle(); +} + Remove? ===

[Lldb-commits] [PATCH] D56590: breakpad: Add FUNC records to the symtab

clayborg added inline comments. Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp:29-48 +Token breakpad::toToken(llvm::StringRef str) { return llvm::StringSwitch(str) .Case("MODULE", Token::Module) .Case("INFO", Token::Info) .Case("FILE

[Lldb-commits] [PATCH] D56590: breakpad: Add FUNC records to the symtab

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:232 + }; + for (llvm::StringRef line: lines(*m_obj_file, Token::Func)) { +// Here we can get either FUNC records (starting with FUNC), or line records Sho

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

clayborg added a comment. So LLDB treats compile units special depending on the inline strategy. Because of this, I outlined some examples of how and why we should create a compile unit per "FUNC" token. Let me know if anything above was unclear Comment at: source/Plugins/Sym

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

clayborg accepted this revision. clayborg added a comment. Looks good to me CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56688/new/ https://reviews.llvm.org/D56688 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

clayborg added a comment. Might be good to have a test where we have a relative DW_AT_name and not DW_AT_comp_dir? Just in case? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56543/new/ https://reviews.llvm.org/D56543 ___ lldb-commits mail

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Ok, thanks for explaining. Your reasoning makes sense. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56543/new/ https://reviews.llvm.org/D56543

[Lldb-commits] [PATCH] D55724: [ARC] Add SystemV ABI

clayborg added inline comments. Comment at: source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp:43 + +namespace dwarf { +enum regnums { add "namespace {" around all of this Comment at: source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp:52 +}; +} // namespace

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

clayborg added inline comments. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4707 if (comm.GetQXferLibrariesSVR4ReadSupported()) { list.clear(); My two cents: I fully support a stub being able to provide the register info for DWAR

[Lldb-commits] [PATCH] D56844: Breakpad: Extract parsing code into a separate file

clayborg added a comment. Are we missing parsing of FUNC and line entry lines in this patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56844/new/ https://reviews.llvm.org/D56844 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D56844: Breakpad: Extract parsing code into a separate file

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Never mind, just read the update to your other patch. Would be nice to identify line records in toToken, but that will probably cost more CPU cycles that it is worth, so I think this is a

[Lldb-commits] [PATCH] D56590: breakpad: Add FUNC records to the symtab

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Much cleaner! Thanks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56590/new/ https://reviews.llvm.org/D56590 ___ lldb-commits maili

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: include/lldb/Core/Architecture.h:118-127 + static constexpr uint8_t max_features_count = 32u; + virtual std::bitset GetFeatures() const { retu

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: include/lldb/Core/Architecture.h:119-123 + virtual std::vector + ConfigurationRegisterNames() const { return {}; } + + using ConfigurationReg

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

clayborg added a comment. Let me know what you think of the UUID code and if you want to include that in this patch Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:67 +/// to the endian-specific integer N. Return true on success. +template static bool consu

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

clayborg added inline comments. Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:87-97 // The textual module id encoding should be between 33 and 40 bytes long, // depending on the size of the age field, which is of variable length. // The first three c

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

clayborg added inline comments. Comment at: source/Plugins/Architecture/Arc/ArchitectureArc.cpp:1-8 +//===-- ArchitectureArc.cpp -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed und

[Lldb-commits] [PATCH] D57273: Make Type::GetByteSize optional

clayborg added inline comments. Comment at: include/lldb/Symbol/Type.h:221 + uint64_t m_byte_size : 63; + unsigned m_byte_size_has_value : 1; Declaration m_decl; Does this need to be uint64_t to share the same space as m_byte_size? CHANGES SINCE LAST ACTIO

[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

clayborg added a comment. In D56237#1373234 , @labath wrote: > I think there are some architectural issues here in how ProcessWindows works. > Normally (i.e., on all non-windows process classes), it is the job of the > dynamic loader plugin to create mod

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

clayborg added inline comments. Comment at: source/Target/PathMappingList.cpp:223 + + if (prefix_ref == ".") { +prefix_is_relative = true; We are we finding a "." in any path now? I thought we normalized those all out? Can a PathMappingList co

[Lldb-commits] [PATCH] D51578: Contiguous .debug_info+.debug_types for D32167

clayborg added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:693-707 +DataBufferHeap *databufferheap = new DataBufferHeap(); +DataBufferSP databuffer = DataBufferSP(databufferheap); +databufferheap->AppendData( +debug_in

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

clayborg added a comment. I like the way you did the compile units and the line tables and support file list. It would be nice to change this to do things more lazily. Right now we are parsing all compile unit data into CompUnitData structures and then passing their info along to the lldb_priva

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

clayborg added a comment. Check the comment about m_platform_file Comment at: include/lldb/Core/Module.h:176-177 + +// Also update the module FileSpec. +module_sp->m_file = module_sp->m_objfile_sp->GetFileSpec(); +return std::move(module_sp); Why do

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

clayborg accepted this revision. clayborg added a comment. Thanks for the answers. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57552/new/ https://reviews.llvm.org/D57552 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

clayborg added a comment. Do we have any callers that call GetNumTemplateArguments with false? If not, remove the argument? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51387/new/ https://reviews.llvm.org/D51387 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

clayborg added a comment. Just bounds check "index" in parse compile unit and this is good to go Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:196 + + CompUnitData &data = m_cu_data->GetEntryRef(index).data; + Validate "index" first? We

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

clayborg accepted this revision. clayborg added a comment. In D57751#1390345 , @labath wrote: > Hi Greg, what do you think of my replies to your `CreateModuleFromObjectFile` > comment? Your explanation makes sense. I missed the fact that this function w

[Lldb-commits] [PATCH] D56537: ObjectFilePECOFF: Create a "container" section spanning the entire module image

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1370 -auto section = section_list->GetSectionAtIndex(section_idx); +auto section = section_list->FindSectionByID(section_id); if (!section) How fast is this

[Lldb-commits] [PATCH] D56537: ObjectFilePECOFF: Create a "container" section spanning the entire module image

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1370 -auto section = section_list->GetSectionAtIndex(section_idx); +auto section = section_list-

[Lldb-commits] [PATCH] D58129: Move UnwindTable from ObjectFile to Module

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. You changed the API to return a pointer which can be NULL so we either need to check all returns for this or change the API to return a reference to a default constructed UnwindT

[Lldb-commits] [PATCH] D58131: [lldb] [unittest] Avoid mixing '127.0.0.1' and 'localhost'

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Yeah localhost can cause problems if someone has a custom mapping in their network config file. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58131/new/

[Lldb-commits] [PATCH] D58167: Refactor user/group name resolving code

clayborg added inline comments. Comment at: include/lldb/Host/UserIDResolver.h:24 +public: + typedef uint32_t id_t; + virtual ~UserIDResolver(); // anchor make this 64 bit for future proofing? And if so, just use lldb::user_id_t? Comment at:

[Lldb-commits] [PATCH] D58129: Move UnwindTable from ObjectFile to Module

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just a question of if we need an optional unwind table instance instead of just an instance. I am fine either way. Comment at: include/lldb/Core/Module.h:1108-1110 + ll

[Lldb-commits] [PATCH] D58222: [ClangExpressionParser] Reuse the FileManager from the compiler instance.

clayborg added a comment. This change was probably due to things changing over time and there may be more of these kinds of cleanups to be had. nice catch. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58222/new/ https://reviews.llvm.org/D58222

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

clayborg added inline comments. Comment at: source/Core/Module.cpp:1451-1454 +// Clear the unwind table too, as that may also be affected by the +// symbol file information. +m_unwind_table.reset(); + Are we sure no one is holding onto inf

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

clayborg created this revision. clayborg added reviewers: labath, abdulras, zturner. Facebook creates minidump files that contain specific information about why things crash. Adding ways to dump these allows tools to be made that can auto download symbols based on the information that is contain

[Lldb-commits] [PATCH] D54670: 03/03: .debug_types: Update of D32167 (.debug_types) on top of D51578 (section concatenation)

clayborg added a comment. I am definitely ready for this to go in ASAP if we are good on this! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54670/new/ https://reviews.llvm.org/D54670 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

clayborg added a comment. BTW: I did add a - character after the CHECK before I checked it in Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58398/new/ https://reviews.llvm.org/D58398 ___ lldb-commits mailing list lldb

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

clayborg added inline comments. Comment at: source/Target/Process.cpp:2638-2639 } - ModuleSP module_sp(new Module(file_spec, ArchSpec())); + ModuleSP module_sp(new Module( + file_spec, GetTarget().GetExecutableModule()->GetArchitecture())); if (module_sp) { -

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

clayborg added inline comments. Comment at: source/Target/Process.cpp:2639 + ModuleSP module_sp(new Module( + file_spec, GetTarget().GetExecutableModule()->GetArchitecture())); if (module_sp) { So if you have a target that is set to "aarch64-linux-androi

[Lldb-commits] [PATCH] D58330: 01/03: new SectionPart for Section subranges (for effective .debug_types concatenation)

clayborg added a comment. So I question the need for the SectionPart class. Seems like it would be easier to just add extra args to the ReadSectionData calls? Comments? Comment at: lldb/include/lldb/Symbol/ObjectFile.h:739 // be larger than what Section::GetFileSize reports

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

clayborg accepted this revision. clayborg added a comment. Looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58405/new/ https://reviews.llvm.org/D58405 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I seem to remember that we currently expect that there are two cases when it comes to unknowns: - specified unknowns - unspecific unknowns A "specified unknown" is when the user

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

clayborg created this revision. clayborg added a reviewer: jingham. Herald added a reviewer: serge-sans-paille. Herald added a project: LLDB. Currently when we single step over a source line, we run and stop at every branch in the source line range. We can reduce the number of times we stop when

<    4   5   6   7   8   9   10   11   12   13   >