在 2021年02月22日 09:12, crash-utility-request@xxxxxxxxxx 写道: > Date: Mon, 22 Feb 2021 00:37:36 +0000 > From: HAGIO KAZUHITO(?????) <k-hagio-ab@xxxxxxx> > To: "Discussion list for crash utility usage, maintenance and > development" <crash-utility@xxxxxxxxxx> > Cc: "d.hatayama@xxxxxxxxxxx" <d.hatayama@xxxxxxxxxxx> > Subject: Re: [PATCH v1 0/3] Add valgrind support for > the crash's custom memory > Message-ID: > <OSBPR01MB1991940E91708000BFEDE9D3DD819@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> > > Content-Type: text/plain; charset="utf-8" > > Hi Lianbo, Hatayama-san, > > -----Original Message----- >> Hi, HATAYAMA >> >> ? 2021?01?21? 12:47, crash-utility-request@xxxxxxxxxx ??: >>> From: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxx> >>> Sent: Monday, January 4, 2021 14:28 >>> To: crash-utility@xxxxxxxxxx >>> 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 -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility
