From: Paul Floyd Date: Fri, 21 Apr 2023 19:21:23 +0000 (+0200) Subject: Bug 464103 - Enhancement: add a client request to DHAT to mark memory to be histogrammed X-Git-Tag: VALGRIND_3_21_0~26 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=424340403c5d9eab23e883f6d27f780243bf8435;p=thirdparty%2Fvalgrind.git Bug 464103 - Enhancement: add a client request to DHAT to mark memory to be histogrammed --- diff --git a/.gitignore b/.gitignore index 9e26e2fbf5..71f1828d66 100644 --- a/.gitignore +++ b/.gitignore @@ -298,6 +298,7 @@ /dhat/tests/empty /dhat/tests/sig /dhat/tests/single +/dhat/tests/user_histo1 # /docs/ /docs/FAQ.txt diff --git a/NEWS b/NEWS index 50f171cd58..373de5ef1a 100644 --- a/NEWS +++ b/NEWS @@ -167,6 +167,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 460356 s390: Sqrt32Fx4 -- cannot reduce tree 462830 WARNING: unhandled amd64-freebsd syscall: 474 463027 broken check for MPX instruction support in assembler +464103 Enhancement: add a client request to DHAT to mark memory to be histogrammed 464476 Firefox fails to start under Valgrind 464609 Valgrind memcheck should support Linux pidfd_open 464680 Show issues caused by memory policies like selinux deny_execmem diff --git a/dhat/dh_main.c b/dhat/dh_main.c index 57d94237c5..ac0a7478e9 100644 --- a/dhat/dh_main.c +++ b/dhat/dh_main.c @@ -45,6 +45,7 @@ #include "dhat.h" #define HISTOGRAM_SIZE_LIMIT 1024 +#define USER_HISTOGRAM_SIZE_LIMIT 25*HISTOGRAM_SIZE_LIMIT //------------------------------------------------------------// //--- Globals ---// @@ -1229,6 +1230,52 @@ static Bool dh_handle_client_request(ThreadId tid, UWord* arg, UWord* ret) return True; } + case VG_USERREQ__DHAT_HISTOGRAM_MEMORY: { + Addr address = (Addr)arg[1]; + UWord initial_count = arg[2]; + + Block* bk = find_Block_containing( address ); + // bogus address + if (!bk) { + VG_(message)( + Vg_UserMsg, + "Warning: address for user histogram request not found %llx\n", (ULong)address + ); + return False; + } + + // already histogrammed + if (bk->req_szB <= HISTOGRAM_SIZE_LIMIT) { + VG_(message)( + Vg_UserMsg, + "Warning: request for user histogram of size %lu is smaller than the normal histogram limit, request ignored\n", + bk->req_szB + ); + return False; + } + + // already histogrammed + if (bk->req_szB > USER_HISTOGRAM_SIZE_LIMIT) { + VG_(message)( + Vg_UserMsg, + "Warning: request for user histogram of size %lu is larger than the maximum user request limit, request ignored\n", + bk->req_szB + ); + return False; + } + + + bk->histoW = VG_(malloc)("dh.new_block.3", bk->req_szB * sizeof(UShort)); + if (initial_count == 0U) { + VG_(memset)(bk->histoW, 0, bk->req_szB * sizeof(UShort)); + } else { + for (SizeT i = 0U; i < bk->req_szB; ++i) { + bk->histoW[i] = 1U; + } + } + return True; + } + case _VG_USERREQ__DHAT_COPY: { SizeT len = (SizeT)arg[1]; diff --git a/dhat/dhat.h b/dhat/dhat.h index 653ed417b2..cbedbe88b6 100644 --- a/dhat/dhat.h +++ b/dhat/dhat.h @@ -56,11 +56,16 @@ ---------------------------------------------------------------- */ +#if !defined(VALGRIND_DHAT_H) +#define VALGRIND_DHAT_H + #include "valgrind.h" typedef enum { VG_USERREQ__DHAT_AD_HOC_EVENT = VG_USERREQ_TOOL_BASE('D', 'H'), + VG_USERREQ__DHAT_HISTOGRAM_MEMORY, + VG_USERREQ__DHAT_HISTOGRAM_ARRAY, // This is just for DHAT's internal use. Don't use it. _VG_USERREQ__DHAT_COPY = VG_USERREQ_TOOL_BASE('D','H') + 256 @@ -73,3 +78,21 @@ typedef VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__DHAT_AD_HOC_EVENT, \ (_qzz_weight), 0, 0, 0, 0) +// for limited histograms of memory larger than 1k +#define DHAT_HISTOGRAM_MEMORY(_qzz_address, _qzz_initial_count) \ + VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__DHAT_HISTOGRAM_MEMORY, \ + (_qzz_address), (_qzz_initial_count), 0, 0, 0) + +// convenience macro for DHAT_HISTOGRAM_MEMORY +// for initialized memory (calloc, std::vector with initialization) +#define DHAT_HISTOGRAM_MEMORY_INIT(_qzz_address) \ + DHAT_HISTOGRAM_MEMORY(_qzz_address, 1U) + +// convenience macro for DHAT_HISTOGRAM_MEMORY +// for uninitialized memory (malloc, std::vector without initialization) +#define DHAT_HISTOGRAM_MEMORY_UNINIT(_qzz_address) \ + DHAT_HISTOGRAM_MEMORY(_qzz_address, 0U) + + +#endif + diff --git a/dhat/docs/dh-manual.xml b/dhat/docs/dh-manual.xml index 17aa9ab61c..9d99b4f352 100644 --- a/dhat/docs/dh-manual.xml +++ b/dhat/docs/dh-manual.xml @@ -422,6 +422,30 @@ this example) means "exceeded the maximum tracked count". probably have four separate byte-sized fields, followed by a four-byte field, and so on. +The size of the blocks that measure and display access counts is limited +to 1024 bytes. This is done to limit the performance overhead and also to keep +the size of the generated output reasonable. However, it is possible to override +this limit using client requests. The use-case for this is to first run DHAT +normally, and then identify any large blocks that you would like to further +investigate with access histograms. The client requests are declared in +dhat/dhat.h. There are two versions, DHAT_HISTOGRAM_MEMORY_INIT and +DHAT_HISTOGRAM_MEMORY_UNINIT. The +UNINIT version should be used with allocators that return uninitialized +memory (like malloc and new +without a constructor). The INIT version should be used with allocators +that initialize memory (like calloc and new +with a constructor). The macros should be placed immediately after the +call to the allocator, and use the pointer returned by the allocator. + + + +The memory that can be profiled in this way with user requests +has a further upper limit of 25kbytes. + Access counts can be useful for identifying data alignment holes or other layout inefficiencies. diff --git a/dhat/tests/Makefile.am b/dhat/tests/Makefile.am index b86fc416d4..e504b46551 100644 --- a/dhat/tests/Makefile.am +++ b/dhat/tests/Makefile.am @@ -11,7 +11,8 @@ EXTRA_DIST = \ copy.stderr.exp copy.vgtest \ empty.stderr.exp empty.vgtest \ sig.stderr.exp sig.vgtest \ - single.stderr.exp single.vgtest + single.stderr.exp single.vgtest \ + user_histo1.stderr.exp user_histo1.vgtest check_PROGRAMS = \ acc \ @@ -21,7 +22,8 @@ check_PROGRAMS = \ copy \ empty \ sig \ - single + single \ + user_histo1 AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) @@ -32,3 +34,5 @@ big_CFLAGS = $(AM_CFLAGS) -Wno-unused-result # Prevent the copying functions from being inlined copy_CFLAGS = $(AM_CFLAGS) -fno-builtin + +user_histo1_SOURCES = user_histo1.cpp diff --git a/dhat/tests/user_histo1.cpp b/dhat/tests/user_histo1.cpp new file mode 100644 index 0000000000..f5c8e69bbc --- /dev/null +++ b/dhat/tests/user_histo1.cpp @@ -0,0 +1,34 @@ +#include +#include +#include +#include +#include "dhat/dhat.h" + +int main() +{ + std::vector vec(2000, 0); + DHAT_HISTOGRAM_MEMORY_INIT(vec.data()); + std::mt19937 gen(42);; + std::uniform_int_distribution<> index_distrib(0, 1999); + std::uniform_int_distribution<> val_distrib(0, 255); + + for (int i = 0; i < 20; ++i) + { + int index = index_distrib(gen); + int val = val_distrib(gen); + vec[index] = val; + //std::cout << "wrote " << val << " to index " << index << "\n"; + } + + // try to generate some warnings + vec.resize(500); + vec.shrink_to_fit(); + DHAT_HISTOGRAM_MEMORY_UNINIT(vec.data()); + + auto old = vec.data(); + vec.resize(100000); + // old should have been deleted + DHAT_HISTOGRAM_MEMORY_UNINIT(old); + // and this is too big + DHAT_HISTOGRAM_MEMORY_UNINIT(vec.data()); +} diff --git a/dhat/tests/user_histo1.stderr.exp b/dhat/tests/user_histo1.stderr.exp new file mode 100644 index 0000000000..59206b17e3 --- /dev/null +++ b/dhat/tests/user_histo1.stderr.exp @@ -0,0 +1,8 @@ +Warning: request for user histogram of size 500 is smaller than the normal histogram limit, request ignored +Warning: address for user histogram request not found 55b2820 +Warning: request for user histogram of size 100000 is larger than the maximum user request limit, request ignored +Total: 102,500 bytes in 3 blocks +At t-gmax: 100,500 bytes in 2 blocks +At t-end: 0 bytes in 0 blocks +Reads: 1,000 bytes +Writes: 102,520 bytes diff --git a/dhat/tests/user_histo1.stdout.exp b/dhat/tests/user_histo1.stdout.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/dhat/tests/user_histo1.vgtest b/dhat/tests/user_histo1.vgtest new file mode 100644 index 0000000000..c44637a14a --- /dev/null +++ b/dhat/tests/user_histo1.vgtest @@ -0,0 +1,3 @@ +prog: user_histo1 +vgopts: --dhat-out-file=dhat.out +cleanup: rm dhat.out