[Crash-utility] [PATCH v1 0/3] Add valgrind support for the crash's custom memory

[lijiang]

Hi, HATAYAMA

在 2021年01月21日 12:47, crash-utility-request at redhat.com 写道:
> From: HATAYAMA Daisuke <d.hatayama at fujitsu.com>
> Sent: Monday, January 4, 2021 14:28
> To: crash-utility at 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?

[2] only pass the '-DVALGRIND' when using 'make valgrind' explicitly?

For example:

Step1: [root at 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 at 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? 


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