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

clayborg added inline comments. Comment at: include/lldb/API/SBMemoryRegionInfoList.h:31 - bool GetMemoryRegionAtIndex(uint32_t idx, SBMemoryRegionInfo ®ion_info); + void Reserve(size_t capacity); zturner wrote: > clayborg wrote: > > This you can add, but s

[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 177567. clayborg added a comment. Removed Xcode scheme changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55522/new/ https://reviews.llvm.org/D55522 Files: include/lldb/Target/MemoryRegionInfo.h source/Plugins/Process/minidump/MinidumpPars

[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 177566. clayborg marked 2 inline comments as done. clayborg added a comment. Fixed Zach's issues. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55522/new/ https://reviews.llvm.org/D55522 Files: include/lldb/Target/MemoryRegionInfo.h lldb.xcode

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

clayborg marked 6 inline comments as done. clayborg added inline comments. Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:412 + constexpr const auto yes = MemoryRegionInfo::eYes; + constexpr const auto no = MemoryRegionInfo::eNo; + while (!text.empty()) { -

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

clayborg added a comment. In D55522#1325839 , @labath wrote: > This patch certainly provides a more comprehensive treatment of memory region > information in minidump. However, there might be some value in removing the > excessive shared_ptrs in the othe

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

clayborg added a comment. In D55522#1326035 , @tatyana-krasnukha wrote: > Don't you mind to take overridden GetMemoryRegions in this patch so that I > can remove all minidump-related changes from D55472 > ? That is fin

[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 177592. clayborg added a comment. Add reserve and shrink to fit when parsing memory regions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55522/new/ https://reviews.llvm.org/D55522 Files: include/lldb/Target/MemoryRegionInfo.h source/Plugins/

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

clayborg added a comment. FYI: my approach to getting symbols to load was a bit different. I always let a simple PlaceholderModule be created, but I played some tricks in the GetObjectFile() method if someone had setting symbols for the module with "target symbols add ..". I will attach my Plac

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

clayborg added a comment. In D55356#1327099 , @labath wrote: > Actually, this now causes an lldb-mi test to fail, but it's not clear to me > if the problem is in the test, or this patch. This issue happens when lldb-mi > is printing the "library loaded"

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

clayborg added a comment. In D55522#1326955 , @labath wrote: > It just occurred to me that we have another copy of /proc//maps -> > MemoryRegionInfo conversion code. It lives in NativeProcessLinux.cpp > (ParseMemoryRegionInfoFromProcMapsLine). It would b

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

clayborg added a comment. In D55356#1327224 , @clayborg wrote: > In D55356#1327099 , @labath wrote: > > > Actually, this now causes an lldb-mi test to fail, but it's not clear to me > > if the problem is in the tes

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

clayborg added a comment. In D55356#1327242 , @labath wrote: > In D55356#1327224 , @clayborg wrote: > > > In D55356#1327099 , @labath wrote: > > > > > Actually, this now caus

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good to me. Jim should ok as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55571/new/ https://reviews.llvm.org/D55571 __

[Lldb-commits] [PATCH] D55614: Fix MinidumpParser::GetFilteredModuleList() and test it

clayborg created this revision. clayborg added reviewers: zturner, labath, markmentovai, lemo. The MinidumpParser::GetFilteredModuleList() code was attempting to iterate through the entire module list and if it found more than one entry for a given module name, it wanted to pick the MinidumpModu

[Lldb-commits] [PATCH] D55614: Fix MinidumpParser::GetFilteredModuleList() and test it

clayborg added a comment. The order in the modules list is maintained by changing the llvm::StringMap to map from module name to "filtered_modules" index. This avoids having to iterate across the StringMap in the end and make the filtered_modules in a different ordering. CHANGES SINCE LAST AC

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

clayborg added inline comments. Comment at: source/Target/Target.cpp:2867-2880 // Get a weak pointer to the previous process if we have one ProcessWP process_wp; if (m_process_sp) process_wp = m_process_sp; -m_process_sp = -GetPlatform()->DebugPr

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just need a way to verify symbols are good. See my inline comment. Comment at: source/Commands/CommandObjectTarget.cpp:4246 if (symbol_file) { -

[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 178168. clayborg added a comment. Herald added a subscriber: mgorny. Fixed errors, fully test, and make static function that are local to MinidumpParser.cpp that parse the region info from linux maps, memory info list, memory list and memory 64 list. CHAN

[Lldb-commits] [PATCH] D55706: ELF: more section creation cleanup

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

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

clayborg added a comment. Fixed the test inputs CMakeLists.txt file: $ svn commit SendingCMakeLists.txt Transmitting file data .done Committing transaction... Committed revision 349183. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55522/new/ https://revi

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

clayborg requested changes to this revision. clayborg added inline comments. Comment at: examples/python/crashlog.py:99 image_regex_uuid = re.compile( -'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)') +'(0x[0-9a-fA-F]+)[-

[Lldb-commits] [PATCH] D55727: Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used.

clayborg created this revision. clayborg added reviewers: zturner, labath, lemo. Each process plug-in can create its own custom commands. I figured it would be nice to be able to dump things from the minidump file from the lldb command line, so I added the start of the some custom commands. Cur

[Lldb-commits] [PATCH] D55727: Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used.

clayborg added a comment. In D55727#1331843 , @zturner wrote: > This sounds like a good use-case for a lit / FileCheck test. Could you add > some simple tests that run lldb with -c on a static minidump with known > information inside that we don't chang

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

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: examples/python/crashlog.py:99 image_regex_uuid = re.compile( -'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9

[Lldb-commits] [PATCH] D55757: ELF: Don't create sections for section header index 0

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. See inline comment and fix if you agree. Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1993 SymbolType symbol_type = eSymbolTypeInvalid;

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

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

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55724/new/ https://reviews.llvm.org/D55724 ___ l

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. It would be nice to see context when you submit diffs. with SVN: svn diff -x -U999 ... Or git: git diff -U999 Comment at: include/lldb/Core/Archi

[Lldb-commits] [PATCH] D55776: Fix lldb's macosx/heap.py cstr command

clayborg added a comment. nice! A lot of great commands in heap.py. Like see them fixed and improved CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55776/new/ https://reviews.llvm.org/D55776 ___ lldb-commits mailing list lldb-commits@lists.l

[Lldb-commits] [PATCH] D55727: Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used.

clayborg updated this revision to Diff 178549. clayborg added a comment. Fixed all requested changes. Added a complete test in lit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55727/new/ https://reviews.llvm.org/D55727 Files: lit/Minidump/Inputs/dump-content.dmp lit/Minidump/dump

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS

clayborg added inline comments. Comment at: CMakeLists.txt:160 list(APPEND LLDB_TEST_DEPS clang) +if(APPLE) + list(APPEND LLDB_TEST_DEPS cxx) What if the user wants to use a different compiler for tests? cmake has LLDB_TEST_USE_CUSTOM_C_COMPILER an

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Change looks good. We need to add a test for this. You can use the minidump file from "lldb/unittests/Process/minidump/Inputs/regions-linux-map.dmp" and copy it over into "lldb

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

clayborg added a comment. See inline comments and let me know what you think. Comment at: include/lldb/Utility/ArchSpec.h:91-92 + // ARC configuration flags + enum ARCflags { eARC_rf32 = 0 /*0b00*/, eARC_rf16 = 2 /*0b10*/ }; + Since no other place needs the

[Lldb-commits] [PATCH] D55854: Show the memory region name if there is one in the output of the "memory region" command

clayborg created this revision. clayborg added reviewers: zturner, labath, lemo, jingham. Prior to this change we would show the name of the section that a memory region belonged to but not its actual region name. Now we show this,. Added a test that reuses the regions-linux-map.dmp minidump fil

[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. One quick change required to avoid STL in public headers. Comment at: include/lldb/lldb-forward.h:16 #include "lldb/Utility/SharingPtr.h" +#include

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

clayborg added a comment. In D55718#1336045 , @tatyana-krasnukha wrote: > `ARCflags` are used by ABISysV_arc (related patch D55724 > ). I would be glad to move it to > architecture plugin, but I ought to add SetFlags/Get

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

clayborg added a comment. In D55841#1336515 , @tatyana-krasnukha wrote: > Add a test There was one added. See code Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55841/new/ https://reviews.llvm.org/D55841 _

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

clayborg added a comment. In D55841#1336519 , @clayborg wrote: > In D55841#1336515 , > @tatyana-krasnukha wrote: > > > Add a test > > > There was one added. See code Never mind, I thought this was my patch! Ignor

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just remove the "regions-linux-map.dmp" as I already checked one in with rLLDB349658 and this is good to go Comment at:

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55841/new/ https://reviews.llvm.org/D55841 ___ l

[Lldb-commits] [PATCH] D55998: ELF: create "container" sections from PT_LOAD segments

clayborg added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1841-1844 +struct SegmentAddressInfo { + addr_t Address; + addr_t Size; +}; Use existing range code from Range.h? ``` #include "lldb/Utility/Range.h" typedef Range Segme

[Lldb-commits] [PATCH] D55995: [lldb] - Fix compilation with MSVS 2015 update 3

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. The changes from "constexpr" to "const" might introduce global constructors in the shared library which is what we were trying to avoid. The less work that the LLDB shared librar

[Lldb-commits] [PATCH] D55991: DWARF: Fix a bug in array size computation

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp:619 + case DW_FORM_ref_addr: + case DW_FORM_ref_sig8: + case DW_FORM_GNU_ref_alt:

[Lldb-commits] [PATCH] D56010: [NativePDB] Fix setting breakpoint by file and line

clayborg added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56010/new/ https://reviews.llvm.org/D56010 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56173: Introduce SymbolFileBreakpad and use it to fill symtab

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

[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

clayborg added inline comments. Comment at: include/lldb/Core/RangeMap.h:636 -template class RangeDataArray { +template +class RangeDataVector { labath wrote: > tberghammer wrote: > > Would it make sense to have a default value of 0 for N so people don't hav

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

clayborg added a comment. So is this done as one section per function? Or one section for contiguous functions? What about if there are only symbols? I tried to read the code but wasn't able to decipher everything clearly in my head. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55434/

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

clayborg added a comment. Just read the original description again and now code makes sense. Main questions for me: is there a benefit to creating multiple sections? Can we just create one section and name it ".breakpad"? Should we not try to find a section that contains the address from the FU

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

clayborg added a comment. excited to see this starting as well! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56233/new/ https://reviews.llvm.org/D56233 ___ lldb-commits mailing list lldb-commits@lists.llvm.org htt

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

clayborg added a comment. Seems like DynamicLoaderWindowsDYLD isn't doing its job correctly here and DynamicLoaderWindowsDYLD should be fixed. We shouldn't have to go to the process at all in order to set the section load addresses correctly. Why? As you might have seen lldb-server.exe is being

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So it seems like DynamicLoaderWindowsDYLD is not doings its job correctly and it needs to be fixed. DynamicLoaderWindowsDYLD is responsible for setting all of the section load ad

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

clayborg added a comment. I have a minidump generator if you need me to make any specific minidump files for you. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56293/new/ https://reviews.llvm.org/D56293 ___ lldb-c

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. You have convinced me! Sorry I had paged out the original intent you conveyed from before the break. Thanks for the details. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55434/ne

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

clayborg added a comment. The UUID that is used in ELF and Mach-o is designed to be something that is stable in a binary after it has been linked and should be the same before and after any kind of post production (stripping symbols, stripping section content to make a stand alone symbol file,

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

clayborg added a comment. Second vote from me to switch to a std::vector since we have a function get breakpoint at index. Other than that, looks good. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56425/new/ https://reviews.llvm.org/D56425

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

clayborg added inline comments. Comment at: source/Breakpoint/BreakpointList.cpp:22 +Target::eBroadcastBitBreakpointChanged, +new Breakpoint::BreakpointEventData(event, bp)); +} If we don't pass a "BreakpointSP& sp", then use std::move?

[Lldb-commits] [PATCH] D55998: ELF: create "container" sections from PT_LOAD segments

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

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

clayborg accepted this revision. clayborg added a comment. If we keep the list sorted we might be able to improve finding breakpoints by ID, but that can be done if we need to. BreakpointList::Add would need to insert it sorted, then we can get better than O(n) performance on FindBreakpointByID

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

clayborg added a comment. In D56425#1350253 , @JDevlieghere wrote: > In D56425#1350236 , @JDevlieghere > wrote: > > > In D56425#1350234 , @clayborg > > wrote: > > > > > If

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

clayborg updated this revision to Diff 145969. clayborg added a comment. Broke up with patch further by submitting: [PATH] https://reviews.llvm.org/D46529: Add support to object files for accessing the .debug_types section https://reviews.llvm.org/D46529 [PATCH] https://reviews.llvm.org/D46606:

[Lldb-commits] [PATCH] D46687: Remove custom path manipulation functions from FileSpec

clayborg added inline comments. Comment at: source/Utility/FileSpec.cpp:277-281 + auto style = LLVMPathSyntax(syntax); + m_filename.SetString(llvm::sys::path::filename(resolved, style)); + llvm::StringRef dir = llvm::sys::path::parent_path(resolved, style); + if (!dir.empty()

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

clayborg added a comment. In https://reviews.llvm.org/D32167#1094173, @labath wrote: > The patch looks much better now, but I think we still need to discuss the > data extractor sliding issue, as right now that's very hacky. It makes things just work with .debug_info as is pretends that the .d

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

clayborg added a comment. I really need to get this in as well. It is a very important feature and I am trying to do the right thing and get it into open source so we don't end up keeping the patch locally. So any help on expediting any comments and iterations would be great. https://reviews.

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1046-1047 + { +static std::mutex do_dlopen_mutex; +std::lock_guard lock(do_dlopen_mutex

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

clayborg added inline comments. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1047 +static std::mutex do_dlopen_mutex; +std::lock_guard lock(do_dlopen_mutex); + Accessor would be fine. The other reason for putting this in the process is mul

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

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

[Lldb-commits] [PATCH] D46753: FileSpec: Remove PathSyntax enum and use llvm version instead

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Fine to use llvm's enum as long as we don't export the style through the SB API https://reviews.llvm.org/D46753 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

clayborg added a comment. After reading Pavel's and Paul's comments, I will come up with a solution that allows multiple sections to be combined into one large section. That way it will be all legal and easy to extend. If we have just .debug_info, then we can use a base class that just calls di

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg created this revision. clayborg added reviewers: labath, jingham, davide, zturner. After switching to LLVM normalization, if we init FileSpec with "." we would end up with m_directory being NULL and m_filename being "". This patch fixes this by allowing the path to be normalized and if

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. This patch is needed for some PathMappingList fixes I have coming up. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. I agree, but we should still guard against it and test in in LLDB to ensure this doesn't regress. I'll be happy to make a LLVM patch. Many people have branches that link against older LLVMs and the check is cheap. https://reviews.llvm.org/D46783 ___

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So this problem exists both in the LLDB and LLVM DWARF parsers. I am not sure this fix is safe. I would rather fix this by fixing DWARFDIE class to "do the right thing". We shoul

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. So the function in llvm is called llvm::sys::path::remove_dots(...) and it is removing the dots. Not sure it is correct to be changing a function that says "remove_dots" to not remove dots and actually return something with a . in it... Seems like this should be taken

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

clayborg added a comment. In https://reviews.llvm.org/D46588#1099536, @aprantl wrote: > > If we're not able to come up with a command to make lldb-mi wait until the > > target stops (maybe there is one already? I know very little about > > lldb-mi), we may have to revisit the whole testing stra

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. In https://reviews.llvm.org/D46783#1097549, @labath wrote: > Preserving the dot if it is the only path component is perfectly reasonable > -- "" is not a valid path and we shouldn't convert a valid path into an > invalid one. However, I think this should be done on the

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. So I made a fix to LLVM and there are tests that are testing for the empty string: [ RUN ] Support.RemoveDots /Users/gclayton/Documents/src/llvm/clean/llvm/unittests/Support/Path.cpp:1149: Failure Expected: "" To be equal to: remove_dots(".\\", false,

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. Submitted the LLVM patch: https://reviews.llvm.org/D46887 https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg abandoned this revision. clayborg added a comment. Fixed LLDB test botes with: Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332511. https://reviews.llvm.org/D46783

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg reclaimed this revision. clayborg added a comment. Reviving this patch so I can get the changes into LLDB. Clang is expecting an empty path in many locations and I don't feel comfortable changing remove_dots in clang after trying it. So I would like to use this patch to fix things in L

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

clayborg added a comment. Looks fine. Just one question on keeping the DWARFIndex::Create() functions so they all have the same signature. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40 + +std::unique_ptr AppleIndex::Create( +Module &module, DWARFDataExtract

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40 + +std::unique_ptr AppleIndex::Create( +Module &module, DWARFDataExtractor apple_names, clayborg wrote: > Move all AppleIndex stuff to a dedicated .cpp file? Do we w

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1984 GetObjectFile()->GetModule()->GetMutex()); - Index(); -} Yikes, who was

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

clayborg added a comment. In https://reviews.llvm.org/D46588#1103237, @aprantl wrote: > For the experiment you can probably just stick it into > `CMICmnLLDBDebugger::InitSBDebugger()`. But don't do it here permanently... Repository: rL LLVM https://reviews.llvm.org/D46588 _

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg updated this revision to Diff 147345. clayborg added a comment. Updated to what was committed. https://reviews.llvm.org/D46783 Files: source/Utility/FileSpec.cpp unittests/Utility/FileSpecTest.cpp Index: unittests/Utility/FileSpecTest.cpp =

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. I did run unit tests and they all passed here? https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a subscriber: labath. clayborg added a comment. Fixed with svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. https://reviews.llvm.org/D46783

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

clayborg added a comment. Fixed with: svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. https://reviews.llvm.org/D46783

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

clayborg created this revision. clayborg added reviewers: labath, zturner, davide. Herald added subscribers: JDevlieghere, aprantl, mgorny. PathMappingList was broken for relative and empty paths after normalization changes in FileSpec. There were also no tests for PathMappingList so I added tho

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

clayborg updated this revision to Diff 147354. clayborg added a comment. Fix issues found by Zach. https://reviews.llvm.org/D47021 Files: include/lldb/Target/PathMappingList.h lldb.xcodeproj/project.pbxproj source/Target/PathMappingList.cpp source/Target/Target.cpp unittests/Utility/C

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

clayborg marked 7 inline comments as done. clayborg added a comment. Marked things as done. https://reviews.llvm.org/D47021 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

clayborg added a comment. I will make the fixes and also test out mapping "" to "." as suggested. Comment at: source/Target/PathMappingList.cpp:194 + // path and any path we appended would end up being relative. + fixed.SetFile(path_ref, false); +} else {

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

clayborg updated this revision to Diff 147587. clayborg added a comment. - Fixed Pavel's issues - If user specifies "" as the first directory in PathMappingList, it will match "." - User can specify "/" as the first directory to remap all absolute paths - Fixed FileSpec::IsAbsolute() and added te

[Lldb-commits] [PATCH] D47110: [LLDB, lldb-mi] Add option --synchronous.

clayborg accepted this revision. clayborg added a comment. Looks fine, just move the option parsing code down by the others instead of having it before the checking for file args. Comment at: tools/lldb-mi/MIDriver.cpp:441-444 + if (strArg.compare("--synchronous") == 0) {

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

clayborg added a comment. Before I look too closely, we can probably add a DWARFDIE accessor function like: bool DWARFDIE::IsClassMethod() const; Then this would allow each DWARFIndex to just check and we might be able to get rid of the 3 lists in GetFunctionsByBaseOrMethodName? Seems like i

[Lldb-commits] [PATCH] D47250: Move ObjectFile initialization out of SystemInitializerCommon

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

[Lldb-commits] [PATCH] D47253: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex

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

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

clayborg added a comment. Marked things that don't belong in DWARFBasicDIE. Also DWARFBasicDIE doesn't really explain what it actually is. Maybe we should rename this DWARFUnitDIE? DWARFTopDIE? DWARFRootDIE? Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:49 + +

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

clayborg added a comment. It would be good to verify all headers that are in the LLDB.framework produced by Xcode builds are also in the LLDB.framework from cmake/ninja https://reviews.llvm.org/D47278 ___ lldb-commits mailing list lldb-commits@list

[Lldb-commits] [PATCH] D47276: 2/3: Use DWARFBasicDIE as compile-time protection

clayborg added a comment. Waiting for answers on renaming from patch 1/3 before commenting. https://reviews.llvm.org/D47276 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

clayborg added a comment. In https://reviews.llvm.org/D47278#1110092, @xiaobai wrote: > I also think that'd be a great idea. The only way I can think to do that > would be to build LLDB.framework using both build systems and compare the > two. Is there a faster way? That is the only guarantee

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