- User Since
- Sep 23 2014, 10:11 AM (225 w, 16 h)
Ok, thanks for explaining. Your reasoning makes sense.
Might be good to have a test where we have a relative DW_AT_name and not DW_AT_comp_dir? Just in case?
Looks good to me
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
Mon, Jan 14
Thu, Jan 10
This is fine to start.
All the change to the symbol vendor make sense, but it seems like all of the call sites should be:
DW_AT_comp_dir isn't enough. See inline suggestions.
Wed, Jan 9
But if everyone else thinks differently, I'll go with the majority. Just my initial thoughts.
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
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.
Tue, Jan 8
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).
Second vote from me to switch to a std::vector since we have a function get breakpoint at index. Other than that, looks good.
Fri, Jan 4
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.
You have convinced me! Sorry I had paged out the original intent you conveyed from before the break. Thanks for the details.
Thu, Jan 3
I have a minidump generator if you need me to make any specific minidump files for you.
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.
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
excited to see this starting as well!
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?
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.
Wed, Jan 2
Fri, Dec 21
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?)
Wed, Dec 19
Just remove the "regions-linux-map.dmp" as I already checked one in with rLLDB349658 and this is good to go
One quick change required to avoid STL in public headers.
Tue, Dec 18
See inline comments and let me know what you think.
Change looks good.
Mon, Dec 17
Fixed all requested changes. Added a complete test in lit.
nice! A lot of great commands in heap.py. Like see them fixed and improved
It would be nice to see context when you submit diffs. with SVN:
svn diff -x -U9999999 ...
git diff -U9999999
Looks good. See inline comment and fix if you agree.
Dec 16 2018
Dec 14 2018
Fixed the test inputs CMakeLists.txt file:
Dec 13 2018
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.
Just need a way to verify symbols are good. See my inline comment.
Dec 12 2018
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 11 2018
Looks good to me. Jim should ok as well.
Dec 10 2018
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());
Add reserve and shrink to fit when parsing memory regions.
Fixed Zach's issues.
Removed Xcode scheme changes.
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).
Add sorting functions needed for MemoryRegionInfo
See my patch: https://reviews.llvm.org/D55522
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 7 2018
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?
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.