[PATCH 1/3] makedep: Align PE sections so they can be mmapped.

[Jacek Caban]

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