From: Nicholas Nethercote <nnethercote@mozilla.com>
Date: Tue, 1 Dec 2020 02:46:25 +0000 (+1100)
Subject: Fix an obscure problem with peak finding.
X-Git-Tag: VALGRIND_3_17_0~106
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=48ba17f87d07842f4b8ec254f2d2b4e060e6fe51;p=thirdparty%2Fvalgrind.git

Fix an obscure problem with peak finding.

Currently, if there are multiple equal global peaks, `intro_Block` and
`resize_Block` record the first one while `check_for_peak` records the
last one. This could lead to inconsistent output, though it's unlikely
in practice.

This commit fixes things so that all functions record the last peak.
---

diff --git a/dhat/dh_main.c b/dhat/dh_main.c
index 565b0ae2d8..5d1e89b592 100644
--- a/dhat/dh_main.c
+++ b/dhat/dh_main.c
@@ -287,7 +287,10 @@ static void intro_Block ( Block* bk )
 
    g_curr_blocks++;
    g_curr_bytes += bk->req_szB;
-   if (g_curr_bytes > g_max_bytes) {
+
+   // The use of `>=` rather than `>` means that if there are multiple equal
+   // peaks we record the latest one, like `check_for_peak` does.
+   if (g_curr_bytes >= g_max_bytes) {
       g_max_blocks = g_curr_blocks;
       g_max_bytes = g_curr_bytes;
       g_max_instrs = g_curr_instrs;
@@ -300,7 +303,10 @@ static void intro_Block ( Block* bk )
 
    api->curr_blocks++;
    api->curr_bytes += bk->req_szB;
-   if (api->curr_bytes > api->max_bytes) {
+
+   // The use of `>=` rather than `>` means that if there are multiple equal
+   // peaks we record the latest one, like `check_for_peak` does.
+   if (api->curr_bytes >= api->max_bytes) {
       api->max_blocks = api->curr_blocks;
       api->max_bytes  = api->curr_bytes;
    }
@@ -469,7 +475,10 @@ static void resize_Block(ExeContext* ec, SizeT old_req_szB, SizeT new_req_szB)
 
    g_curr_blocks += 0;  // unchanged
    g_curr_bytes += delta;
-   if (g_curr_bytes > g_max_bytes) {
+
+   // The use of `>=` rather than `>` means that if there are multiple equal
+   // peaks we record the latest one, like `check_for_peak` does.
+   if (g_curr_bytes >= g_max_bytes) {
       g_max_blocks = g_curr_blocks;
       g_max_bytes = g_curr_bytes;
       g_max_instrs = g_curr_instrs;
@@ -482,7 +491,10 @@ static void resize_Block(ExeContext* ec, SizeT old_req_szB, SizeT new_req_szB)
 
    api->curr_blocks += 0;  // unchanged
    api->curr_bytes += delta;
-   if (api->curr_bytes > api->max_bytes) {
+
+   // The use of `>=` rather than `>` means that if there are multiple equal
+   // peaks we record the latest one, like `check_for_peak` does.
+   if (api->curr_bytes >= api->max_bytes) {
       api->max_blocks = api->curr_blocks;
       api->max_bytes  = api->curr_bytes;
    }