[fuchsia] Make profile processing resilient to bogus overlaps from dladdr.

[fuchsia] Make profile processing resilient to bogus overlaps from dladdr.

R=zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/08b7b1d06014784edf0e844e488eebcce1537f48

[rmacnak, 2017-06-13 23:04:10]

Description was changed from

==========
[fuchsia] Make profile processing resilent to bogus overlaps from dladdr.
==========

to

==========
[fuchsia] Make profile processing resilient to bogus overlaps from dladdr.
==========

[rmacnak, 2017-06-14 00:40:14]

rmacnak@google.com changed reviewers:
+ zra@google.com

[zra, 2017-06-14 05:00:06]

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:721: class ProfileCodeTable : public
ZoneAllocated {
Could you add unit tests for this class? If you move the declaration of this
class to the header file, you can add unit tests for it in profiler_test.cc.

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:738: return -1;  // No found.
Not

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:799: // (dladdr doesn't give us symbol size so we
may new PCs we didn't
may see new PCs

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:809: // (dladdr doesn't give us symbol size so we
may new PCs we didn't
may see new PCs

[rmacnak, 2017-06-14 17:32:14]

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:721: class ProfileCodeTable : public
ZoneAllocated {
On 2017/06/14 05:00:05, zra wrote:
> Could you add unit tests for this class? If you move the declaration of this
> class to the header file, you can add unit tests for it in profiler_test.cc.

Done. Adjusted neighbor test to compare end instead of start to avoid a
degenerate truncation.

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:738: return -1;  // No found.
On 2017/06/14 05:00:05, zra wrote:
> Not

Done.

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:799: // (dladdr doesn't give us symbol size so we
may new PCs we didn't
On 2017/06/14 05:00:05, zra wrote:
> may see new PCs

Done.

https://codereview.chromium.org/2939853002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:809: // (dladdr doesn't give us symbol size so we
may new PCs we didn't
On 2017/06/14 05:00:05, zra wrote:
> may see new PCs

Done.

[zra, 2017-06-14 17:49:22]

lgtm

https://codereview.chromium.org/2939853002/diff/60001/runtime/vm/profiler_tes...
File runtime/vm/profiler_test.cc (right):

https://codereview.chromium.org/2939853002/diff/60001/runtime/vm/profiler_tes...
runtime/vm/profiler_test.cc:2826: EXPECT_EQ(table->FindCodeForPC(50), code1);
Maybe try intermediate and end values here and below as well.

[rmacnak, 2017-06-14 18:14:08]

https://codereview.chromium.org/2939853002/diff/60001/runtime/vm/profiler_tes...
File runtime/vm/profiler_test.cc (right):

https://codereview.chromium.org/2939853002/diff/60001/runtime/vm/profiler_tes...
runtime/vm/profiler_test.cc:2826: EXPECT_EQ(table->FindCodeForPC(50), code1);
On 2017/06/14 17:49:22, zra wrote:
> Maybe try intermediate and end values here and below as well.

Added

[rmacnak, 2017-06-14 18:14:29]

Description was changed from

==========
[fuchsia] Make profile processing resilient to bogus overlaps from dladdr.
==========

to

==========
[fuchsia] Make profile processing resilient to bogus overlaps from dladdr.

R=zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/08b7b1d06014784edf0e844e488eebcce1537f48
==========

[rmacnak, 2017-06-14 18:14:31]

Committed patchset #5 (id:80001) manually as
08b7b1d06014784edf0e844e488eebcce1537f48 (presubmit successful).