|
From: Alex <zhi...@gm...> - 2019-05-14 10:55:59
|
we have have the simple code like this:
#include <stdlib.h>
int main(int argc, char *argv[])
{
malloc(10);
return 0;
}
then run `valgrind --leak-check=no --xml=yes --xml-file=x.xml ./main`
check the x.xml file, we see:
<error>
<unique>0x0</unique>
<tid>1</tid>
<kind>Leak_DefinitelyLost</kind>
<xwhat>
<text>10 bytes in 1 blocks are definitely lost in loss record 1 of
1</text>
<leakedbytes>10</leakedbytes>
<leakedblocks>1</leakedblocks>
</xwhat>
<stack>
<frame>
<ip>0x4C29E4B</ip>
<obj>/usr/local/lib/valgrind/vgpreload_memcheck-amd64-linux.so</obj>
<fn>malloc</fn>
<dir>/home/yizhi.fzh/valgrind-3.15.0/coregrind/m_replacemalloc</dir>
<file>vg_replace_malloc.c</file>
<line>309</line>
</frame>
<frame>
<ip>0x4004FF</ip>
<obj>/home/yizhi.fzh/github/c-quick-test/main</obj>
<fn>main</fn>
<dir>/home/yizhi.fzh/github/c-quick-test</dir>
<file>main.c</file>
<line>6</line>
</frame>
</stack>
</error>
$uname -a
Linux e18c07352.et15sqa 3.10.0-327.ali2008.alios7.x86_64 #1 SMP Tue Nov 29
17:56:13 CST 2016 x86_64 x86_64 x86_64 GNU/Linux
$valgrind --version
valgrind-3.15.0
Thanks
|
|
From: Alex <zhi...@gm...> - 2019-05-15 05:42:22
|
is anyone here?
On Tue, May 14, 2019 at 6:55 PM Alex <zhi...@gm...> wrote:
>
> we have have the simple code like this:
>
> #include <stdlib.h>
> int main(int argc, char *argv[])
> {
> malloc(10);
> return 0;
> }
>
> then run `valgrind --leak-check=no --xml=yes --xml-file=x.xml ./main`
>
> check the x.xml file, we see:
>
> <error>
> <unique>0x0</unique>
> <tid>1</tid>
> <kind>Leak_DefinitelyLost</kind>
> <xwhat>
> <text>10 bytes in 1 blocks are definitely lost in loss record 1 of
> 1</text>
> <leakedbytes>10</leakedbytes>
> <leakedblocks>1</leakedblocks>
> </xwhat>
> <stack>
> <frame>
> <ip>0x4C29E4B</ip>
> <obj>/usr/local/lib/valgrind/vgpreload_memcheck-amd64-linux.so</obj>
> <fn>malloc</fn>
> <dir>/home/yizhi.fzh/valgrind-3.15.0/coregrind/m_replacemalloc</dir>
> <file>vg_replace_malloc.c</file>
> <line>309</line>
> </frame>
> <frame>
> <ip>0x4004FF</ip>
> <obj>/home/yizhi.fzh/github/c-quick-test/main</obj>
> <fn>main</fn>
> <dir>/home/yizhi.fzh/github/c-quick-test</dir>
> <file>main.c</file>
> <line>6</line>
> </frame>
> </stack>
> </error>
>
> $uname -a
> Linux e18c07352.et15sqa 3.10.0-327.ali2008.alios7.x86_64 #1 SMP Tue Nov 29
> 17:56:13 CST 2016 x86_64 x86_64 x86_64 GNU/Linux
>
> $valgrind --version
> valgrind-3.15.0
>
> Thanks
>
|
|
From: John R. <jr...@bi...> - 2019-05-15 14:01:40
|
On Tue, 14 May 2019 18:55:40 +0800, Alex wrote: > we have have the simple code like this: > [[snipped]] On Wed, 15 May 2019 13:41:49 +0800, Alex wrote: > is anyone here? > If it's that urgent (19 hours) then fix it yourself, and include the patch along with the bug report. See the Bug Reports link in the Contact section of http://valgrind.org/ |
|
From: Philippe W. <phi...@sk...> - 2019-05-15 22:17:44
|
On Wed, 2019-05-15 at 13:41 +0800, Alex wrote:
> is anyone here?
>
> On Tue, May 14, 2019 at 6:55 PM Alex <zhi...@gm...> wrote:
> > we have have the simple code like this:
> >
> > #include <stdlib.h>
> > int main(int argc, char *argv[])
> > {
> > malloc(10);
> > return 0;
> > }
> >
> > then run `valgrind --leak-check=no --xml=yes --xml-file=x.xml ./main`
> >
> > check the x.xml file, we see:
> >
> > <error>
> > <unique>0x0</unique>
> > <tid>1</tid>
> > <kind>Leak_DefinitelyLost</kind>
> > <xwhat>
> > <text>10 bytes in 1 blocks are definitely lost in loss record 1 of 1</text>
> > <leakedbytes>10</leakedbytes>
> > <leakedblocks>1</leakedblocks>
> > </xwhat>
This is not a bug, it is a feature.
See the below code extracted from mc_main.c:
...
/* If we've been asked to emit XML, mash around various other
options so as to constrain the output somewhat. */
if (VG_(clo_xml)) {
/* Extract as much info as possible from the leak checker. */
MC_(clo_leak_check) = LC_Full;
}
...
The idea is that if you use xml, it means that you have another tool
that extracts/shows/presents/... the information produced by Valgrind,
and so, the filtering of what the 'real end user' wants to see is
to be done by the xml tool, not anymore by Valgrind.
The user manual (e.g. the paragraph that describes --leak-check option
should however describe this behaviour ...
Philippe
|
|
From: John R. <jr...@bi...> - 2019-05-16 14:35:10
|
>>> then run `valgrind --leak-check=no --xml=yes --xml-file=x.xml ./main`
>>>
>>> check the x.xml file, we see:
>>>
>>> <error>
>>> <unique>0x0</unique>
>>> <tid>1</tid>
>>> <kind>Leak_DefinitelyLost</kind>
>>> <xwhat>
>>> <text>10 bytes in 1 blocks are definitely lost in loss record 1 of 1</text>
>>> <leakedbytes>10</leakedbytes>
>>> <leakedblocks>1</leakedblocks>
>>> </xwhat>
> This is not a bug, it is a feature.
>
> See the below code extracted from mc_main.c:
> ...
> /* If we've been asked to emit XML, mash around various other
> options so as to constrain the output somewhat. */
> if (VG_(clo_xml)) {
> /* Extract as much info as possible from the leak checker. */
> MC_(clo_leak_check) = LC_Full;
> }
> ...
>
> The idea is that if you use xml, it means that you have another tool
> that extracts/shows/presents/... the information produced by Valgrind,
> and so, the filtering of what the 'real end user' wants to see is
> to be done by the xml tool, not anymore by Valgrind.
The original poster has a point: --leak-check=full has a run-time cost,
and the intent of --leak-check=no is to avoid that cost as much as
possible. On a medium-to-large size program, just the presentation
of the leak report can take several seconds, not even counting the cost
of the [logical] garbage collection to compute the leak status,
or possibly any per-allocation cost to preserve enough data to compute leaks.
If the garbage collection involves demand paging using backing storage,
then the cost can be large. An explicit --leak-check=no should
override compiled-in assumptions about how much --xml-file= contains.
|
|
From: Alex <zhi...@gm...> - 2019-05-16 15:01:57
|
On Thu, May 16, 2019 at 10:38 PM John Reiser <jr...@bi...> wrote:
> >>> then run `valgrind --leak-check=no --xml=yes --xml-file=x.xml ./main`
> >>>
> >>> check the x.xml file, we see:
> >>>
> >>> <error>
> >>> <unique>0x0</unique>
> >>> <tid>1</tid>
> >>> <kind>Leak_DefinitelyLost</kind>
> >>> <xwhat>
> >>> <text>10 bytes in 1 blocks are definitely lost in loss record 1
> of 1</text>
> >>> <leakedbytes>10</leakedbytes>
> >>> <leakedblocks>1</leakedblocks>
> >>> </xwhat>
>
> > This is not a bug, it is a feature.
> >
> > See the below code extracted from mc_main.c:
> > ...
> > /* If we've been asked to emit XML, mash around various other
> > options so as to constrain the output somewhat. */
> > if (VG_(clo_xml)) {
> > /* Extract as much info as possible from the leak checker. */
> > MC_(clo_leak_check) = LC_Full;
> > }
> > ...
> >
> > The idea is that if you use xml, it means that you have another tool
> > that extracts/shows/presents/... the information produced by Valgrind,
> > and so, the filtering of what the 'real end user' wants to see is
> > to be done by the xml tool, not anymore by Valgrind.
>
> The original poster has a point: --leak-check=full has a run-time cost,
> and the intent of --leak-check=no is to avoid that cost as much as
> possible. On a medium-to-large size program, just the presentation
> of the leak report can take several seconds, not even counting the cost
> of the [logical] garbage collection to compute the leak status,
> or possibly any per-allocation cost to preserve enough data to compute
> leaks.
> If the garbage collection involves demand paging using backing storage,
> then the cost can be large. An explicit --leak-check=no should
> override compiled-in assumptions about how much --xml-file= contains.
>
>
Yes, I agree with you about this. and thanks Philippe very much for
pointing this out. I will try to comment the option and to see if
everything goes well.
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
>
|
|
From: Philippe W. <phi...@sk...> - 2019-05-18 15:10:32
|
On Thu, 2019-05-16 at 07:34 -0700, John Reiser wrote:
> The original poster has a point: --leak-check=full has a run-time cost,
> and the intent of --leak-check=no is to avoid that cost as much as
> possible. On a medium-to-large size program, just the presentation
> of the leak report can take several seconds, not even counting the cost
> of the [logical] garbage collection to compute the leak status,
> or possibly any per-allocation cost to preserve enough data to compute leaks.
> If the garbage collection involves demand paging using backing storage,
> then the cost can be large. An explicit --leak-check=no should
> override compiled-in assumptions about how much --xml-file= contains.
The below patch implements the idea that an explicit --leak-check overrides
the --leak-check=full default when --xml=yes is given.
Before pushing this to HEAD, I would however prefer to have some feedback
(e.g. from Julian) about this.
Philippe
diff --git a/NEWS b/NEWS
index ba0f1285b..53038936c 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,9 @@ support for X86/macOS 10.13 and AMD64/macOS 10.13.
* Massif:
* Memcheck:
+ - The option --xml=yes used to unconditionally set --leak-check=full.
+ Now, when --xml=yes, --leck-check=full is used only if the user did
+ not explicitely give a --leak-check value.
* ==================== OTHER CHANGES ====================
diff --git a/memcheck/docs/mc-manual.xml b/memcheck/docs/mc-manual.xml
index a993479a9..24a9c7fcc 100644
--- a/memcheck/docs/mc-manual.xml
+++ b/memcheck/docs/mc-manual.xml
@@ -747,6 +747,8 @@ is <option>--errors-for-leak-kinds=definite,possible</option>
in detail and/or counted as an error, as specified by the options
<option>--show-leak-kinds</option> and
<option>--errors-for-leak-kinds</option>. </para>
+ <para>If <varname>--xml=yes</varname> is given, the default value
+ is set to <varname>full</varname>. </para>
</listitem>
</varlistentry>
diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c
index 253f091dc..a9ecab138 100644
--- a/memcheck/mc_main.c
+++ b/memcheck/mc_main.c
@@ -5938,6 +5938,9 @@ Bool MC_(clo_partial_loads_ok) = True;
Long MC_(clo_freelist_vol) = 20*1000*1000LL;
Long MC_(clo_freelist_big_blocks) = 1*1000*1000LL;
LeakCheckMode MC_(clo_leak_check) = LC_Summary;
+/* user_clc is set to True if the user explicitly gave the clo option
+ --leak-check. If True, --xml will not change the value of --leak-check. */
+Bool user_clc = False;
VgRes MC_(clo_leak_resolution) = Vg_HighRes;
UInt MC_(clo_show_leak_kinds) = R2S(Possible) | R2S(Unreached);
UInt MC_(clo_error_for_leak_kinds) = R2S(Possible) | R2S(Unreached);
@@ -6044,14 +6047,17 @@ static Bool mc_process_cmd_line_options(const HChar* arg)
MC_(clo_freelist_big_blocks),
0, 10*1000*1000*1000LL) {}
- else if VG_XACT_CLO(arg, "--leak-check=no",
- MC_(clo_leak_check), LC_Off) {}
+ else if VG_XACT_CLO(arg, "--leak-check=no", MC_(clo_leak_check), LC_Off)
+ { user_clc = True; }
else if VG_XACT_CLO(arg, "--leak-check=summary",
- MC_(clo_leak_check), LC_Summary) {}
+ MC_(clo_leak_check), LC_Summary)
+ { user_clc = True; }
else if VG_XACT_CLO(arg, "--leak-check=yes",
- MC_(clo_leak_check), LC_Full) {}
+ MC_(clo_leak_check), LC_Full)
+ { user_clc = True; }
else if VG_XACT_CLO(arg, "--leak-check=full",
- MC_(clo_leak_check), LC_Full) {}
+ MC_(clo_leak_check), LC_Full)
+ { user_clc = True; }
else if VG_XACT_CLO(arg, "--leak-resolution=low",
MC_(clo_leak_resolution), Vg_LowRes) {}
@@ -7727,8 +7733,9 @@ static void mc_post_clo_init ( void )
{
/* If we've been asked to emit XML, mash around various other
options so as to constrain the output somewhat. */
- if (VG_(clo_xml)) {
- /* Extract as much info as possible from the leak checker. */
+ if (VG_(clo_xml) & !user_clc) {
+ /* Extract as much info as possible from the leak checker, unless
+ the user explicitly gave the --leak-check option. */
MC_(clo_leak_check) = LC_Full;
}
diff --git a/memcheck/tests/xml1.vgtest b/memcheck/tests/xml1.vgtest
index 6585b18f4..d0ee11da2 100644
--- a/memcheck/tests/xml1.vgtest
+++ b/memcheck/tests/xml1.vgtest
@@ -1,3 +1,3 @@
prog: xml1
-vgopts: --xml=yes --xml-fd=2 --log-file=/dev/null --keep-stacktraces=alloc-then-free
+vgopts: --xml=yes --xml-fd=2 --log-file=/dev/null --keep-stacktraces=alloc-then-free --leak-check=full
stderr_filter: filter_xml
|
|
From: Philippe W. <phi...@sk...> - 2019-05-31 09:18:26
|
Julian and myself we just discussed the below on IRC.
The conclusion is that we do not believe the below patch should
be pushed upstream, for the following reason:
It is preferable that the xml format/behaviour is touched as less
as possible. Even if the below change is very probably
backward compatible with the existing xml readers, we must have
a clear benefit to do this change.
I just measured the speed of a leak search on an Intel i5 @2.8GHz.
The leak searcher is scanning 1GB of heap in 100_000 blocks
in about 1.3 seconds.
Also, there is no additional data recorded during the run for
the 'end of run' leak search.
So, the leak search speed is very probably not an issue.
If the issue is linked to the amount of leak errors output
in the xml file, this can be trimmed e.g. by using
--show-leak-kinds=none
and/or by using a suppression file suppressing all (or most)
of the leak errors.
I will in any case improve the manual to better document the effect
of --xml=yes on --leak-check, and maybe also indicate that
--show-leak-kinds=none can be used to reduce the xml output file
size if needed.
Thanks for the discussion
Philippe
On Sat, 2019-05-18 at 17:10 +0200, Philippe Waroquiers wrote:
> On Thu, 2019-05-16 at 07:34 -0700, John Reiser wrote:
>
> > The original poster has a point: --leak-check=full has a run-time cost,
> > and the intent of --leak-check=no is to avoid that cost as much as
> > possible. On a medium-to-large size program, just the presentation
> > of the leak report can take several seconds, not even counting the cost
> > of the [logical] garbage collection to compute the leak status,
> > or possibly any per-allocation cost to preserve enough data to compute leaks.
> > If the garbage collection involves demand paging using backing storage,
> > then the cost can be large. An explicit --leak-check=no should
> > override compiled-in assumptions about how much --xml-file= contains.
>
> The below patch implements the idea that an explicit --leak-check overrides
> the --leak-check=full default when --xml=yes is given.
>
> Before pushing this to HEAD, I would however prefer to have some feedback
> (e.g. from Julian) about this.
>
> Philippe
>
> diff --git a/NEWS b/NEWS
> index ba0f1285b..53038936c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -24,6 +24,9 @@ support for X86/macOS 10.13 and AMD64/macOS 10.13.
> * Massif:
>
> * Memcheck:
> + - The option --xml=yes used to unconditionally set --leak-check=full.
> + Now, when --xml=yes, --leck-check=full is used only if the user did
> + not explicitely give a --leak-check value.
>
> * ==================== OTHER CHANGES ====================
>
> diff --git a/memcheck/docs/mc-manual.xml b/memcheck/docs/mc-manual.xml
> index a993479a9..24a9c7fcc 100644
> --- a/memcheck/docs/mc-manual.xml
> +++ b/memcheck/docs/mc-manual.xml
> @@ -747,6 +747,8 @@ is <option>--errors-for-leak-kinds=definite,possible</option>
> in detail and/or counted as an error, as specified by the options
> <option>--show-leak-kinds</option> and
> <option>--errors-for-leak-kinds</option>. </para>
> + <para>If <varname>--xml=yes</varname> is given, the default value
> + is set to <varname>full</varname>. </para>
> </listitem>
> </varlistentry>
>
> diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c
> index 253f091dc..a9ecab138 100644
> --- a/memcheck/mc_main.c
> +++ b/memcheck/mc_main.c
> @@ -5938,6 +5938,9 @@ Bool MC_(clo_partial_loads_ok) = True;
> Long MC_(clo_freelist_vol) = 20*1000*1000LL;
> Long MC_(clo_freelist_big_blocks) = 1*1000*1000LL;
> LeakCheckMode MC_(clo_leak_check) = LC_Summary;
> +/* user_clc is set to True if the user explicitly gave the clo option
> + --leak-check. If True, --xml will not change the value of --leak-check. */
> +Bool user_clc = False;
> VgRes MC_(clo_leak_resolution) = Vg_HighRes;
> UInt MC_(clo_show_leak_kinds) = R2S(Possible) | R2S(Unreached);
> UInt MC_(clo_error_for_leak_kinds) = R2S(Possible) | R2S(Unreached);
> @@ -6044,14 +6047,17 @@ static Bool mc_process_cmd_line_options(const HChar* arg)
> MC_(clo_freelist_big_blocks),
> 0, 10*1000*1000*1000LL) {}
>
> - else if VG_XACT_CLO(arg, "--leak-check=no",
> - MC_(clo_leak_check), LC_Off) {}
> + else if VG_XACT_CLO(arg, "--leak-check=no", MC_(clo_leak_check), LC_Off)
> + { user_clc = True; }
> else if VG_XACT_CLO(arg, "--leak-check=summary",
> - MC_(clo_leak_check), LC_Summary) {}
> + MC_(clo_leak_check), LC_Summary)
> + { user_clc = True; }
> else if VG_XACT_CLO(arg, "--leak-check=yes",
> - MC_(clo_leak_check), LC_Full) {}
> + MC_(clo_leak_check), LC_Full)
> + { user_clc = True; }
> else if VG_XACT_CLO(arg, "--leak-check=full",
> - MC_(clo_leak_check), LC_Full) {}
> + MC_(clo_leak_check), LC_Full)
> + { user_clc = True; }
>
> else if VG_XACT_CLO(arg, "--leak-resolution=low",
> MC_(clo_leak_resolution), Vg_LowRes) {}
> @@ -7727,8 +7733,9 @@ static void mc_post_clo_init ( void )
> {
> /* If we've been asked to emit XML, mash around various other
> options so as to constrain the output somewhat. */
> - if (VG_(clo_xml)) {
> - /* Extract as much info as possible from the leak checker. */
> + if (VG_(clo_xml) & !user_clc) {
> + /* Extract as much info as possible from the leak checker, unless
> + the user explicitly gave the --leak-check option. */
> MC_(clo_leak_check) = LC_Full;
> }
>
> diff --git a/memcheck/tests/xml1.vgtest b/memcheck/tests/xml1.vgtest
> index 6585b18f4..d0ee11da2 100644
> --- a/memcheck/tests/xml1.vgtest
> +++ b/memcheck/tests/xml1.vgtest
> @@ -1,3 +1,3 @@
> prog: xml1
> -vgopts: --xml=yes --xml-fd=2 --log-file=/dev/null --keep-stacktraces=alloc-then-free
> +vgopts: --xml=yes --xml-fd=2 --log-file=/dev/null --keep-stacktraces=alloc-then-free --leak-check=full
> stderr_filter: filter_xml
|