xasprintf and -Wanalyzer-possible-null-argument

[Bruno Haible]

Another warning from gcc13's -Wanalyzer-possible-null-argument, seen in
GNU gettext, is:

gettext-tools/gnulib-lib/javacomp.c:296:20: warning: use of possibly-NULL 
'javac' where non-null expected [CWE-690]

The problem here is that we use xasprintf with a format string such as
  "%s -source %s"
to construct a new string.

The possible error conditions of asprintf are:
  - ENOMEM when malloc() fails,
  - EOVERFLOW when the result is larger than INT_MAX bytes,
  - EINVAL if the format string is invalid,
  - EILSEQ if there is a conversion error in a %ls directive.

With a fixed string such as "%s -source %s", EINVAL and EILSEQ cannot occur.
ENOMEM is caught by xasprintf. But EOVERFLOW can still occur.

On the other hand, xasprintf was created with the purpose of simplifying
the application's code, by not requiring error handling.

The warning can be silenced through an 'assume' invocation, as in the patch
below. However, the justification for this 'assume' is specific to this file
and not universally generalizable: it depends on the fact that all involved
strings (values of environment variables, as well as file names) are
significantly smaller than 2 GiB.

What should we do in general?
  (a) Use -Wno-analyzer-possible-null-argument, to silence this warning.
  (b) Make xasprintf() fail also when the error condition is EOVERFLOW.
  (c) Change the implementation of xasprintf() in such a way that it will
      actually return a result > 2 GiB, and never fail with EOVERFLOW.

(a) is not good IMO, as this warning option often points to real bugs.

(b) would mean that a program exits with an error message, even though
there may be 20 GiB of RAM available for processing. In other words, it
would be arbitrary limit of 2 GiB, which is against the GNU Coding Standards
<https://www.gnu.org/prep/standards/html_node/Semantics.html> .

I'm therefore considering (c). Although it will mean that xasprintf
will depend on vasnprintf, even on glibc systems. What do you think?
Is this acceptable?


2023-06-02  Bruno Haible  <bruno@clisp.org>

        javacomp: Silence -Wanalyzer-possible-null-argument warning.
        * lib/javacomp.c: Include verify.h.
        (is_envjavac_gcj43_usable, is_envjavac_oldgcj_14_13_usable,
        is_envjavac_nongcj_usable, compile_java_class): Assert that the
        xasprintf results are non-NULL. This is possible since all involved
        format strings are valid and don't use %ls, and all argument strings
        are small compared to INT_MAX.
        * modules/javacomp (Depends-on): Add verify.

diff --git a/lib/javacomp.c b/lib/javacomp.c
index 802742dd95..bb4ef919f4 100644
--- a/lib/javacomp.c
+++ b/lib/javacomp.c
@@ -46,6 +46,7 @@
 #include "clean-temp.h"
 #include "error.h"
 #include "xvasprintf.h"
+#include "verify.h"
 #include "c-strstr.h"
 #include "gettext.h"
 
@@ -849,6 +850,7 @@ is_envjavac_gcj43_usable (const char *javac,
           /* Try adding -fsource option if it is useful.  */
           char *javac_source =
             xasprintf ("%s -fsource=%s", javac, source_version);
+          assume (javac_source != NULL);
 
           unlink (compiled_file_name);
 
@@ -917,6 +919,7 @@ is_envjavac_gcj43_usable (const char *javac,
           char *javac_target =
             xasprintf ("%s -fsource=%s -ftarget=%s",
                        javac, source_version, target_version);
+          assume (javac_target != NULL);
 
           unlink (compiled_file_name);
 
@@ -1060,6 +1063,7 @@ is_envjavac_oldgcj_14_13_usable (const char *javac,
       unlink (compiled_file_name);
 
       javac_noassert = xasprintf ("%s -fno-assert", javac);
+      assume (javac_noassert != NULL);
 
       java_sources[0] = conftest_file_name;
       if (!compile_using_envjavac (javac_noassert,
@@ -1200,6 +1204,7 @@ is_envjavac_nongcj_usable (const char *javac,
           /* Try adding -source option if it is useful.  */
           char *javac_source =
             xasprintf ("%s -source %s", javac, source_version_for_javac);
+          assume (javac_source != NULL);
 
           unlink (compiled_file_name);
 
@@ -1268,6 +1273,7 @@ is_envjavac_nongcj_usable (const char *javac,
              option but no -source option.)  */
           char *javac_target =
             xasprintf ("%s -target %s", javac, target_version);
+          assume (javac_target != NULL);
 
           unlink (compiled_file_name);
 
@@ -1284,6 +1290,7 @@ is_envjavac_nongcj_usable (const char *javac,
               /* Try adding -source option if it is useful.  */
               char *javac_target_source =
                 xasprintf ("%s -source %s", javac_target, 
source_version_for_javac);
+              assume (javac_target_source != NULL);
 
               unlink (compiled_file_name);
 
@@ -1358,6 +1365,7 @@ is_envjavac_nongcj_usable (const char *javac,
                  higher.)  */
               char *javac_target_source =
                 xasprintf ("%s -source %s", javac_target, 
source_version_for_javac);
+              assume (javac_target_source != NULL);
 
               unlink (compiled_file_name);
 
@@ -2267,6 +2275,7 @@ compile_java_class (const char * const *java_sources,
                             fsource_option ? source_version : "",
                             ftarget_option ? " -ftarget=" : "",
                             ftarget_option ? target_version : ""));
+            assume (javac_with_options != NULL);
 
             err = compile_using_envjavac (javac_with_options,
                                           java_sources, java_sources_count,
diff --git a/modules/javacomp b/modules/javacomp
index cdd6473c4e..f3ce944100 100644
--- a/modules/javacomp
+++ b/modules/javacomp
@@ -28,6 +28,7 @@ clean-temp
 stat
 error
 xvasprintf
+verify
 c-strstr
 gettext-h
 javacomp-script