[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:

[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] 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

[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] 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

[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] 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

[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] 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

[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

[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

[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

[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-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 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

[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

[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

[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

[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] 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] 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

[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:

[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] [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

[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-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

[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 ___

[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

[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

[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

[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] 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

[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

[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

[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.

[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]+) +[+]?([^ ]+)

[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/

[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.

[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

[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 = -

[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!

[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 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] 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

[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

[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] 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

[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

[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] 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

[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/

[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

[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

[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

[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

[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

[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-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-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

[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

[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] 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] 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);

[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] 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

[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. -

[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 =

[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] [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 =

[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] 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

[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

[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 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

[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

[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

[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: > > > > >

[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] [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] 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

[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

[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

[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

[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

[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

[Lldb-commits] [PATCH] D56564: [SymbolFile] Make ParseCompileUnitXXX accept a CompileUnit&.

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

[Lldb-commits] [PATCH] D56564: [SymbolFile] Make ParseCompileUnitXXX accept a CompileUnit&.

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

[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

[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

[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] 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-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] 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-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

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

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

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

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