[PATCH v2 0/3] Add valgrind support for the crash's custom memory
by HATAYAMA Daisuke
This patch set adds valgrind support for the crash's custom memory
allocator. The motivation comes from investigation at
https://github.com/crash-utility/crash-extensions/issues/1.
The 1st patch implements the valgrind support, while 2nd and 3rd fixes
the actual issues on the crash's custom memory allocator detected by
valgrind.
v2:
- Unlink tools.o when make valgrind
- Add description about make valgrind in README
HATAYAMA Daisuke (3):
Add valgrind support for the crash's custom memory allocator
symbols: Fix potential read to already freed object
tools: Fix potential write to object of 0 size
Makefile | 4 ++++
README | 5 +++++
configure.c | 21 +++++++++++++++++-
help.c | 5 +++++
symbols.c | 10 +++------
tools.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
6 files changed, 107 insertions(+), 10 deletions(-)
--
1.8.3.1
3 years, 8 months
- 2
- 4
[PATCH] x86_64: fix "bt" command on 5.12-rc1 and later kernels
by HAGIO KAZUHITO(萩尾 一仁)
Fix "bt" command on Linux 5.12-rc1 and later kernels that contains
kernel commit 951c2a51ae75 ("x86/irq/64: Adjust the per CPU irq stack
pointer by 8"). Without the patch, the "bt" command and some options
that read irq stack fails with the error message "bt: read of stack
at <address> failed".
Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
---
x86_64.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/x86_64.c b/x86_64.c
index 23a40a04bbc4..f5b2f7b5f040 100644
--- a/x86_64.c
+++ b/x86_64.c
@@ -1326,6 +1326,8 @@ x86_64_per_cpu_init(void)
KVADDR, &hardirq_stack_ptr, sizeof(void *),
"hardirq_stack_ptr (per_cpu)", QUIET|RETURN_ON_ERROR))
continue;
+ if (hardirq_stack_ptr != PAGEBASE(hardirq_stack_ptr))
+ hardirq_stack_ptr += 8;
ms->stkinfo.ibase[i] = hardirq_stack_ptr - ms->stkinfo.isize;
} else if (irq_sp)
ms->stkinfo.ibase[i] = irq_sp->value + kt->__per_cpu_offset[i];
--
2.18.4
3 years, 8 months
- 2
- 2
Re: [Crash-utility] [PATCH v2 1/3] Add valgrind support for the crash's custom memory allocator
by lijiang
Hi, HATAYAMA
Thank you for the update.
在 2021年03月04日 19:21, crash-utility-request(a)redhat.com 写道:
> Date: Thu, 4 Mar 2021 20:20:28 +0900
> From: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
> To: crash-utility(a)redhat.com
> Cc: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
> Subject: [Crash-utility] [PATCH v2 1/3] Add valgrind support for the
> crash's custom memory allocator
> Message-ID: <1614856830-14414-2-git-send-email-d.hatayama(a)fujitsu.com>
> Content-Type: text/plain; charset="US-ASCII"
>
> This adds valgrind support for the crash's custom memory allocator
> using the way described in the following valgrind's Memcheck manual:
>
> https://www.valgrind.org/docs/manual/mc-manual.html#mc-manual.mempools
>
> This helps detecting various memory errors on the crash's custom
> memory allocator.
>
> To enable this feature, build crash command as:
>
> # make valgrind
>
> Then, run crash commnad using valgrind as:
>
> # valgrind ./crash vmlinux vmcore
>
> Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
> ---
> Makefile | 4 ++++
> README | 5 +++++
> configure.c | 21 ++++++++++++++++++-
> help.c | 5 +++++
> tools.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 103 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f66eba7..e0c922f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -332,6 +332,10 @@ snappy: make_configure
> @./configure -x snappy ${CONF_TARGET_FLAG} -w -b
> @make --no-print-directory gdb_merge
>
> +valgrind: make_configure
> + @./configure -x valgrind ${CONF_TARGET_FLAG} -w -b
> + @make --no-print-directory gdb_merge
> +
> main.o: ${GENERIC_HFILES} main.c
> ${CC} -c ${CRASH_CFLAGS} main.c ${WARNING_OPTIONS} ${WARNING_ERROR}
>
> diff --git a/README b/README
> index e2af924..9b9c6e6 100644
> --- a/README
> +++ b/README
> @@ -105,6 +105,11 @@
> use either the LZO or snappy compression libraries. To build crash with
> either or both of those libraries, type "make lzo" or "make snappy".
>
> + crash supports valgrind Memcheck tool on the crash's custom memory
> + allocator. To build crash with this feature enabled, type "make
> + valrind" and then run crash with valgrind as "valgrind crash
^^^^^^^
Here the "valrind" should be a typo.
> + vmlinux vmcore".
> +
> All of the alternate build commands above are "sticky" in that the
> special "make" targets only have to be entered one time; all subsequent
> builds will follow suit.
> diff --git a/configure.c b/configure.c
> index 7f6d19e..52d3c24 100644
> --- a/configure.c
> +++ b/configure.c
> @@ -1710,12 +1710,16 @@ get_extra_flags(char *filename, char *initial)
> * For snappy:
> * - enter -DLZO in the CFLAGS.extra file
> * - enter -llzo2 in the LDFLAGS.extra file.
> + *
> + * For valgrind:
> + * - enter -DVALGRIND in the CFLAGS.extra file
> */
> void
> add_extra_lib(char *option)
> {
> int lzo, add_DLZO, add_llzo2;
> int snappy, add_DSNAPPY, add_lsnappy;
> + int valgrind, add_DVALGRIND;
> char *cflags, *ldflags;
> FILE *fp_cflags, *fp_ldflags;
> char *mode;
> @@ -1723,6 +1727,7 @@ add_extra_lib(char *option)
>
> lzo = add_DLZO = add_llzo2 = 0;
> snappy = add_DSNAPPY = add_lsnappy = 0;
> + valgrind = add_DVALGRIND = 0;
>
> ldflags = get_extra_flags("LDFLAGS.extra", NULL);
> cflags = get_extra_flags("CFLAGS.extra", NULL);
> @@ -1743,12 +1748,24 @@ add_extra_lib(char *option)
> add_lsnappy++;
> }
>
> + if (strcmp(option, "valgrind") == 0) {
> + valgrind++;
> + if (!cflags || !strstr(cflags, "-DVALGRIND"))
> + add_DVALGRIND++;
> + }
> +
> if ((lzo || snappy) &&
> file_exists("diskdump.o") && (unlink("diskdump.o") < 0)) {
> perror("diskdump.o");
> return;
> }
>
> + if (valgrind &&
> + file_exists("tools.o") && (unlink("tools.o") < 0)) {
> + perror("tools.o");
> + return;
> + }
> +
> mode = file_exists("CFLAGS.extra") ? "r+" : "w+";
> if ((fp_cflags = fopen("CFLAGS.extra", mode)) == NULL) {
> perror("CFLAGS.extra");
> @@ -1762,13 +1779,15 @@ add_extra_lib(char *option)
> return;
> }
>
> - if (add_DLZO || add_DSNAPPY) {
> + if (add_DLZO || add_DSNAPPY || add_DVALGRIND) {
> while (fgets(inbuf, 512, fp_cflags))
> ;
> if (add_DLZO)
> fputs("-DLZO\n", fp_cflags);
> if (add_DSNAPPY)
> fputs("-DSNAPPY\n", fp_cflags);
> + if (add_DVALGRIND)
> + fputs("-DVALGRIND\n", fp_cflags);
> }
>
> if (add_llzo2 || add_lsnappy) {
> diff --git a/help.c b/help.c
> index d3427a3..f2b1007 100644
> --- a/help.c
> +++ b/help.c
> @@ -9428,6 +9428,11 @@ README_ENTER_DIRECTORY,
> " use either the LZO or snappy compression libraries. To build crash with",
> " either or both of those libraries, type \"make lzo\" or \"make snappy\".",
> "",
> +" crash supports valgrind Memcheck tool on the crash's custom memory",
> +" allocator. To build crash with this feature enabled, type \"make",
> +" valrind\" and then run crash with valgrind as \"valgrind crash",
^^^^^^^
Ditto.
Otherwise, for the v2 series: Acked-by: Lianbo Jiang <lijiang(a)redhat.com>
> +" vmlinux vmcore\".",
> +"",
> " All of the alternate build commands above are \"sticky\" in that the",
> " special \"make\" targets only have to be entered one time; all subsequent",
> " builds will follow suit.",
> diff --git a/tools.c b/tools.c
> index 89352b1..91d5baf 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -18,6 +18,11 @@
> #include "defs.h"
> #include <ctype.h>
>
> +#ifdef VALGRIND
> +#include <valgrind/valgrind.h>
> +#include <valgrind/memcheck.h>
> +#endif
> +
> static void print_number(struct number_option *, int, int);
> static long alloc_hq_entry(void);
> struct hq_entry;
> @@ -5679,8 +5684,21 @@ buf_init(void)
>
> bp->smallest = 0x7fffffff;
> bp->total = 0.0;
> -}
>
> +#ifdef VALGRIND
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_1K, sizeof(bp->buf_1K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_2K, sizeof(bp->buf_2K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_4K, sizeof(bp->buf_4K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_8K, sizeof(bp->buf_8K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_32K, sizeof(bp->buf_32K));
> +
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_1K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_2K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_4K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_8K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_32K, 0, 1);
> +#endif
> +}
>
> /*
> * Free up all buffers used by the last command.
> @@ -5707,6 +5725,26 @@ void free_all_bufs(void)
> if (bp->mallocs != bp->frees)
> error(WARNING, "malloc/free mismatch (%ld/%ld)\n",
> bp->mallocs, bp->frees);
> +
> +#ifdef VALGRIND
> + VALGRIND_DESTROY_MEMPOOL(&bp->buf_1K);
> + VALGRIND_DESTROY_MEMPOOL(&bp->buf_2K);
> + VALGRIND_DESTROY_MEMPOOL(&bp->buf_4K);
> + VALGRIND_DESTROY_MEMPOOL(&bp->buf_8K);
> + VALGRIND_DESTROY_MEMPOOL(&bp->buf_32K);
> +
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_1K, sizeof(bp->buf_1K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_2K, sizeof(bp->buf_2K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_4K, sizeof(bp->buf_4K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_8K, sizeof(bp->buf_8K));
> + VALGRIND_MAKE_MEM_NOACCESS(&bp->buf_32K, sizeof(bp->buf_32K));
> +
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_1K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_2K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_4K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_8K, 0, 1);
> + VALGRIND_CREATE_MEMPOOL(&bp->buf_32K, 0, 1);
> +#endif
> }
>
> /*
> @@ -5731,6 +5769,9 @@ freebuf(char *addr)
> for (i = 0; i < NUMBER_1K_BUFS; i++) {
> if (addr == (char *)&bp->buf_1K[i]) {
> bp->buf_inuse[B1K] &= ~(1 << i);
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_FREE(&bp->buf_1K, addr);
> +#endif
> return;
> }
> }
> @@ -5738,6 +5779,9 @@ freebuf(char *addr)
> for (i = 0; i < NUMBER_2K_BUFS; i++) {
> if (addr == (char *)&bp->buf_2K[i]) {
> bp->buf_inuse[B2K] &= ~(1 << i);
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_FREE(&bp->buf_2K, addr);
> +#endif
> return;
> }
> }
> @@ -5745,6 +5789,9 @@ freebuf(char *addr)
> for (i = 0; i < NUMBER_4K_BUFS; i++) {
> if (addr == (char *)&bp->buf_4K[i]) {
> bp->buf_inuse[B4K] &= ~(1 << i);
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_FREE(&bp->buf_4K, addr);
> +#endif
> return;
> }
> }
> @@ -5752,6 +5799,9 @@ freebuf(char *addr)
> for (i = 0; i < NUMBER_8K_BUFS; i++) {
> if (addr == (char *)&bp->buf_8K[i]) {
> bp->buf_inuse[B8K] &= ~(1 << i);
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_FREE(&bp->buf_8K, addr);
> +#endif
> return;
> }
> }
> @@ -5759,6 +5809,9 @@ freebuf(char *addr)
> for (i = 0; i < NUMBER_32K_BUFS; i++) {
> if (addr == (char *)&bp->buf_32K[i]) {
> bp->buf_inuse[B32K] &= ~(1 << i);
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_FREE(&bp->buf_32K, addr);
> +#endif
> return;
> }
> }
> @@ -5924,6 +5977,9 @@ getbuf(long reqsize)
> bp->buf_inuse[B1K] |= (1 << bdx);
> bp->buf_1K_maxuse = MAX(bp->buf_1K_maxuse,
> count_bits_int(bp->buf_inuse[B1K]));
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_ALLOC(&bp->buf_1K, bufp, 1024);
> +#endif
> BZERO(bufp, 1024);
> return(bufp);
> }
> @@ -5938,6 +5994,9 @@ getbuf(long reqsize)
> bp->buf_inuse[B2K] |= (1 << bdx);
> bp->buf_2K_maxuse = MAX(bp->buf_2K_maxuse,
> count_bits_int(bp->buf_inuse[B2K]));
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_ALLOC(&bp->buf_2K, bufp, 2048);
> +#endif
> BZERO(bufp, 2048);
> return(bufp);
> }
> @@ -5952,6 +6011,9 @@ getbuf(long reqsize)
> bp->buf_inuse[B4K] |= (1 << bdx);
> bp->buf_4K_maxuse = MAX(bp->buf_4K_maxuse,
> count_bits_int(bp->buf_inuse[B4K]));
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_ALLOC(&bp->buf_4K, bufp, 4096);
> +#endif
> BZERO(bufp, 4096);
> return(bufp);
> }
> @@ -5966,6 +6028,9 @@ getbuf(long reqsize)
> bp->buf_inuse[B8K] |= (1 << bdx);
> bp->buf_8K_maxuse = MAX(bp->buf_8K_maxuse,
> count_bits_int(bp->buf_inuse[B8K]));
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_ALLOC(&bp->buf_8K, bufp, 8192);
> +#endif
> BZERO(bufp, 8192);
> return(bufp);
> }
> @@ -5980,6 +6045,9 @@ getbuf(long reqsize)
> bp->buf_inuse[B32K] |= (1 << bdx);
> bp->buf_32K_maxuse = MAX(bp->buf_32K_maxuse,
> count_bits_int(bp->buf_inuse[B32K]));
> +#ifdef VALGRIND
> + VALGRIND_MEMPOOL_ALLOC(&bp->buf_32K, bufp, 32768);
> +#endif
> BZERO(bufp, 32768);
> return(bufp);
> }
> -- 1.8.3.1
3 years, 8 months
- 1
- 0
Re: [Crash-utility] [PATCH v1 0/3] Add valgrind support for the crash's custom memory
by lijiang
在 2021年02月22日 09:12, crash-utility-request(a)redhat.com 写道:
> Date: Mon, 22 Feb 2021 00:37:36 +0000
> From: HAGIO KAZUHITO(?????) <k-hagio-ab(a)nec.com>
> To: "Discussion list for crash utility usage, maintenance and
> development" <crash-utility(a)redhat.com>
> Cc: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>
> Subject: Re: [Crash-utility] [PATCH v1 0/3] Add valgrind support for
> the crash's custom memory
> Message-ID:
> <OSBPR01MB1991940E91708000BFEDE9D3DD819(a)OSBPR01MB1991.jpnprd01.prod.outlook.com>
>
> Content-Type: text/plain; charset="utf-8"
>
> Hi Lianbo, Hatayama-san,
>
> -----Original Message-----
>> Hi, HATAYAMA
>>
>> ? 2021?01?21? 12:47, crash-utility-request(a)redhat.com ??:
>>> From: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
>>> Sent: Monday, January 4, 2021 14:28
>>> To: crash-utility(a)redhat.com
>>> Cc: Hatayama, Daisuke/?? ??
>>> Subject: [PATCH v1 0/3] Add valgrind support for the crash's custom memory
>>>
>>> This patch set adds valgrind support for the crash's custom memory
>>> allocator. The motivation comes from investigation at
>>> https://github.com/crash-utility/crash-extensions/issues/1.
>>>
>>> The 1st patch implements the valgrind support, while 2nd and 3rd fixes
>>> the actual issues on the crash's custom memory allocator detected by
>>> valgrind.
>>>
>>> HATAYAMA Daisuke (3):
>>> Add valgrind support for the crash's custom memory allocator
>>> symbols: Fix potential read to already freed object
>>> tools: Fix potential write to object of 0 size
>>>
>> Thank you for the patchset.
>>
>> This changes are fine to me, but I have some other concerns as below:
>>
>> [1] add a simple description for supporting 'make valgrind' in README?
> Good catch, thanks Lianbo.
>
> FYI, note that the README data is also in help.c and needs to be updated.
>
Sure.
>> [2] only pass the '-DVALGRIND' when using 'make valgrind' explicitly?
>>
>> For example:
>>
>> Step1: [root@dell-pet620-01 crash]# make valgrind
>> TARGET: X86_64
>> CRASH: 7.2.9++
>> GDB: 7.6
>>
>> cc -c -g -DX86_64 -DVALGRIND -DGDB_7_6 build_data.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes
>> -fstack-protector -Wformat-security
>> cc -c -g -DX86_64 -DVALGRIND -DGDB_7_6 tools.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes
>> -fstack-protector -Wformat-security
>> cc -c -g -DX86_64 -DVALGRIND -DGDB_7_6 global_data.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes
>> -fstack-protector -Wformat-security
>> ^^^^^^^^^
>> ...
>>
>> Step2: [root@dell-pet620-01 crash]# make lzo
>> TARGET: X86_64
>> CRASH: 7.2.9++
>> GDB: 7.6
>>
>> cc -c -g -DX86_64 -DVALGRIND -DLZO -DGDB_7_6 build_data.c -Wall -O2 -Wstrict-prototypes
>> -Wmissing-prototypes -fstack-protector -Wformat-security
>> cc -c -g -DX86_64 -DVALGRIND -DLZO -DGDB_7_6 main.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes
>> -fstack-protector -Wformat-security
>> cc -c -g -DX86_64 -DVALGRIND -DLZO -DGDB_7_6 tools.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes
>> -fstack-protector -Wformat-security
>> ^^^^^^^^^^^^^^^
>> ...
>>
>> For the 'make lzo', the cflag '-DVALGRIND' is also added here after the step1, is that expected?
> As written in the README, these targets are sticky, so it's expected:
>
> All of the alternate build commands above are "sticky" in that the
> special "make" targets only have to be entered one time; all subsequent
> builds will follow suit.
>
> AFAIK, the only command that can drop a target is "make nowarn", otherwise
> we can drop "lzo" and so on by removing CFLAGS.extra and LDFLAGS.extra for
> the present.
>
Seems yes. Is it possible to separate these CFLAGS? And we may put them together when
it is needed, For example:
make lzo (-DLZO)
make valgrind (-DVALGRIND)
make lzo_valgrind (-DVALGRIND -DLZO)
But I'm not sure if it looks more reasonable. Anyway, this is another issue.
Thanks.
Lianbo
> Thanks,
> Kazu
>
>>
>> BTW: this change could make the distribution add a new dependency of valgrind-devel package(A warm reminder).
>>
>> Thanks.
>> Lianbo
>>
>>> Makefile | 4 ++++
>>> configure.c | 15 ++++++++++++-
>>> symbols.c | 10 +++------
>>> tools.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> 4 files changed, 91 insertions(+), 10 deletions(-)
>>>
>>> --
>>> 1.8.3.1
3 years, 9 months
- 3
- 4