Re: reclaiming memory before exit, take 2

2020-05-16 Thread Paul Smith
On Sat, 2020-05-16 at 16:04 +0200, Bruno Haible wrote:
> Paul writes in [1]:
> > In any event, it seems to me to be a deficiency in the detection if
> > it reports allocated memory which is still referenced to by global
> > variables, or even static variables, as memory leaks.
> 
> On the contrary, reporting leaks through global and static variables
> is a feature. *Hiding* them, which is what the tools do by default,
> is a *misfeature*.
> 
> Why? Because caches that grow without bounds are serious memory leaks
> as well. It is pointless to eliminate serious memory leaks that were
> caused by forgetting free(), while at the same not noticing serious
> memory leaks that were caused by bad design of caches. BOTH have same
> adverse effects.
> 
> This means, when looking for memory leaks, you should use valgrind
> with '--show-reachable=yes'.

I'm afraid I don't agree with this position, although of course I only
speak for the GNU make project.

To me memory leaks have a very specific meaning: memory that was
allocated but is no longer referenced and hence not recoverable.  This
is the only type of problem that leak detectors are good for
discovering.

A cache that grows very large may or may not be an error: no tool can
automatically determine whether it is appropriate or not.

It may mean that your program needs to do a lot of work and is choosing
an entirely reasonable trade-off of memory for another resource (say
disk IO).

Or, certainly, it may be a signal that the algorithms you've chosen to
solve the problem are not up to the task, or even that you have a bug
in them (not reusing cache when you can or should).  To decide that
you'll need to use a memory profiler and discover how much memory is
being used and where, and see if there's an issue.

IMO "--show-reachable=yes" is an anti-pattern and should not be used:
it provides nothing but false positives and the only way to avoid them
is to implement "final free", which won't help you discover too-large
caches anyway, or add suppression files for known-correct allocations,
which you should certainly prefer to avoid.




Licensing issues for gendocs_template_min

2020-05-16 Thread Asher Gordon
Hello,

I would like to use the gendocs.sh script to generate documentation for
my package.¹ However, since my package is not part of the GNU project,
there are naturally some changes I would like to make to the gendocs
template (specifically, gendocs_template_min). Here are the specific
changes I would like to make:
--- gendocs_template_min.gnulib
+++ gendocs_template_min
@@ -4,23 +4,20 @@
 http://www.w3.org/1999/xhtml; xml:lang="en">
 
 
-%%TITLE%% - GNU Project - Free Software Foundation
+%%TITLE%% - Documentation
 
-
+
 
 
 
 
 %%TITLE%%
 
-Free Software Foundation
+
+  Asher Gordon
+  asd...@posteo.net
+
 last updated %%DATE%%
-
-
-	
-
-
 
 
 This manual (%%PACKAGE%%) is available in the following formats:
@@ -73,21 +70,12 @@
 
 
 
-Please send general FSF  GNU inquiries to
-g...@gnu.org.
-There are also other ways to contact
-the FSF.  Broken links and other corrections or suggestions can be sent
-to %%EMAIL%%.
-
-
 Copyright  2020 Free Software Foundation, Inc.
 
 This page is licensed under a https://creativecommons.org/licenses/by-nd/3.0/us/;>Creative
 Commons Attribution-NoDerivs 3.0 United States License.
 
-
-
 
 
 
(maintain) Invoking gendocs.sh says: "(Feel free to modify the generic
template for your own purposes.)" However, gendocs_template_min is
licensed under the Creative Commons Attribution-NoDerivs 3.0 United
States License. To quote the license text:

The above rights include the right to make such modifications as are
technically necessary to exercise the rights in other media and
formats, but otherwise *you have no rights to make Derivative
Works*.

(Emphasis added.) So this seems to be a contradiction.

Also, I understand the FSF would want to license works which convey
their opinions under the CC BY-ND 3.0 US or similar licenses, but
gendocs_template_min is certainly not such a work, and using such a
license seems to me to be a mistake.

These issues also apply to gendocs_template, since that is under the
same license.

Since this is a licensing issue, I've CC'd licens...@fsf.org since they
might have people who understand this stuff better.

Thanks,
Asher

Footnotes: 
¹  https://savannah.nongnu.org/projects/libmu

-- 
Reader, suppose you were an idiot.  And suppose you were a member of
Congress.  But I repeat myself.
-- Mark Twain
   
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68


signature.asc
Description: PGP signature


Re: Remove license modules

2020-05-16 Thread Asher Gordon
Hi Bruno,

Bruno Haible  writes:

> Now it has completed, and it turned out you were confusing the license
> texts in text format (which ought to be added to VCS directly) and the
> license texts in Texinfo format (for which gnulib modules are
> appropriate).

Well I didn't include the GFDL in plain text format in my package. I
only included it in Texinfo format, but Ineiev accepted my package
nevertheless. I think the Texinfo format licenses are human readable
enough that it is sufficient to just include those. In fact, it seems
this is what Gnulib itself does.

But as Ineiev points out, the modules are valid for two of the three
methods described in (gnulib) VCS Issues. So I think instead that the
documentation should be updated. See the patch to update the
documentation below. I added an anchor, VCS Omissions, under the VCS
Issues node in gnulib-tool.texi. I think that since the relevant
material is slightly far down on the page, if I link directly to VCS
Issues, it could cause confusion. However, feel free to remove the
anchor if you see fit. I also fixed a minor issue where "VCS" ended with
"." rather than "@." (see (texinfo) Ending a Sentence).
From 022bd24bebde0e033a37b6c43770382fed996a39 Mon Sep 17 00:00:00 2001
From: Asher Gordon 
Date: Sat, 16 May 2020 13:49:02 -0400
Subject: [PATCH] doc: Update to indicate that license files must always be
 included.

* doc/gnulib-tool.texi (VCS Omissions): New anchor under the VCS
Issues node. Describe the license issues here.
* doc/license-texi.texi: Mention the license issues and link to
VCS Omissions.
---
 ChangeLog  |  8 
 doc/gnulib-tool.texi   | 15 +++
 doc/licenses-texi.texi |  4 
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 889c756eb..75a4c37a5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-16  Asher Gordon  
+
+	doc: Update to indicate that license files must always be included.
+	* doc/gnulib-tool.texi (VCS Omissions): New anchor under the VCS
+	Issues node. Describe the license issues here.
+	* doc/license-texi.texi: Mention the license issues and link to
+	VCS Omissions.
+
 2020-05-16  Bruno Haible  
 
 	findprog-lgpl: Fix link error (existing since 2008-09-02).
diff --git a/doc/gnulib-tool.texi b/doc/gnulib-tool.texi
index 44a393a69..ca9837240 100644
--- a/doc/gnulib-tool.texi
+++ b/doc/gnulib-tool.texi
@@ -914,12 +914,19 @@ Gnulib also contains files generated by @command{make} (and removed by
 into the VCS, but instead added to @file{.gitignore} or equivalent.
 
 @item
+@anchor{VCS Omissions}
 In projects which customarily omit from their VCS all files that are
 generated from other source files, none of these files and directories
-are added into the VCS.  As described in @ref{Modified imports}, there
-are two ways to keep track of options and module names that are passed
-to @code{gnulib-tool}.  The command for restoring the omitted files
-depends on it:
+are added into the VCS@.  However, please note: licenses must always be
+included in the VCS, whether in plain text format or Texinfo format.
+Projects which omit all generated files from their VCS may prefer to
+copy the license files directly rather than using the Gnulib modules.
+Alternatively, they can include the licenses in plain text format and
+use the Gnulib modules for the licenses in Texinfo format.
+@xref{License Texinfo sources}.  As described in @ref{Modified imports},
+there are two ways to keep track of options and module names that are
+passed to @code{gnulib-tool}.  The command for restoring the omitted
+files depends on it:
 
 @itemize @bullet
 @item
diff --git a/doc/licenses-texi.texi b/doc/licenses-texi.texi
index 60110ef09..2bcb2bed5 100644
--- a/doc/licenses-texi.texi
+++ b/doc/licenses-texi.texi
@@ -12,3 +12,7 @@ The conventional name for the GPL node is @samp{Copying} and for the FDL
 a conventional node name.
 
 Of course the license texts themselves should not be changed at all.
+
+Please note that if you use these modules, you must still include a copy
+of the license in plain text format or Texinfo format in the VCS@.
+@xref{VCS Omissions} for more information.
-- 
2.26.2

Thanks,
Asher

-- 
One meets his destiny often on the road he takes to avoid it.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68


signature.asc
Description: PGP signature


Re: reclaiming memory before exit, take 2

2020-05-16 Thread Bruno Haible
Jeffrey Walton wrote:
> I prefer to run a release
> build with diagnostics. Something like '-g2 -O3 -DNDEBUG
> -fsanitize=asan', install it, and then use it long term.
> 
> One of the [many] reasons this is important is, it provides additional
> coverage beyond the test cases. In the wild I may encounter an
> attacker supplied input that tickles a problem, like a buffer
> overflow. Or I may encounter an otherwise benign input that tickles a
> problem.

Excellent point. So, basically you run a hardened system.
(Probably you also have MALLOC_PERTURB_ permanently set?)

And you would like to set
  ASAN_OPTIONS=detect_leaks=1
and have most programs succeed? Let's see what you need to attain
that goal, assuming the developers follow the approach with
'valgrind --show-reachable=yes' and suppressions files.

1) You would need a suppressions file for every executable that you
   install. This means, the suppressions file should better be
   distributed in the tarball. Right?

2) You would need the suppressions file in ASAN syntax, not in
   valgrind syntax. Do you have a converter for the suppressions files?

3) The suppressions files that you use would not need to (but could still)
   contain the suppressions for leaks through global and static variables.

4) You would also need to set the environment variable CLEANUP_BEFORE_EXIT
   and hope that as many programs as possible obey it.

Right? Anything else?

Bruno




Re: reclaiming memory before exit, take 2

2020-05-16 Thread Paul Eggert
On 5/16/20 10:26 AM, Bruno Haible wrote:

> Are you suggesting that QA automation should ignore the global and static
> variables?

No, merely that attention should be focused on important leaks, not unimportant
ones. Memory that is needed during program execution, and that becomes
not-needed just before program exit, does not constitute an important leak (it's
arguably not a leak at all; it depends on one's definition of "leak").

Still-reachable memory can constitute an important leak (if your program has
allocated an object that it will never actually refer to later, for example) and
so in general one should look at globals and statics as well, in order to find
these important leaks.

In the valgrind context, focusing on important leaks may well mean that we need
a suppression file until the tools get better.



Re: reclaiming memory before exit, take 2

2020-05-16 Thread Bruno Haible
Hi Paul,

> > The QA automation needs to rely on the suppression files created by the
> > developers, since it's not a QA engineer's job to evaluate whether a
> > particular memory leak is serious or not.
> 
> This division of labor is not universal. In many shops, QA engineers are more
> involved in determining the seriousness of bugs, and they can maintain
> suppression files.

Indeed. It's a continuum. It can very well be the QA engineer who determines
the seriousness of a leak and proposes suppressions to the developer.

> > On the contrary, reporting leaks through global and static variables is
> > a feature. *Hiding* them, which is what the tools do by default, is a
> > *misfeature*.
> 
> This depends on what sort of leaks you're looking for. I agree that these are
> very often leaks, and that it generally makes sense to look for them. 
> However, I
> don't think we always need to worry about these leaks just before program 
> exit,
> since they are not a performance problem there and introducing 'free' there 
> both
> complicates and slows the program.

I'm not sure I understand what you mean.

We agree that, in the production binaries, it is a waste of computer resources
to free the memory references stored in global and static variables.

Are you suggesting that developers should ignore the global and static
variables, i.e. not use --show-reachable=yes? If so, what would be the 
advantage,
compared to the suppressions file approach?

Are you suggesting that QA automation should ignore the global and static
variables?

Bruno




Re: reclaiming memory before exit, take 2

2020-05-16 Thread Jeffrey Walton
On Sat, May 16, 2020 at 12:47 PM Bruno Haible  wrote:
>
> Tim Rühsen wrote:
> > At GNU wget we have conditional cleanup functions. That is compilation
> > with -DDEBUG_MALLOC in $CFLAGS will add those cleanup functions and they
> > are called before wget exits. Handy for testing, but you have to build
> > an extra executable.
>
> How about using an environment variable instead? You would set it in the
> Automake variable TESTS_ENVIRONMENT.
>
> Then you would not need an extra executable, and the individual tests in
> your testsuite do not need to me modified.

I can't speak for Tim and his projects, but I prefer to run a release
build with diagnostics. Something like '-g2 -O3 -DNDEBUG
-fsanitize=asan', install it, and then use it long term.

One of the [many] reasons this is important is, it provides additional
coverage beyond the test cases. In the wild I may encounter an
attacker supplied input that tickles a problem, like a buffer
overflow. Or I may encounter an otherwise benign input that tickles a
problem.

The folks who pursue tailored access do this sort of thing. Their
methodologies are mature, and their attack trees are wide and deep. I
have first hand knowledge of some of the tricks. Years ago I worked
with a firm that sold exploit packages to Northrop Grumman Electronic
Warfare division. Decades ago I worked with another firm that guarded
US supercomputing centers. It had offensive capabilities to work back
to determine the source of the attack.

Jeff



Re: reclaiming memory before exit, take 2

2020-05-16 Thread Bruno Haible
Hi Tim,

> When talking about finding memory leaks, like in your wiki page, you
> should also mention static analyzers and their ability to find memory
> leaks (clang static analyzer, gcc-10 -fanalyzer, Coverity, cppcheck, ...).

Would you like to add one or more wiki pages about these static analyzers?

> These are able to find leaks (and much more) even in code that is seldom
> used during runtime (and perhaps never during testing).
> To find all those leaks with runtime tools like valgrind, you need  a
> very high code coverage in your test suite.

You are right. Your argument applies to memory leaks as well as file
descriptor leaks.

Personally I've used a static analyzer with the purpose of finding file
descriptor leaks but not for memory leaks. I don't know why; maybe
because for file descriptor leaks there are fewer reports and thus
fewer false findings to eliminate? Or because we have valgrind for the
memory leaks but no experience with 'valgrind --track-fds'?

> Another point is: a library exit / cleanup call before exiting the
> application can easily free all reachable memory and by that disguise or
> hide internal memory leaks that lead to OOM in a long-running
> application (daemon or e.g. wget -r).
> In these cases it may be helpful to *not* free memory and not call exit
> library functions before exiting the application. Only then could
> valgrind show the memory hog (maybe the --xtree-memory option is for
> exactly that - I didn't investigate it yet).

It sounds like what you would need here is "heap profiling"
('valgrind --massif').

Bruno




Re: reclaiming memory before exit, take 2

2020-05-16 Thread Bruno Haible
Tim Rühsen wrote:
> At GNU wget we have conditional cleanup functions. That is compilation
> with -DDEBUG_MALLOC in $CFLAGS will add those cleanup functions and they
> are called before wget exits. Handy for testing, but you have to build
> an extra executable.

How about using an environment variable instead? You would set it in the
Automake variable TESTS_ENVIRONMENT.

Then you would not need an extra executable, and the individual tests in
your testsuite do not need to me modified.

Bruno




Re: reclaiming memory before exit, take 2

2020-05-16 Thread Tim Rühsen
Hi,

thanks for your excellent write-up, Bruno.

Just want to add my thoughts :)

When talking about finding memory leaks, like in your wiki page, you
should also mention static analyzers and their ability to find memory
leaks (clang static analyzer, gcc-10 -fanalyzer, Coverity, cppcheck, ...).

These are able to find leaks (and much more) even in code that is seldom
used during runtime (and perhaps never during testing).
To find all those leaks with runtime tools like valgrind, you need  a
very high code coverage in your test suite. Even with a good coverage
you  miss *code paths* during your tests. So you better use coverage
guided fuzzing *and* static analysis in combination.

Another point is: a library exit / cleanup call before exiting the
application can easily free all reachable memory and by that disguise or
hide internal memory leaks that lead to OOM in a long-running
application (daemon or e.g. wget -r).
In these cases it may be helpful to *not* free memory and not call exit
library functions before exiting the application. Only then could
valgrind show the memory hog (maybe the --xtree-memory option is for
exactly that - I didn't investigate it yet).

Hmmm, while writing the above I was interrupted *five* times here... so
please let me know if it sounds a bit weird :).

Regards, Tim

On 16.05.20 16:04, Bruno Haible wrote:
> Hi all,
> 
> Let me try to bring some structure into this discussion.
> 
> 1) The memory-leak tools
> 2) The developer's perspective
> 3) The QA automation perspective
> 
> 
> 1) The memory-leak tools
> 
> 
> Paul and I apparently use the tools differently [1]. So, I've written
> a wiki page
>   https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks
> that describes what memory leaks are, what tools exist to find them,
> what are advantages and drawbacks. Please read it.
> 
> (If you want to contribute to this wiki, drop me a note, and I'll give
> you write access.)
> 
> In particular, all three tools by default omit memory leaks caused by
> global and static variables.
> 
> Paul writes in [1]:
>> In any event, it seems to me to be a deficiency in the detection if it
>> reports allocated memory which is still referenced to by global
>> variables, or even static variables, as memory leaks.
> 
> On the contrary, reporting leaks through global and static variables is
> a feature. *Hiding* them, which is what the tools do by default, is a
> *misfeature*.
> 
> Why? Because caches that grow without bounds are serious memory leaks
> as well. It is pointless to eliminate serious memory leaks that were
> caused by forgetting free(), while at the same not noticing serious memory
> leaks that were caused by bad design of caches. BOTH have same adverse
> effects.
> 
> This means, when looking for memory leaks, you should use valgrind with
> '--show-reachable=yes'.
> 
> 
> 2) The developer's perspective
> ==
> 
> A developer will want to eliminate all serious memory leaks and not spend
> much time on the not-serious ones. However:
> 
>   The tools cannot distinguish serious from not serious memory leaks.
> 
> Therefore the developer will typically want to silence the not serious
> memory leaks (like they also want to silence "gcc -Wall" warnings when
> they are pointless).
> 
> I can see three good and one bad ways of doing so:
> 
> * [good] Enhance the suppressions file(s).
> 
> * [good] In unit tests, add free() statements right before exit(). Like
>   Paul said in [2]:
> "tests should not contain those types of memory leaks and if someone
>  comes along with a fix, it should be applied."
> 
> * [acceptable] In production binaries, add free() statements guarded by an
>   environment variable test:
> 
> if (getenv ("CLEANUP_BEFORE_EXIT") != NULL)
>   free (data);
> exit (0);
> 
>   Removing the getenv test would, however, violate the GNU standards [3].
> 
> * [bad] Omit the valgrind option '--show-reachable=yes'.
>   This is bad because, as explained above, it will make you blind against
>   some types of serious memory leaks.
> 
> 
> 3) The QA automation perspective
> 
> 
> For QA automation, a multitude of program executions are done with a
> memory leak checker enabled. Since it needs to be automated, the usual
> policy will be that a valgrind or leak sanitizer report is considered
> a failure.
> 
> The QA automation needs to rely on the suppression files created by the
> developers, since it's not a QA engineer's job to evaluate whether a
> particular memory leak is serious or not.
> 
> A good QA automation will thus use 'valgrind --show-reachable=yes'
> and the developer's suppression files.
> 
> 
> Bruno
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00154.html
> [2] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00149.html
> [3] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html
> 
> 



signature.asc
Description: OpenPGP 

Re: reclaiming memory before exit, take 2

2020-05-16 Thread Tim Rühsen
On 16.05.20 17:33, Jeffrey Walton wrote:
> On Sat, May 16, 2020 at 10:05 AM Bruno Haible  wrote:
>> 3) The QA automation perspective
>> 
>>
>> For QA automation, a multitude of program executions are done with a
>> memory leak checker enabled. Since it needs to be automated, the usual
>> policy will be that a valgrind or leak sanitizer report is considered
>> a failure.
>>
>> The QA automation needs to rely on the suppression files created by the
>> developers, since it's not a QA engineer's job to evaluate whether a
>> particular memory leak is serious or not.
>>
>> A good QA automation will thus use 'valgrind --show-reachable=yes'
>> and the developer's suppression files.
>>
>>
> 
> GNU values choice for the user.
> 
> Perhaps GNU should provide a configuration option
> --enable-memory-leaks for those who want them. The rest of the world
> can enjoy a clean module that can pass through code quality and
> security gates.

Free'ing memory can cost *a lot* of CPU. In the best case this is
completely superfluous, in most cases this leads to a slowdown of
software, to increased energy consumption, to higher costs, and produces
more CO2 than needed.

So by default, applications should not free memory before exit.

But I agree that people who want to test for memory leaks (and most
developers want this for their own projects) should have an easy way to
do this.

At GNU wget we have conditional cleanup functions. That is compilation
with -DDEBUG_MALLOC in $CFLAGS will add those cleanup functions and they
are called before wget exits. Handy for testing, but you have to build
an extra executable.

If the GNU project could agree on a standard regarding such a flag or
even a command line option - I wouldn't be against it.

Regards, Tim



signature.asc
Description: OpenPGP digital signature


Re: reclaiming memory before exit, take 2

2020-05-16 Thread Paul Eggert
Thanks for the summary.

On 5/16/20 7:04 AM, Bruno Haible wrote:

> Paul writes in [1]:
>> In any event, it seems to me to be a deficiency in the detection if it
>> reports allocated memory which is still referenced to by global
>> variables, or even static variables, as memory leaks.
> 
> On the contrary, reporting leaks through global and static variables is
> a feature. *Hiding* them, which is what the tools do by default, is a
> *misfeature*.

This depends on what sort of leaks you're looking for. I agree that these are
very often leaks, and that it generally makes sense to look for them. However, I
don't think we always need to worry about these leaks just before program exit,
since they are not a performance problem there and introducing 'free' there both
complicates and slows the program.

This underlying issue dates back at least to the 1960s, by the way. PL/I has
based pointers, and you needn't free the objects they point to if you delete the
containing area ("area" is PL/I-ese for "heap").
> The QA automation needs to rely on the suppression files created by the
> developers, since it's not a QA engineer's job to evaluate whether a
> particular memory leak is serious or not.

This division of labor is not universal. In many shops, QA engineers are more
involved in determining the seriousness of bugs, and they can maintain
suppression files.



Re: reclaiming memory before exit, take 2

2020-05-16 Thread Jeffrey Walton
On Sat, May 16, 2020 at 10:05 AM Bruno Haible  wrote:
>
> Hi all,
>
> Let me try to bring some structure into this discussion.
>
> 1) The memory-leak tools
> 2) The developer's perspective
> 3) The QA automation perspective
>
>
> 1) The memory-leak tools
> 
>
> Paul and I apparently use the tools differently [1]. So, I've written
> a wiki page
>   https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks
> that describes what memory leaks are, what tools exist to find them,
> what are advantages and drawbacks. Please read it.
>
> (If you want to contribute to this wiki, drop me a note, and I'll give
> you write access.)
>
> In particular, all three tools by default omit memory leaks caused by
> global and static variables.
>
> Paul writes in [1]:
> > In any event, it seems to me to be a deficiency in the detection if it
> > reports allocated memory which is still referenced to by global
> > variables, or even static variables, as memory leaks.
>
> On the contrary, reporting leaks through global and static variables is
> a feature. *Hiding* them, which is what the tools do by default, is a
> *misfeature*.
>
> Why? Because caches that grow without bounds are serious memory leaks
> as well. It is pointless to eliminate serious memory leaks that were
> caused by forgetting free(), while at the same not noticing serious memory
> leaks that were caused by bad design of caches. BOTH have same adverse
> effects.
>
> This means, when looking for memory leaks, you should use valgrind with
> '--show-reachable=yes'.
>
>
> 2) The developer's perspective
> ==
>
> A developer will want to eliminate all serious memory leaks and not spend
> much time on the not-serious ones. However:
>
>   The tools cannot distinguish serious from not serious memory leaks.
>
> Therefore the developer will typically want to silence the not serious
> memory leaks (like they also want to silence "gcc -Wall" warnings when
> they are pointless).
>
> I can see three good and one bad ways of doing so:
>
> * [good] Enhance the suppressions file(s).
>
> * [good] In unit tests, add free() statements right before exit(). Like
>   Paul said in [2]:
> "tests should not contain those types of memory leaks and if someone
>  comes along with a fix, it should be applied."
>
> * [acceptable] In production binaries, add free() statements guarded by an
>   environment variable test:
>
> if (getenv ("CLEANUP_BEFORE_EXIT") != NULL)
>   free (data);
> exit (0);
>
>   Removing the getenv test would, however, violate the GNU standards [3].
>
> * [bad] Omit the valgrind option '--show-reachable=yes'.
>   This is bad because, as explained above, it will make you blind against
>   some types of serious memory leaks.
>
>
> 3) The QA automation perspective
> 
>
> For QA automation, a multitude of program executions are done with a
> memory leak checker enabled. Since it needs to be automated, the usual
> policy will be that a valgrind or leak sanitizer report is considered
> a failure.
>
> The QA automation needs to rely on the suppression files created by the
> developers, since it's not a QA engineer's job to evaluate whether a
> particular memory leak is serious or not.
>
> A good QA automation will thus use 'valgrind --show-reachable=yes'
> and the developer's suppression files.
>
>

GNU values choice for the user.

Perhaps GNU should provide a configuration option
--enable-memory-leaks for those who want them. The rest of the world
can enjoy a clean module that can pass through code quality and
security gates.

Jeff



findprog-lgpl: fix link error

2020-05-16 Thread Bruno Haible
The module 'findprog-lgpl' is pretty broken, since it was introduced on
2008-09-02:
  - It never produces any code.
  - When used together with the module 'findprog', that module produces no code
either.
In both cases, a link error regarding the symbol 'find_in_path' results.

This patch fixes it.


2020-05-16  Bruno Haible  

findprog-lgpl: Fix link error (existing since 2008-09-02).
* modules/findprog-lgpl (Makefile.am): Arrange to compile
findprog-lgpl.c, not findprog.c.
* lib/findprog.c (find_in_path): Add LGPLed replacement code for
XNMALLOC.

diff --git a/lib/findprog.c b/lib/findprog.c
index b562e9d..fb76417 100644
--- a/lib/findprog.c
+++ b/lib/findprog.c
@@ -122,7 +122,17 @@ find_in_path (const char *progname)
   /* Add the "./" prefix for real, that 
xconcatenated_filename()
  optimized away.  This avoids a second PATH search when the
  caller uses execlp/execvp.  */
+# if !IN_FINDPROG_LGPL
   progpathname = XNMALLOC (2 + strlen (progname) + 1, char);
+# else
+  progpathname = (char *) malloc (2 + strlen (progname) + 1);
+  if (progpathname == NULL)
+{
+  /* Out of memory.  */
+  free (path);
+  return progname;
+}
+# endif
   progpathname[0] = '.';
   progpathname[1] = '/';
   memcpy (progpathname + 2, progname, strlen (progname) + 1);
diff --git a/modules/findprog-lgpl b/modules/findprog-lgpl
index 3c56f02..658183d 100644
--- a/modules/findprog-lgpl
+++ b/modules/findprog-lgpl
@@ -22,7 +22,7 @@ gl_FINDPROG
 gl_MODULE_INDICATOR([findprog-lgpl])
 
 Makefile.am:
-lib_SOURCES += findprog.h findprog.c
+lib_SOURCES += findprog.h findprog-lgpl.c
 
 Include:
 "findprog.h"




Re: c-stack.c and DEBUG: missing import

2020-05-16 Thread Marc Nieper-Wißkirchen
Thank you, both of you!

Have a nice weekend,

Marc

Am Fr., 15. Mai 2020 um 23:04 Uhr schrieb Bruno Haible :
>
> Hi Paul,
>
> > I don't think we need to go that far, since c-stack is already using
> > ignore_value. I installed the attached.
>
> You beat me to it by 2 minutes :)
>
> I verified that the patch fixes the warning that occurred with
>   ./configure CPPFLAGS="-Wall -DDEBUG" --with-libsigsegv-prefix=...
>
> Bruno
>



reclaiming memory before exit, take 2

2020-05-16 Thread Bruno Haible
Hi all,

Let me try to bring some structure into this discussion.

1) The memory-leak tools
2) The developer's perspective
3) The QA automation perspective


1) The memory-leak tools


Paul and I apparently use the tools differently [1]. So, I've written
a wiki page
  https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks
that describes what memory leaks are, what tools exist to find them,
what are advantages and drawbacks. Please read it.

(If you want to contribute to this wiki, drop me a note, and I'll give
you write access.)

In particular, all three tools by default omit memory leaks caused by
global and static variables.

Paul writes in [1]:
> In any event, it seems to me to be a deficiency in the detection if it
> reports allocated memory which is still referenced to by global
> variables, or even static variables, as memory leaks.

On the contrary, reporting leaks through global and static variables is
a feature. *Hiding* them, which is what the tools do by default, is a
*misfeature*.

Why? Because caches that grow without bounds are serious memory leaks
as well. It is pointless to eliminate serious memory leaks that were
caused by forgetting free(), while at the same not noticing serious memory
leaks that were caused by bad design of caches. BOTH have same adverse
effects.

This means, when looking for memory leaks, you should use valgrind with
'--show-reachable=yes'.


2) The developer's perspective
==

A developer will want to eliminate all serious memory leaks and not spend
much time on the not-serious ones. However:

  The tools cannot distinguish serious from not serious memory leaks.

Therefore the developer will typically want to silence the not serious
memory leaks (like they also want to silence "gcc -Wall" warnings when
they are pointless).

I can see three good and one bad ways of doing so:

* [good] Enhance the suppressions file(s).

* [good] In unit tests, add free() statements right before exit(). Like
  Paul said in [2]:
"tests should not contain those types of memory leaks and if someone
 comes along with a fix, it should be applied."

* [acceptable] In production binaries, add free() statements guarded by an
  environment variable test:

if (getenv ("CLEANUP_BEFORE_EXIT") != NULL)
  free (data);
exit (0);

  Removing the getenv test would, however, violate the GNU standards [3].

* [bad] Omit the valgrind option '--show-reachable=yes'.
  This is bad because, as explained above, it will make you blind against
  some types of serious memory leaks.


3) The QA automation perspective


For QA automation, a multitude of program executions are done with a
memory leak checker enabled. Since it needs to be automated, the usual
policy will be that a valgrind or leak sanitizer report is considered
a failure.

The QA automation needs to rely on the suppression files created by the
developers, since it's not a QA engineer's job to evaluate whether a
particular memory leak is serious or not.

A good QA automation will thus use 'valgrind --show-reachable=yes'
and the developer's suppression files.


Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00154.html
[2] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00149.html
[3] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html