On 27.05.2020 13:49, Rémi Bernon wrote:
On 5/25/20 5:26 PM, Jacek Caban wrote:
On 25.05.2020 16:32, Rémi Bernon wrote:
I could also see that some CROSSDEBUG checks look for "split" prefix in makedep, and some other set the variable to dwarf. It's not clear to me what this variable is supposed to be set to (other than "pdb"). Does "dwarf" mean separate file as well?
No, "dwarf" means that debug info will be embedded in module PE files, which has always been the default. "dwarf" part of "split-dwarf" is important for configure script to apply dwarf-specific flags. "dwarf," "split-dwarf" and "pdb" are the only configs for which I know a real use, but in theory you could set it to anything else as long as you take care of CROSSCFLAGS yourself. "stabs" and "split-stabs" should be in theory achievable this way (but I didn't try).
Jacek
Alright. So, "split*" means that the debug sections are extracted to a separate file, and, currently, this file is also a PE file, as the default objcopy operation is to use the same output format as the source file.
Having the debug section in an ELF file instead in this case shouldn't be an issue right? Gdb and any other tool that were already able to parse DWARF from PE files should be able to parse DWARF from ELF without trouble. Using an ELF container allows perf to read it. I don't think there's anything that requires PE container with DWARF debug info, but maybe there is?
Regarding the default name and location of this ELF module, I see that we already implement in dbghelp the same logic as gdb does, which includes looking for .debug/debugfile, where debugfile is the contents of GNU debuglink, so this should work fine as well, while at the same time making perf work OOTB.
From a quick test, as long as the .debug folder is also copied alongside the dll in the prefix, both winedbg and winedbg --gdb are happy with the ELF container.
So, in the end, although I understand the root issue is some limitation of perf itself, this feels much simpler to fix in Wine with these small tweaks that makes every tool happy, than to try implementing PE support in perf. I think it would require much more work there, and would be way harder to justify if it were to be upstreamed (this is clearly a Wine specific use-case and I'm not sure how eager perf maintainers would be to have huge code changes just for the sake of PE module parsing).
Yes, I agree that it could all work, I even made sure that dbghelp.dll can handle it when I was adding support for debug links in PE. I'm not against (at least part) of the patchset. But the thing is that it's quite a lot of perf-specific tweaking. Even if we take those patches, it would still be interesting to be better supported by perf, so why not at least evaluate it first.
I took a quick look at perf sources and I'm not convinced that it would be hard. It already has some abstraction, see dso_binary_type enum and its usage. It already has special cases for things like DSO_BINARY_TYPE__JAVA_JIT, so DSO_BINARY_TYPE__PE doesn't seem out of place. The amount of PE understanding that perf would need to find debug info is very minimal: it just needs to be able to enumerate sections.
The above is about two other patches. I didn't look at PE alignment fix, but I just expect that it should be fixable upstream. With that in mind, my comments to patches:
- PE alignment
The patch breaks build with llvm-mingw because lld doesn't support --file-alignment. It's also not exactly free. Note that you could just pass CROSSLDFLAGS=... to configure instead of patching Wine.
- .debug directory
I don't have a strong opinion here. If it's indeed a standard, maybe we should follow it. GDB documentation mentions both %s.debug and .debug/%s and, not being a fan of hidden directories in build tree, I chose the one that seemed nicer:
https://sourceware.org/gdb/current/onlinedocs/gdb/Separate-Debug-Files.html
- symbols in ELF file
We could do that, creating an exotic mix then building on mac, with PE, Mach-O and ELF files. Another option would be CROSSLDFLAGS=-Wl,--objcopy,-Oelf64-x86-64, but it doesn't seem nice...
Thanks,
Jacek