[PATCH v2 0/2] Help Linux perf get debug information from PE files​.

[Rémi Bernon]

This is an updated version of the perf patches, as I saw in some
discussion that "ifeq" should not be used in Makefiles for portability.

I believe it could be useful with only a few drawbacks (increased binary
size due to the alignment constraint, and the additional objcopy
requirement).

Rémi Bernon (2):
  makefiles: Align PE sections so they can be directly mmaped.
  makefiles: Split PE debug sections to separate files.

tools/makedep.c | 5 +++++
 1 file changed, 5 insertions(+)

--
2.23.0

[PATCH v2 1/2] makefiles: Align PE sections so they can be directly mmaped.

[Rémi Bernon]

This should help linux perf tool match the binary files on disk with the
code regions in memory.

Signed-off-by: Rémi Bernon 
---
 tools/makedep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/makedep.c b/tools/makedep.c
index 67dfea43bb3..6d74507661e 100644
--- a/tools/makedep.c
+++ b/tools/makedep.c
@@ -3247,6 +3247,7 @@ static void output_module( struct makefile *make )
     output_filenames_obj_dir( make, make->res_files );
     output_filenames( all_libs );
     output_filename( make->is_cross ? "$(CROSSLDFLAGS)" : "$(LDFLAGS)" );
+    output_filename( make->is_cross ? "-Wl,--file-alignment,4096" : "" );
     output( "\n" );

if (spec_file && make->importlib)

[PATCH v2 2/2] makefiles: Split PE debug sections to separate files.

[Rémi Bernon]

Linux perf tool cannot parse PE files for debug information, but it also
looks for external debug info alongside to the binaries it detects.

We can use this feature to provide DWARF information for PE files by
splitting the sections to a separate ELF that perf will understand.

Signed-off-by: Rémi Bernon 
---
 tools/makedep.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/makedep.c b/tools/makedep.c
index 6d74507661e..4b1d68f71f6 100644
--- a/tools/makedep.c
+++ b/tools/makedep.c
@@ -3189,6 +3189,7 @@ static void output_module( struct makefile *make )
     struct strarray dep_libs = empty_strarray;
     char *module_path = obj_dir_path( make, make->module );
     char *spec_file = NULL;
+    char const *bfd_format = strstr( crosstarget, "x86_64" ) ? "elf64-x86-64" : "elf32-i386";
     unsigned int i;

if (!make->is_exe) spec_file = src_dir_path( make, replace_extension( make->module, ".dll", ".spec" ));
@@ -3249,6 +3250,9 @@ static void output_module( struct makefile *make )
     output_filename( make->is_cross ? "$(CROSSLDFLAGS)" : "$(LDFLAGS)" );
     output_filename( make->is_cross ? "-Wl,--file-alignment,4096" : "" );
     output( "\n" );
+    if (make->is_cross && !make->is_win16)
+        output( "\tmkdir -p $(@D)/.debug && %s-objcopy -O %s --only-keep-debug"
+                " --file-alignment=4096 $@ $(@D)/.debug/$(@F)\n", crosstarget, bfd_format );

if (spec_file && make->importlib)
     {

[PATCH v2 2/2] makefiles: Split PE debug sections to separate files.

[Chip Davis]

October 2, 2019 1:39 PM, "Rémi Bernon"  wrote:
> diff --git a/tools/makedep.c b/tools/makedep.c
> index 6d74507661e..4b1d68f71f6 100644
> --- a/tools/makedep.c
> +++ b/tools/makedep.c
> @@ -3189,6 +3189,7 @@ static void output_module( struct makefile *make )
>      struct strarray dep_libs = empty_strarray;
>      char *module_path = obj_dir_path( make, make->module );
>      char *spec_file = NULL;
> -
>
> char const *bfd_format = strstr( crosstarget, "x86_64" ) ? "elf64-x86-64" : "elf32-i386";
> unsigned int i;
>
> if (!make->is_exe) spec_file = src_dir_path( make, replace_extension( make->module, ".dll", ".spec"));
>
> @@ -3249,6 +3250,9 @@ static void output_module( struct makefile *make )
>      output_filename( make->is_cross ? "$(CROSSLDFLAGS)" : "$(LDFLAGS)" );
>      output_filename( make->is_cross ? "-Wl,--file-alignment,4096" : "" );
>      output( "\n" );
> - if (make->is_cross && !make->is_win16)
> -
>    output( "\tmkdir -p $(@D)/.debug && %s-objcopy -O %s --only-keep-debug"
> -
>            " --file-alignment=4096 $@ $(@D)/.debug/$(@F)\n", crosstarget, bfd_format );

We really don't want to do this on Mac OS.

Chip

[PATCH v2 2/2] makefiles: Split PE debug sections to separate files.

[Rémi Bernon]

On 10/2/19 8:53 PM, Chip Davis wrote:
> October 2, 2019 1:39 PM, "Rémi Bernon"  wrote:
> > diff --git a/tools/makedep.c b/tools/makedep.c
> > index 6d74507661e..4b1d68f71f6 100644
> > --- a/tools/makedep.c
> > +++ b/tools/makedep.c
> > @@ -3189,6 +3189,7 @@ static void output_module( struct makefile *make )
> >       struct strarray dep_libs = empty_strarray;
> >       char *module_path = obj_dir_path( make, make->module );
> >       char *spec_file = NULL;
> > -
> >
> > char const *bfd_format = strstr( crosstarget, "x86_64" ) ? "elf64-x86-64" : "elf32-i386";
> >  unsigned int i;
> >
> > if (!make->is_exe) spec_file = src_dir_path( make, replace_extension( make->module, ".dll", ".spec"));
> >
> > @@ -3249,6 +3250,9 @@ static void output_module( struct makefile *make )
> >       output_filename( make->is_cross ? "$(CROSSLDFLAGS)" : "$(LDFLAGS)" );
> >       output_filename( make->is_cross ? "-Wl,--file-alignment,4096" : "" );
> >       output( "\n" );
> > - if (make->is_cross && !make->is_win16)
> > -
> >    output( "\tmkdir -p $(@D)/.debug && %s-objcopy -O %s --only-keep-debug"
> > -
> >            " --file-alignment=4096 $@ $(@D)/.debug/$(@F)\n", crosstarget, bfd_format );
>
> We really don't want to do this on Mac OS.
>
> Chip

Sure, it's not useful on MacOS but is it an issue? Or is it because of 
mkdir?

[PATCH v2 2/2] makefiles: Split PE debug sections to separate files.

[Chip Davis]

October 3, 2019 3:29 AM, "Rémi Bernon"  wrote:
> On 10/2/19 8:53 PM, Chip Davis wrote:
> > October 2, 2019 1:39 PM, "Rémi Bernon"  wrote:
> > diff --git a/tools/makedep.c b/tools/makedep.c
> > > index 6d74507661e..4b1d68f71f6 100644
> > > --- a/tools/makedep.c
> > > +++ b/tools/makedep.c
> > > @@ -3189,6 +3189,7 @@ static void output_module( struct makefile *make )
> > > struct strarray dep_libs = empty_strarray;
> > > char *module_path = obj_dir_path( make, make->module );
> > > char *spec_file = NULL;
> > > - char const *bfd_format = strstr( crosstarget, "x86_64" ) ? "elf64-x86-64" : "elf32-i386";
> > >
> > > unsigned int i;
> > >
> > > if (!make->is_exe) spec_file = src_dir_path( make, replace_extension( make->module, ".dll",
> > > ".spec"));
> > > @@ -3249,6 +3250,9 @@ static void output_module( struct makefile *make )
> > > output_filename( make->is_cross ? "$(CROSSLDFLAGS)" : "$(LDFLAGS)" );
> > > output_filename( make->is_cross ? "-Wl,--file-alignment,4096" : "" );
> > > output( "\n" );
> > > - if (make->is_cross && !make->is_win16)
> > > - output( "\tmkdir -p $(@D)/.debug && %s-objcopy -O %s --only-keep-debug"
> > > - " --file-alignment=4096 $@ $(@D)/.debug/$(@F)\n", crosstarget, bfd_format );
> >
> > We really don't want to do this on Mac OS.
>
> Sure, it's not useful on MacOS but is it an issue? Or is it because of mkdir?

It's because of objcopy, which may not be available--and the fact that the Mac OS native binary format is Mach-O, not ELF.

Chip

[PATCH v2 2/2] makefiles: Split PE debug sections to separate files.

[Rémi Bernon]

On 10/3/19 5:53 PM, Chip Davis wrote:
> October 3, 2019 3:29 AM, "Rémi Bernon"  wrote:
> > On 10/2/19 8:53 PM, Chip Davis wrote:
> > > October 2, 2019 1:39 PM, "Rémi Bernon"  wrote:
> > > diff --git a/tools/makedep.c b/tools/makedep.c
> > > > index 6d74507661e..4b1d68f71f6 100644
> > > > --- a/tools/makedep.c
> > > > +++ b/tools/makedep.c
> > > > @@ -3189,6 +3189,7 @@ static void output_module( struct makefile *make )
> > > > struct strarray dep_libs = empty_strarray;
> > > > char *module_path = obj_dir_path( make, make->module );
> > > > char *spec_file = NULL;
> > > > - char const *bfd_format = strstr( crosstarget, "x86_64" ) ? "elf64-x86-64" : "elf32-i386";
> > > >
> > > > unsigned int i;
> > > >
> > > > if (!make->is_exe) spec_file = src_dir_path( make, replace_extension( make->module, ".dll",
> > > > ".spec"));
> > > > @@ -3249,6 +3250,9 @@ static void output_module( struct makefile *make )
> > > > output_filename( make->is_cross ? "$(CROSSLDFLAGS)" : "$(LDFLAGS)" );
> > > > output_filename( make->is_cross ? "-Wl,--file-alignment,4096" : "" );
> > > > output( "\n" );
> > > > - if (make->is_cross && !make->is_win16)
> > > > - output( "\tmkdir -p $(@D)/.debug && %s-objcopy -O %s --only-keep-debug"
> > > > - " --file-alignment=4096 $@ $(@D)/.debug/$(@F)\n", crosstarget, bfd_format );
> > >
> > > We really don't want to do this on Mac OS.
> >
> > Sure, it's not useful on MacOS but is it an issue? Or is it because of mkdir?
>
> It's because of objcopy, which may not be available--and the fact that the Mac OS native binary format is Mach-O, not ELF.
>
> Chip

Here objcopy is prefixed by crosstarget and it should be the one 
provided with MinGW binutils, as %s-ranlib or %s-ar in the 
output_static_lib function.

Of course, it still may not be available and it could be checked during 
configure - and the other tools maybe too.

[Jacek Caban]

Hi Rémi,

On 10/2/19 8:26 PM, Rémi Bernon wrote:
> This is an updated version of the perf patches, as I saw in some
> discussion that "ifeq" should not be used in Makefiles for portability.

I think it may be useful to put two more semi-related things for the 
consideration. Currently, strip breaks Wine builtin DLLs by rewriting PE 
headers and losing Wine mark (see bug 47718). Usage of strip would need 
to be followed by restoring builtin mark. One way to handle that would 
be to produce binaries that don't need to be stripped later (by make 
install or whatever). Once we move debug info to the separate file, we 
could strip original DLLs.

I experimented with llvm-mingw, which can produce PDB files. It may be 
handy if you want to use Windows tools, so I think we will want to 
support that as an option. PDBs are generated by linker during binary 
linking, so it would need makefiles support at some point anyway.

Taking both above into consideration, I think that the logic would 
better fit winegcc. winegcc could have an additional argument for 
passing output symbol file. When used, it could invoke objcopy and strip 
as needed, before invoking make_wine_builtin(). It would solve both your 
problem and problem with strip. In the future, PDB build would just pass 
-Wl,-pdb= linker argument instead.

Thanks,

Jacek