[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

[Lldb-commits] [PATCH] D52981: [LLDB] - Add basic support for .debug_rnglists section (DWARF5)

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just add a switch statement when handling the encodings and a lldbassert as mentioned in inlined comments and this will be good to go. Comment at: source/Plugi

[Lldb-commits] [PATCH] D52981: [LLDB] - Add basic support for .debug_rnglists section (DWARF5)

clayborg added a comment. For space savings it seems like we would want to support the DW_RLE_base_address and DW_RLE_offset_pair pretty soon. https://reviews.llvm.org/D52981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.ll

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h:51 + uint32_t idx, dw_attr_t &attr, dw_form_t &form, +

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. A few things inlined. Very close. DumpAttribute will need to take a DWARFFormValue in order to dump the value correctly. Comment at: source/Plugins/SymbolFile/

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

clayborg added a comment. Down to just modifying the DWARFFormValue constructor to be able to take a CU only. Looks good. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:241-242 +for (uint32_t i = 0; i < numAttributes; ++i) { + DWARFFormVal

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for making the changes. Looks good! https://reviews.llvm.org/D52689 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://l

[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)

clayborg added a comment. See inlined comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp:150-151 +case DW_RLE_base_address: { + dw_addr_t base = data.GetMaxU64(offset_ptr, addrSize); + rangeList.push_back({DW_RLE_base_address, base, 0}); +

[Lldb-commits] [PATCH] D53297: [lldbsuite] Make the names of test classes unique

clayborg added a comment. It would be great if we can detect this when all of the test files are loaded and emit an error instead of waiting for results to be overwritten Repository: rLLDB LLDB https://reviews.llvm.org/D53297 ___ lldb-commits ma

[Lldb-commits] [PATCH] D53193: [LLDB] - Add support for DW_RLE_start_end entries (.debug_rnglists)

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

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

clayborg added a comment. m_has_children was used to represent what was in the DWARF in the byte that follows the DW_TAG. m_empty_children was used for DIEs that said they had children but actually only contain a single NULL tag. It is fine to not differentiate between the two. =

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

clayborg added a comment. See inlined comment. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:287 + m_has_children : 1; uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE, m_tag : 16; // A copy of the DW_TAG value so we don't

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. Nice cleanup. Repository: rLLDB LLDB https://reviews.llvm.org/D53321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. Since this is duplicated in so many spots, can we make a helper function to do this to ensure no one gets it wrong. We might be able to pass in the "guard" and "stop_locker" as reference variables and modify them in the helper function. Repository: rLLDB LLDB https

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inlined comments Comment at: include/lldb/Symbol/SymbolFile.h:98 + virtual std::recursive_mutex &GetModuleMutex() const; + Might add a c

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. All symbol tables are currently extracted from the object files via ObjectFile::GetSymtab(). Are symbols only in the PDB file? If so I would vote to add a "virtual void SymbolVen

[Lldb-commits] [PATCH] D53436: [LLDB] - Implement the support for the .debug_loclists section.

clayborg added a comment. Just fix the assert to use lldbassert so we don't crash as mentioned in the inline comment and this is good to go. Comment at: source/Expression/DWARFExpression.cpp:3070 // Not supported entry type + assert(false && "Not supported location

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.

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

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Parsing a type shouldn't need an execution context and we shouldn't be re-parsing a type over and over for each frame. We should be encoding the array expression somewhere that w

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close. Just a bit of cleanup now that we changed SymbolFilePDB where we don't seem to need m_public_symbols anymore. See inlined comments and let me know what you think =

[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()

clayborg added a comment. Zach: yes the module mutex needs to be recursive. Plenty of places where the symbol file stuff can call other symbol file APIs. To avoid A/B locking issues, the lldb_private::Module, lldb_private::ObjectFile and lldb_private::SymbolVendor and lldb_private::SymbolFile a

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

clayborg added a comment. As long as Swig is happy and the ABI doesn't change I am ok with this. Will we see the variables better when debugging? Or is this solely so the SymbolContextItem type doesn't disappear from the debug info? https://reviews.llvm.org/D53597 __

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close, just down to making the SymbolVendor::GetSymtab() call symtab.CalculateSymbolSizes() and symtab.Finalize(). Comment at: source/Plugins/SymbolFile/P

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

clayborg added a comment. The current SymbolFile::FindTypes(...) in was designed with type base name only due to how DWARF stores it variables. It has a "const CompilerDeclContext *parent_decl_ctx" which can be specified in order to limit what we find. So we might be able to think of this as a

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

clayborg added a comment. In https://reviews.llvm.org/D53368#1276152, @aleksandr.urakov wrote: > Yes, I'll implement it tomorrow (I'm already OOO now), thanks. But is it > really necessary to check the number of symbols added if we must to calculate > / finalize the symtab after getting it from

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

clayborg added a comment. My issue with adding "exact_match" is it renders a few of the arguments useless: size_t FindTypes( const SymbolContext &sc, llvm::StringRef name, const CompilerDeclContext *parent_decl_ctx, bool exact_match, ...); With exact_match "parent_decl_ct

[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects

clayborg added a comment. Looks fine to me. Make sure no one else has any objections. Repository: rLLDB LLDB https://reviews.llvm.org/D53506 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[Lldb-commits] [PATCH] D53929: [LLDB] - Add support for DW_FORM_rnglistx and relative DW_RLE_* entries.

clayborg added a comment. Just one question about extracting the value for DW_AT_ranges. It would be nice if we just took care of extracting the value so the form value was more useful Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1072-1079 + if (at_rang

[Lldb-commits] [PATCH] D53929: [LLDB] - Add support for DW_FORM_rnglistx and relative DW_RLE_* entries.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Solution is fine. As long as we don't have to duplicate the work everywhere that needs a ranges offset. https://reviews.llvm.org/D53929 ___

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

clayborg added a comment. The only other thing you would need to change to get the usability back in check when doing things in GetNumChildren() would be to have the function that gets the typename take on optional execution context for dynamic types. The ValueObject can easily pass its executi

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

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

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

clayborg added a comment. In https://reviews.llvm.org/D53530#1284267, @aprantl wrote: > > I didn't realize that the string int [] is produced by ValueObject itself; > > I think this makes this option more palatable to me. I'll give it a try. > > So, it turns out it isn't. The only way to get the

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

clayborg added a comment. Just a few nits, but _very_ close. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:65 typedef std::vector CompileUnitColl; + typedef std::unordered_map TypeSignatureMap; Use llvm::DenseMap here? Co

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

clayborg added a comment. So it depends on what code was retrieving the symbol table from the object file. Can you detail where this was happening? Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits

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

clayborg added a comment. LGTM. lldbassert to check that AST in "enum_type" is the same as "this" since we now can after switching to CompilerType as arg instead of opaque clang type pointer. Comment at: include/lldb/Symbol/ClangASTContext.h:909 clang::EnumConstantDecl *Ad

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for making the changes! https://reviews.llvm.org/D53530 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.or

[Lldb-commits] [PATCH] D54241: Fix bug in printing ValueObjects and in PE/COFF Plugin

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close, just use the layout type as mentioned in the inline comment. Comment at: lldb/source/Core/ValueObjectVariable.cpp:70 if (var_type) -return va

[Lldb-commits] [PATCH] D54333: [CMake] Allow version overrides with -DLLDB_VERSION_MAJOR/MINOR/PATCH/SUFFIX

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

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

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:65 typedef std::vector CompileUnitColl; + typedef llvm::DenseMap TypeSignatureMap;

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

clayborg added a comment. Since we only handle . and -> this general idea seems ok to me. Do we even need a regex then? Maybe just search for first of ".-"? https://reviews.llvm.org/D54454 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

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

clayborg added a comment. Anything that takes a process shared pointer should be per process. A long time ago these plug-ins didn't take process, and as time went on they did take a process: 306633 jmolenda ABI(lldb::ProcessSP process_sp) { 306633 jmolenda if (process_sp.get())

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

clayborg added a comment. In https://reviews.llvm.org/D54454#1296392, @zturner wrote: > BTW, I will have to see if it's possible to write a test for this. Even when > I compiled and built a program with DWARF on Linux, the `target variable > Pi` example didn't "just work" for me, because `Find

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.

clayborg added a comment. yep Repository: rLLDB LLDB https://reviews.llvm.org/D52543 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54863: [ASTImporter] Set MustBuildLookupTable on PrimaryContext

clayborg accepted this revision. clayborg added a comment. Looks good to me as long as test suite is happy. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54863/new/ https://reviews.llvm.org/D54863 ___ lldb-commits

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

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:404 -// non-Apple platforms, but for now it is needed. -m_compiler->getL

[Lldb-commits] [PATCH] D54751: [LLDB] - Fix setting the breakpoints when -gsplit-dwarf and DWARF 5 were used for building the executable.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inlined comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:353 + // unit in contrast. + if (!addr_base) +addr_base = cu_die.GetAttrib

[Lldb-commits] [PATCH] D54751: [LLDB] - Fix setting the breakpoints when -gsplit-dwarf and DWARF 5 were used for building the executable.

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:354-355 + if (!addr_base) +addr_base = cu_die.GetAttributeValueAsUnsigned(m_dwarf, this, + DW_AT_GNU_addr_base, 0); dwo_cu->SetAddrB

[Lldb-commits] [PATCH] D54751: [LLDB] - Fix setting the breakpoints when -gsplit-dwarf and DWARF 5 were used for building the executable.

clayborg added a comment. Missed an inline comment on last comment Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:311 + dw_addr_t addr_base = + cu_die.GetAttributeValueAsUnsigned(m_dwarf, this, DW_AT_addr_base, 0); + SetAddrBase(addr_base); --

[Lldb-commits] [PATCH] D28305: [Host] Handle short reads and writes, take 3

clayborg added a comment. Any reason we are removing File::SeekFromCurrent and File::SeekFromEnd? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28305/new/ https://reviews.llvm.org/D28305 ___ lldb-commits mailing list lldb-commits@lists.llvm

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

clayborg added a comment. Would be great to see old and new output like Zach suggested. Is there a reason we need to use TableGen? Other command line tools just use llvm:🆑:opt stuff. Seems a bit obtuse to use TableGen? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.

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

clayborg added a comment. the old usage: Usage: lldb -h lldb -v [[--] [ ...]] lldb -a -f [-c ] [-s ] [-o ] [-S ] [-O ] [-k ] [-K ] [-Q] [-b] [-e] [-x] [-X] [-l ] [-d] [-z ] [[--] [ ...]] lldb -n -w [-s ] [-o ] [-S ] [-O ] [-k ] [-K ] [-Q] [-b] [-e] [-x] [-X] [-l ] [

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

clayborg added a comment. Can we still group the options as mentioned in my previous comment? Comment at: tools/driver/Driver.cpp:943 +usage << '\n'; +usage << argv[0] << " -h" << '\n'; +usage << argv[0] << " -v [[--] [ ...]]\n"; Get file base name

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

clayborg added a comment. I know cl::opt had ways to group options and the table gen is more powerful so it must have this feature? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 ___ lldb-commits m

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

clayborg added a comment. Can you attach new output with the grouping and extra usage? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:/

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

clayborg added a comment. I would wait to get another OK from anyone else just to be sure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

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

clayborg added a comment. I would be happy if we just have an EXAMPLES section much like many of the man pages for built in shell commands where we show a few examples. I would like to see setting up a target a program with arguments that might conflict with the argument parser like: % lldb

[Lldb-commits] [PATCH] D54751: [LLDB] - Fix setting the breakpoints when -gsplit-dwarf and DWARF 5 were used for building the executable.

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:355-356 + if (addr_base == LLDB_INVALID_ADDRESS) +addr_base = cu_die.GetAttributeValueAsUnsigned(m_dwarf, this, + DW_AT_GNU_addr_base,

[Lldb-commits] [PATCH] D54751: [LLDB] - Fix setting the breakpoints when -gsplit-dwarf and DWARF 5 were used for building the executable.

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:355-356 + if (addr_base == LLDB_INVALID_ADDRESS) +addr_base = cu_die.GetAttributeValueAsUnsigned(m_dwar

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Seems like we should just add a "bool interactive" as a second parameter to "IOHandlerActivated". Then it will be easy to find the other places that need to be fixed up. =

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

clayborg added a comment. Looks fine to me. Jim Ingham should ok this as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54843/new/ https://reviews.llvm.org/D54843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.l

[Lldb-commits] [PATCH] D55214: Introduce ObjectFileBreakpad

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. This looks like a good start. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55214/new/ https://reviews.llvm.org/D55214 ___ lldb-comm

[Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

clayborg added a comment. What is the reason we aren't using cmake + ninja for this kind of stuff? Or is it using it under the covers? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55230/new/ https://reviews.llvm.org/D55230 ___ lldb-commit

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

clayborg added a comment. I like seeing all of the cmake modifications for the LLDB.framework. Are we planning on trying to get rid of the Xcode project at some point soon and use the auto generated one made by cmake? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55328/new/ https://re

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. This is already available with: virtual lldb_private::Address ObjectFile::GetHeaderAddress(); It return a lldb_private::Address which is section offset, but nothing stopping

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

clayborg added a comment. Marked as requesting changes in case "GetBaseFileAddress() == GetHeaderAddress().GetFileAddress()" in all cases. If the base file address differs from where the object file header is located, then this change would work. Else we should use GetHeaderAddress() and possib

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

clayborg added inline comments. Comment at: include/lldb/Symbol/ObjectFile.h:569 + /// Returns the base file address of an object file (also known as the + /// preferred load address or image base address). This is typically the file lemo wrote: > "file addre

[Lldb-commits] [PATCH] D55422: Rename ObjectFile::GetHeaderAddress to GetBaseAddress

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks fine. Just one clarification in the header documentation and this is good to go. Comment at: include/lldb/Symbol/ObjectFile.h:561 + /// will have this section set

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So sections should correspond to different kinds of sections, like .text, .data, etc. If we have the following breakpad file: MODULE Linux x86_64 24B5D199F0F766FF5D

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg added a comment. Another point of clarification is that sections exist in order to lookup addresses and resolve addresses to a section within a file. The section should be something that can easily be slid around when loaded by LLDB when we are debugging or symbolicating. So any sectio

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg added a comment. So we do something like you describe with the DYSM files. The object file is "a.out" and it has a dSYM file "a.out.dSYM/Context/Resources/DWARF/a.out" and the dSYM file will share sections with the "a.out" object file. So if you plan on loading the breakpad file as a s

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg added a comment. If you plan on not making the breakpad file ever stand alone, then you will need to take any addresses and look them up in the module section list and use the other sections. I don't see why the breakpad file can't be stand alone though. It won't be as accurate, but it

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I am going to stop making comments as I have been working on a similar patch that does more than this one. I will post it today and we can decide which approach we want to use.

[Lldb-commits] [PATCH] D55522: Cache memory regions and use the linux maps as the source of the information if available.

clayborg created this revision. clayborg added reviewers: labath, markmentovai, zturner, tatyana-krasnukha. Herald added a subscriber: mgrang. Breakpad creates minidump files that sometimes have: - linux maps textual content - no MemoryInfoList Right now unless the file has a MemoryInfoList we g

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

clayborg added a comment. See my patch: https://reviews.llvm.org/D55522 Let me know what you think Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55472/new/ https://reviews.llvm.org/D55472 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available.

clayborg updated this revision to Diff 177555. clayborg added a comment. Add sorting functions needed for MemoryRegionInfo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55522/new/ https://reviews.llvm.org/D55522 Files: include/lldb/Target/MemoryRegionInfo.h source/Plugins/Process/m

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg added a comment. In D55434#1325739 , @labath wrote: > In D55434#1323912 , @clayborg wrote: > > > If you plan on not making the breakpad file ever stand alone, then you will > > need to take any addresses a

[Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

clayborg added a comment. BTW: check my changes in: https://reviews.llvm.org/D55522 It will be interesting to you since it parses the linux maps info if it is available in breakpad generated minidump files. This will give us enough info to create correct sections for object files when we have no

  1   2   3   4   5   6   7   8   9   10   >