Disable clank relocation packer when profiling is turned on.

Disable clank relocation packer when profiling is turned on.

When packing is enabled, the profiler can no longer properly map the
symbols which results in incorrect callstacks. Packing should be
disable when profiling is turned on.

R=vmiura@chromium.org
BUG=None
TEST=trybots

Committed: https://crrev.com/5adad7f853b67ce4d10a1c04e3d132566d3ebc1a
Cr-Commit-Position: refs/heads/master@{#299151}

[David Yen, 2014-10-10 17:04:34]

On 2014/10/10 17:03:23, David Yen wrote:

CC anyone you think is necessary.

I am not too sure about the formatting, I couldn't find another example of a
long condition like this one.

[vmiura, 2014-10-10 17:50:33]

vmiura@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[vmiura, 2014-10-10 17:50:33]

rmcilroy@: please review this patch.  Thanks.

[rmcilroy, 2014-10-10 18:22:55]

rmcilroy@chromium.org changed reviewers:
+ simonb@chromium.org

[rmcilroy, 2014-10-10 18:22:55]

I take it profiling is only used locally when doing perf measurements. If so
this seems fine - lgtm.

Adding simomb as FYI.

[David Yen, 2014-10-10 18:29:46]

The CQ bit was checked by dyen@chromium.org

[commit-bot: I haz the power, 2014-10-10 18:30:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/641373005/1

[commit-bot: I haz the power, 2014-10-10 18:54:23]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-10-10 18:55:01]

Patchset 1 (id:??) landed as
https://crrev.com/5adad7f853b67ce4d10a1c04e3d132566d3ebc1a
Cr-Commit-Position: refs/heads/master@{#299151}

[simonb (inactive), 2014-10-13 17:01:43]

Relocation packing splits a single executable load segment into two, and this
renders faulty some assumptions in several tools that parse the process memory
map.  Eg, Breakpad and Android libbacktrace:

https://bugzilla.mozilla.org/show_bug.cgi?id=637316
https://breakpad.appspot.com/7714003
https://code.google.com/p/android/issues/detail?id=76717

David, can you try with relocation packing turned back on and with a patched
pprof?  The following diff for gperftools-2.2.1/src/pprof worked for me in
simple (non-Chromium) testing.  If this solves the issue for you then I will
file a bug report against pprof.  Thanks.


$ diff -c ./gperftools-2.2.1/src/pprof ./pprof
*** ./gperftools-2.2.1/src/pprof	2014-04-13 02:35:40.000000000 +0100
--- ./pprof	2014-10-13 16:48:37.039256051 +0100
***************
*** 4352,4357 ****
--- 4352,4358 ----
    my $zero_offset = HexExtend("0");
  
    my $buildvar = "";
+   my $priorlib = "";
    foreach my $l (split("\n", $map)) {
      if ($l =~ m/^\s*build=(.*)$/) {
        $buildvar = $1;
***************
*** 4408,4414 ****
--- 4409,4424 ----
        }
      }
  
+     # If we find multiple executable segments for a single library, merge them
+     # into a single entry that spans the complete address range.
+     if ($lib eq $priorlib) {
+       my $prior = pop(@{$result});
+       $start = @$prior[1];
+       # TODO $offset may be wrong if .text is not in the final segment.
+     }
+ 
      push(@{$result}, [$lib, $start, $finish, $offset]);
+     $priorlib = $lib;
    }
  
    # Append special entry for additional library (not relocated)

[David Yen, 2014-10-29 21:37:21]

On 2014/10/13 17:01:43, simonb wrote:
> Relocation packing splits a single executable load segment into two, and this
> renders faulty some assumptions in several tools that parse the process memory
> map.  Eg, Breakpad and Android libbacktrace:
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=637316
> https://breakpad.appspot.com/7714003
> https://code.google.com/p/android/issues/detail?id=76717
> 
> David, can you try with relocation packing turned back on and with a patched
> pprof?  The following diff for gperftools-2.2.1/src/pprof worked for me in
> simple (non-Chromium) testing.  If this solves the issue for you then I will
> file a bug report against pprof.  Thanks.
> 
> 
> $ diff -c ./gperftools-2.2.1/src/pprof ./pprof
> *** ./gperftools-2.2.1/src/pprof	2014-04-13 02:35:40.000000000 +0100
> --- ./pprof	2014-10-13 16:48:37.039256051 +0100
> ***************
> *** 4352,4357 ****
> --- 4352,4358 ----
>     my $zero_offset = HexExtend("0");
>   
>     my $buildvar = "";
> +   my $priorlib = "";
>     foreach my $l (split("\n", $map)) {
>       if ($l =~ m/^\s*build=(.*)$/) {
>         $buildvar = $1;
> ***************
> *** 4408,4414 ****
> --- 4409,4424 ----
>         }
>       }
>   
> +     # If we find multiple executable segments for a single library, merge
them
> +     # into a single entry that spans the complete address range.
> +     if ($lib eq $priorlib) {
> +       my $prior = pop(@{$result});
> +       $start = @$prior[1];
> +       # TODO $offset may be wrong if .text is not in the final segment.
> +     }
> + 
>       push(@{$result}, [$lib, $start, $finish, $offset]);
> +     $priorlib = $lib;
>     }
>   
>     # Append special entry for additional library (not relocated)

Hi Simon, I am not using pprof so I don't think this is a related problem.

[simonb (inactive), 2014-10-30 18:19:58]

On 2014/10/29 21:37:21, David Yen wrote:
> On 2014/10/13 17:01:43, simonb wrote:
> > Relocation packing splits a single executable load segment into two, and
this
> > renders faulty some assumptions in several tools that parse the process
memory
> > map.  Eg, Breakpad and Android libbacktrace:
> > 
> > https://bugzilla.mozilla.org/show_bug.cgi?id=637316
> > https://breakpad.appspot.com/7714003
> > https://code.google.com/p/android/issues/detail?id=76717
> > 
> > David, can you try with relocation packing turned back on and with a patched
> > pprof?  The following diff for gperftools-2.2.1/src/pprof worked for me in
> > simple (non-Chromium) testing.  If this solves the issue for you then I will
> > file a bug report against pprof.  Thanks.
> > ...
> 
> Hi Simon, I am not using pprof so I don't think this is a related problem.

Can you say how you are profiling, then?  This page:

http://www.chromium.org/developers/profiling-chromium-and-webkit

talks about using 'pprof' after setting profiling=1.

[vmiura, 2014-10-30 21:16:50]

On 2014/10/30 18:19:58, simonb wrote:
> On 2014/10/29 21:37:21, David Yen wrote:
> > On 2014/10/13 17:01:43, simonb wrote:
> > > Relocation packing splits a single executable load segment into two, and
> this
> > > renders faulty some assumptions in several tools that parse the process
> memory
> > > map.  Eg, Breakpad and Android libbacktrace:
> > > 
> > > https://bugzilla.mozilla.org/show_bug.cgi?id=637316
> > > https://breakpad.appspot.com/7714003
> > > https://code.google.com/p/android/issues/detail?id=76717
> > > 
> > > David, can you try with relocation packing turned back on and with a
patched
> > > pprof?  The following diff for gperftools-2.2.1/src/pprof worked for me in
> > > simple (non-Chromium) testing.  If this solves the issue for you then I
will
> > > file a bug report against pprof.  Thanks.
> > > ...
> > 
> > Hi Simon, I am not using pprof so I don't think this is a related problem.
> 
> Can you say how you are profiling, then?  This page:
> 
> http://www.chromium.org/developers/profiling-chromium-and-webkit
> 
> talks about using 'pprof' after setting profiling=1.

David's fix relates to use of 'perf', also on that wiki page although it looks
like that could use some better documentation.

Android 'perf' is also used as part of the "adb_profile_chrome --perf" command:

 
http://www.chromium.org/developers/how-tos/trace-event-profiling-tool/recordi...