Page MenuHomePhabricator

clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 10:11 AM (225 w, 16 h)

Recent Activity

Yesterday

clayborg accepted D56543: DWARF: Add some support for non-native directory separators.

Ok, thanks for explaining. Your reasoning makes sense.

Tue, Jan 15, 1:22 PM
clayborg added a comment to D56543: DWARF: Add some support for non-native directory separators.

Might be good to have a test where we have a relative DW_AT_name and not DW_AT_comp_dir? Just in case?

Tue, Jan 15, 1:22 PM
clayborg accepted D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC).

Looks good to me

Tue, Jan 15, 9:47 AM
clayborg added a comment to D56595: SymbolFileBreakpad: Add line table support.

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

Tue, Jan 15, 9:12 AM
clayborg added inline comments to D56590: breakpad: Add FUNC records to the symtab.
Tue, Jan 15, 8:43 AM
clayborg added inline comments to D56590: breakpad: Add FUNC records to the symtab.
Tue, Jan 15, 8:40 AM
clayborg accepted D56589: Teach the default symbol vendor to respect module.GetSymbolFileFileSpec().
Tue, Jan 15, 8:25 AM
clayborg added inline comments to D56543: DWARF: Add some support for non-native directory separators.
Tue, Jan 15, 8:21 AM
clayborg added inline comments to D56237: Implement GetFileLoadAddress for the Windows process plugin.
Tue, Jan 15, 7:12 AM

Mon, Jan 14

clayborg accepted D56614: [SymbolFile] Change ParseFunctionBlocks(SymbolContext&) to ParseBlocksRecursive(Function&).
Mon, Jan 14, 10:36 AM
clayborg added inline comments to D55718: [ARC] Basic support in gdb-remote process plugin.
Mon, Jan 14, 10:34 AM · Restricted Project

Thu, Jan 10

clayborg accepted D56564: [SymbolFile] Make ParseCompileUnitXXX accept a CompileUnit&..

This is fine to start.

Thu, Jan 10, 4:43 PM
clayborg added a comment to D56564: [SymbolFile] Make ParseCompileUnitXXX accept a CompileUnit&..

All the change to the symbol vendor make sense, but it seems like all of the call sites should be:

Thu, Jan 10, 3:31 PM
clayborg requested changes to D56543: DWARF: Add some support for non-native directory separators.

DW_AT_comp_dir isn't enough. See inline suggestions.

Thu, Jan 10, 10:25 AM

Wed, Jan 9

clayborg added a comment to D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit.

But if everyone else thinks differently, I'll go with the majority. Just my initial thoughts.

Wed, Jan 9, 9:55 AM
clayborg added a comment to D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit.

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 one
Wed, Jan 9, 9:55 AM
clayborg added a comment to D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit.

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 compile unit. If we pass in a symbol context with a function or block, we parse all types in that function or block. Currently it is only being used for compile units, but I don't think we need to change the API.

Wed, Jan 9, 9:53 AM

Tue, Jan 8

clayborg added a comment to D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC).

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 and Remove (anything that was using find_if when it is searching for a breakpoint by ID).

I'll do this as a follow-up.

On second thought, maybe it's not such a good idea after all. Internal breakpoints are negative so if we keep the vector sorted we'd always insert them at the front of the vector. We could change strategies based on whether the list is internal or not, but that seems overkill, especially given the code is not that hot as Vedant pointed out.

Tue, Jan 8, 3:04 PM
clayborg accepted D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC).

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 and Remove (anything that was using find_if when it is searching for a breakpoint by ID).

Tue, Jan 8, 2:03 PM
clayborg accepted D55998: ELF: create "container" sections from PT_LOAD segments.

LGTM

Tue, Jan 8, 8:27 AM
clayborg added inline comments to D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC).
Tue, Jan 8, 8:23 AM
clayborg added a comment to D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC).

Second vote from me to switch to a std::vector since we have a function get breakpoint at index. Other than that, looks good.

Tue, Jan 8, 8:17 AM

Fri, Jan 4

clayborg added a comment to D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID().

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, etc). When someone types "target symbols add /path/to/symbol/file/a.out" we will grab its UUID and try to match it up with an existing object file.

Fri, Jan 4, 2:39 PM
clayborg accepted D55434: ObjectFileBreakpad: Implement sections.

You have convinced me! Sorry I had paged out the original intent you conveyed from before the break. Thanks for the details.

Fri, Jan 4, 11:08 AM
clayborg accepted D56315: Add a verbose mode to "image dump line-table" and use it to write a .debug_line test.
Fri, Jan 4, 10:59 AM

Thu, Jan 3

clayborg added a comment to D56293: Use the minidump exception record if present.

I have a minidump generator if you need me to make any specific minidump files for you.

Thu, Jan 3, 3:49 PM · Restricted Project
clayborg requested changes to D56237: Implement GetFileLoadAddress for the Windows process plugin.

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 addresses correctly using an abstract lldb_private::Process plug-in. No special calls should be added if that is possible. We have someone working on lldb-server.exe and then we will have two clients for DynamicLoaderWindowsDYLD: ProcessWindows and ProcessGDBRemote. DynamicLoaderWindowsDYLD should work for any lldb_private::Process subclass without the need to call back into a specific lldb_private::Process subclass method.

Thu, Jan 3, 11:20 AM
clayborg added a comment to D56237: Implement GetFileLoadAddress for the Windows process plugin.

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 worked on now and then we will have two clients for DynamicLoaderWindowsDYLD: the native ProcessWindows plug-in and ProcessGDBRemote. The

Thu, Jan 3, 11:17 AM
clayborg added a comment to D56233: [lldb-server] Add initial support for building lldb-server on Windows.

excited to see this starting as well!

Thu, Jan 3, 11:11 AM
clayborg added a comment to D55434: ObjectFileBreakpad: Implement sections.

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 FUNC, line entry or PUBLIC and then avoid creating a section? That way we only create breakpad sections of there are not backing sections from a real object or symbol file?

Thu, Jan 3, 11:09 AM
clayborg added a comment to D55434: ObjectFileBreakpad: Implement sections.

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.

Thu, Jan 3, 11:05 AM
clayborg accepted D56170: RangeMap.h: merge RangeDataArray and RangeDataVector.
Thu, Jan 3, 10:58 AM

Wed, Jan 2

clayborg accepted D56218: Rearrange bitfield to allow for more space in file_idx..
Wed, Jan 2, 3:07 PM
clayborg accepted D56132: Symtab: Remove one copy of symbol size computation code.
Wed, Jan 2, 1:21 PM
clayborg accepted D56147: [Core] Use the implementation method `GetAddressOf` in `ValueObjectConstResultChild`.
Wed, Jan 2, 1:19 PM · Restricted Project
clayborg accepted D56174: ProcessLaunchInfo: remove Debugger reference.
Wed, Jan 2, 1:08 PM
clayborg accepted D56196: ProcessLaunchInfo: Remove Target reference.
Wed, Jan 2, 12:57 PM
clayborg added inline comments to D56170: RangeMap.h: merge RangeDataArray and RangeDataVector.
Wed, Jan 2, 9:59 AM
clayborg accepted D56173: Introduce SymbolFileBreakpad and use it to fill symtab.

LGTM.

Wed, Jan 2, 9:54 AM
clayborg accepted D56129: Simplify ObjectFile::GetArchitecture.
Wed, Jan 2, 9:45 AM

Fri, Dec 21

clayborg added a comment to D56010: [NativePDB] Fix setting breakpoint by file and line.

LGTM

Fri, Dec 21, 9:46 AM
clayborg accepted D55991: DWARF: Fix a bug in array size computation.
Fri, Dec 21, 9:43 AM
clayborg requested changes to D55991: DWARF: Fix a bug in array size computation.
Fri, Dec 21, 7:26 AM
clayborg requested changes to D55995: [lldb] - Fix compilation with MSVS 2015 update 3.

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 library does on load the better. We might need to use a macro that expands to "constexpr" for non windows and to "const" for windows in a private LLDB header (PropertyDefinition.h?)

Fri, Dec 21, 7:16 AM
clayborg added inline comments to D55998: ELF: create "container" sections from PT_LOAD segments.
Fri, Dec 21, 7:11 AM

Wed, Dec 19

clayborg accepted D55472: Speadup memory regions handling and cleanup related code.
Wed, Dec 19, 12:24 PM · Restricted Project
clayborg accepted D55841: GetMemoryRegions for the ProcessMinidump.

Looks good!

Wed, Dec 19, 12:18 PM · Restricted Project
clayborg requested changes to D55841: GetMemoryRegions for the ProcessMinidump.

Just remove the "regions-linux-map.dmp" as I already checked one in with rLLDB349658 and this is good to go

Wed, Dec 19, 10:41 AM · Restricted Project
clayborg added a comment to D55841: GetMemoryRegions for the ProcessMinidump.

Add a test

There was one added. See code

Wed, Dec 19, 10:39 AM · Restricted Project
clayborg added a comment to D55841: GetMemoryRegions for the ProcessMinidump.

Add a test

Wed, Dec 19, 10:38 AM · Restricted Project
clayborg added a comment to D55718: [ARC] Basic support in gdb-remote process plugin.

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/GetFlags to Architecture interface in this case. Then we'll have the same members in ArchSpec and in Architecture, that may look confusing.

Wed, Dec 19, 10:26 AM · Restricted Project
clayborg requested changes to D55472: Speadup memory regions handling and cleanup related code.

One quick change required to avoid STL in public headers.

Wed, Dec 19, 9:25 AM · Restricted Project

Tue, Dec 18

clayborg added a comment to D53379: GSYM symbolication format.

2 month ping

Tue, Dec 18, 2:58 PM
clayborg created D55854: Show the memory region name if there is one in the output of the "memory region" command.
Tue, Dec 18, 2:22 PM
clayborg added a comment to D55718: [ARC] Basic support in gdb-remote process plugin.

See inline comments and let me know what you think.

Tue, Dec 18, 1:39 PM · Restricted Project
clayborg accepted D55472: Speadup memory regions handling and cleanup related code.
Tue, Dec 18, 1:22 PM · Restricted Project
clayborg requested changes to D55841: GetMemoryRegions for the ProcessMinidump.

Change looks good.

Tue, Dec 18, 1:14 PM · Restricted Project
clayborg added inline comments to D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS.
Tue, Dec 18, 1:05 PM · Restricted Project

Mon, Dec 17

clayborg updated the diff for D55727: Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used..

Fixed all requested changes. Added a complete test in lit.

Mon, Dec 17, 3:49 PM
clayborg added a comment to D55776: Fix lldb's macosx/heap.py cstr command.

nice! A lot of great commands in heap.py. Like see them fixed and improved

Mon, Dec 17, 10:30 AM
clayborg requested changes to D55718: [ARC] Basic support in gdb-remote process plugin.

It would be nice to see context when you submit diffs. with SVN:

svn diff -x -U9999999 ...

Or git:

git diff -U9999999
Mon, Dec 17, 9:45 AM · Restricted Project
clayborg accepted D55724: [ARC] Add SystemV ABI.

Looks good.

Mon, Dec 17, 9:18 AM · Restricted Project
clayborg accepted D55631: Delay replacing the process till after we've finalized it.
Mon, Dec 17, 9:15 AM
clayborg accepted D55608: Make crashlog.py work or binaries with spaces in their names.

Looks good

Mon, Dec 17, 9:07 AM
clayborg accepted D55757: ELF: Don't create sections for section header index 0.

Looks good. See inline comment and fix if you agree.

Mon, Dec 17, 9:00 AM

Dec 16 2018

clayborg requested changes to D55608: Make crashlog.py work or binaries with spaces in their names.
Dec 16 2018, 2:11 PM

Dec 14 2018

clayborg added a comment to D55727: Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used..

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 change, then run these commands and check the output?

Dec 14 2018, 4:03 PM
clayborg created D55727: Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used..
Dec 14 2018, 3:15 PM
clayborg requested changes to D55608: Make crashlog.py work or binaries with spaces in their names.
Dec 14 2018, 1:52 PM
clayborg added a comment to D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

Fixed the test inputs CMakeLists.txt file:

Dec 14 2018, 11:41 AM
clayborg accepted D55706: ELF: more section creation cleanup.

Looks good.

Dec 14 2018, 10:46 AM

Dec 13 2018

clayborg updated the diff for D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

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.

Dec 13 2018, 7:15 PM
clayborg requested changes to D55142: Minidump debugging using the native PDB reader.

Just need a way to verify symbols are good. See my inline comment.

Dec 13 2018, 1:39 PM · Restricted Project
clayborg added inline comments to D55631: Delay replacing the process till after we've finalized it.
Dec 13 2018, 9:41 AM

Dec 12 2018

clayborg accepted D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing.
Dec 12 2018, 1:45 PM
clayborg updated the summary of D55614: Fix MinidumpParser::GetFilteredModuleList() and test it.
Dec 12 2018, 1:37 PM
clayborg added a comment to D55614: Fix MinidumpParser::GetFilteredModuleList() and test it.

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.

Dec 12 2018, 1:37 PM
clayborg created D55614: Fix MinidumpParser::GetFilteredModuleList() and test it.
Dec 12 2018, 1:35 PM

Dec 11 2018

clayborg accepted D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext..

Looks good to me. Jim should ok as well.

Dec 11 2018, 2:18 PM
clayborg added a comment to D55356: Add a method to get the "base" file address of an object file.

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" message after a module gets added to a not-yet-running target. It tries to print the load address by first getting the base address and then converting that to a load address.

Before this patch, that would always fail, because well.. ELF and PECOFF had this function unimplemented, and for MachO the base address was section-relative, and so it wasn't resolved to a load address without the section being loaded. However, with this patch, in the ELF (and presumably PECOFF) case, the load address is not section-relative and so the GetLoadAddress function happily returns the address.

Is this the expected behavior here? (i.e., object_file->GetLoadAddress().GetLoadAddress(target) returning a valid value even though the target is not running)

Not unless someone has manually set the section load address in the test?

The test is not setting the load address manually. This simply falls out of how Address::GetLoadAddress is implemented:

addr_t Address::GetLoadAddress(Target *target) const {
  SectionSP section_sp(GetSection());
  if (section_sp) {
    ...
  } else {
    // We don't have a section so the offset is the load address
    return m_offset;
  }
}

So, where's the bug here? It's not clear to me how to change Address::GetLoadAddress to do something different than what it is doing now.

Dec 11 2018, 10:53 AM
clayborg added a comment to D55356: Add a method to get the "base" file address of an object file.

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" message after a module gets added to a not-yet-running target. It tries to print the load address by first getting the base address and then converting that to a load address.

Before this patch, that would always fail, because well.. ELF and PECOFF had this function unimplemented, and for MachO the base address was section-relative, and so it wasn't resolved to a load address without the section being loaded. However, with this patch, in the ELF (and presumably PECOFF) case, the load address is not section-relative and so the GetLoadAddress function happily returns the address.

Is this the expected behavior here? (i.e., object_file->GetLoadAddress().GetLoadAddress(target) returning a valid value even though the target is not running)

Not unless someone has manually set the section load address in the test?

Dec 11 2018, 9:53 AM
clayborg added a comment to D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

It just occurred to me that we have another copy of /proc/<pid>/maps -> MemoryRegionInfo conversion code. It lives in NativeProcessLinux.cpp (ParseMemoryRegionInfoFromProcMapsLine). It would be nice to extract this parser into a some place (Plugins/Process/Utility ?), where it can be shared by the two process plugins. As a bonus, that should make it easy to write unit tests for that function.

Dec 11 2018, 9:51 AM
clayborg added a comment to D55356: Add a method to get the "base" file address of an object file.

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" message after a module gets added to a not-yet-running target. It tries to print the load address by first getting the base address and then converting that to a load address.

Before this patch, that would always fail, because well.. ELF and PECOFF had this function unimplemented, and for MachO the base address was section-relative, and so it wasn't resolved to a load address without the section being loaded. However, with this patch, in the ELF (and presumably PECOFF) case, the load address is not section-relative and so the GetLoadAddress function happily returns the address.

Is this the expected behavior here? (i.e., object_file->GetLoadAddress().GetLoadAddress(target) returning a valid value even though the target is not running)

Dec 11 2018, 9:42 AM

Dec 10 2018

clayborg added a comment to D55142: Minidump debugging using the native PDB reader.

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 PlaceholderModule so you can see what I had done. Since these modules always must be associated with a target, I keep a weak pointer to the target in the constructor. Then later, if someone does "target symbols add ..." the module will have Module:: m_symfile_spec filled in, so I set the m_platform_file to be m_file, and then move m_symfile_spec into m_file and then call Module::GetObjectFile(). This is nice and clean because you don't have to make up fake symbols to fake sections. When you create the placeholder module it needs the target:

auto placeholder_module =
    std::make_shared<PlaceholderModule>(module_spec, GetTarget());
Dec 10 2018, 2:48 PM · Restricted Project
clayborg updated the diff for D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

Add reserve and shrink to fit when parsing memory regions.

Dec 10 2018, 1:19 PM
clayborg added a comment to D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

Don't you mind to take overridden GetMemoryRegions in this patch so that I can remove all minidump-related changes from D55472?

Dec 10 2018, 1:06 PM
clayborg added a comment to D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

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 other patch. I think that should be evaluated separately.

Dec 10 2018, 11:14 AM
clayborg added inline comments to D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..
Dec 10 2018, 11:13 AM
clayborg updated the diff for D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

Fixed Zach's issues.

Dec 10 2018, 11:12 AM
clayborg updated the diff for D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

Removed Xcode scheme changes.

Dec 10 2018, 11:12 AM
clayborg added inline comments to D55472: Speadup memory regions handling and cleanup related code.
Dec 10 2018, 10:40 AM · Restricted Project
clayborg added a comment to D55142: Minidump debugging using the native PDB reader.

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 ELF file or no symbol file (ELF/DWARF or breakpad).

Dec 10 2018, 10:32 AM · Restricted Project
clayborg added a comment to D55434: ObjectFileBreakpad: Implement sections.

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 sure would be nice to be able to load a bunch of them into LLDB without needing to find the original executable and just symbolicate no?

I could try to make it stand-alone, but that seems to me like a duplication of effort. And since the sections I could conjure up from the breakpad info would never match the original elf file, I would have to support both cases anyway, one with using the sections from the object file, and one with the own, made-up sections.

I do intend to support both cases, but in a slightly different way. The way, I see it we have actually four cases to consider:

  1. we have an stripped elf file, and a breakpad symbol file (the case of an unstripped elf file is uninteresting, as it will have much better debug info than breakpad can possibly provide)
  2. we don't have an elf file, but we have a breakpad file
  3. we don't have an elf file nor a breakpad file

    Because of case 3, we have to do some section conjuring independently of any breakpad file. We already do that to some extent, and @lemo is getting ready to extend that in D55142. Once we have that, and we find a breakpad file for this module (case 2), the breakpad file should be able to just latch onto the sections created for the placeholder object file. And I believe ProcessMinidump is in a better position to create the "placeholder" sections, as it has both access to the load addresses and sizes of the modules. From breakpad info, it would be hard to determine the size of the module if the highest address is occupied by a "PUBLIC" symbol, as they don't have any sizes associated with them.

    Going back to case 1, if we have a stripped elf file, the breakpad file should latch onto the sections of that one without ever knowing the difference. So in the end, I hope this will produce a clearer code because the concerns will be separated. Breakpad code will always deal with externally-provided sections, regardless of whether they come from a "real" object file, or a made-up one. And the "making-up" code can work independently of there being a breakpad file.
Dec 10 2018, 10:25 AM
clayborg updated the diff for D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..

Add sorting functions needed for MemoryRegionInfo

Dec 10 2018, 10:20 AM
clayborg added a comment to D55472: Speadup memory regions handling and cleanup related code.

See my patch: https://reviews.llvm.org/D55522

Dec 10 2018, 10:09 AM · Restricted Project
clayborg retitled D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available. from Cache memory regions and use the linux maps as the source of the information if available. to Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..
Dec 10 2018, 10:09 AM
clayborg created D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available..
Dec 10 2018, 10:09 AM
clayborg requested changes to D55472: Speadup memory regions handling and cleanup related code.

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.

Dec 10 2018, 9:52 AM · Restricted Project

Dec 7 2018

clayborg added a comment to D55434: ObjectFileBreakpad: Implement sections.

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 sure would be nice to be able to load a bunch of them into LLDB without needing to find the original executable and just symbolicate no?

Dec 7 2018, 1:55 PM
clayborg added a comment to D55434: ObjectFileBreakpad: Implement sections.

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 symbol file that just needs some sections that it can give to the debug info it will eventually create, these sections must be the same between the binary and the breakpad object file. So I would still recommend trying to make sections that make sense like a real object file. It is also nice to be able to live off of the breakpad file itself. In this case the ObjectFile for breakpad when it is stand alone should do as good of a job as possible.

Dec 7 2018, 1:53 PM