public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-7494] analyzer: reduce svalue depth limit from 13 to 12 [PR103521]
@ 2022-03-04 18:57 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-03-04 18:57 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:458ad38ce2bbec85016d88757ec6a35d2c393e2c

commit r12-7494-g458ad38ce2bbec85016d88757ec6a35d2c393e2c
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Mar 4 13:51:14 2022 -0500

    analyzer: reduce svalue depth limit from 13 to 12 [PR103521]
    
    PR analyzer/103521 reports that commit r12-5585-g132902177138c09803d639e12b1daebf2b9edddc
    ("analyzer: further false leak fixes due to overzealous state merging [PR103217]")
    led to failures of gcc.dg/analyzer/pr93032-mztools.c on some targets,
    where rather than reporting FILE * leaks, the analyzer would hit
    complexity limits and give up.
    
    The cause is that pr93032-mztools.c has some 'unsigned char' values that
    are copied to 'char'.  On targets where 'char' defaults to being signed,
    this leads to casts, whereas on targets where 'char' defaults to being
    unsigned, no casts are needed.
    
    When the casts occur, various symbolic values within the loop (the
    locals 'crc', 'cpsize', and 'uncpsize') become sufficiently complex as
    to hit the --param=analyzer-max-svalue-depth= limit, and are treated as
    UNKNOWN, allowing the analysis of the loop to quickly terminate, with
    much of this state as UNKNOWN (but retaining the FILE * information, and
    thus correctly reporting the FILE * leaks).
    
    Without the casts, the symbolic values for these variables don't quite
    hit the complexity limit, and the analyzer attempts to track these
    values in the loop, leading to the analyzer eventually hitting the
    per-program-point limit on the number of states, and giving up on
    these execution paths, thus failing to report the FILE * leaks.
    
    This patch tweaks the default value of the param:
      --param=analyzer-max-svalue-depth=.
    from 13 down to 12.  This allows the pr93032-mztools.c testcase to
    succeeed with both -fsigned-char and -funsigned-char, and thus allows
    this integration test to succeed on both styles of target without
    requiring extra command-line flags.  The patch duplicates the test so
    it runs with both -fsigned-char and -funsigned-char.
    
    My hope is that this will allow similar cases to terminate loop analysis
    earlier.  I tried reducing it further, but doing so caused some test
    cases to regress.
    
    The tradeoff here is between:
    (a) precision of individual states in the analysis, versus
    (b) maximizing code-path coverage in the analysis
    
    I can imagine a more nuanced approach that splits the current
    per-program-point hard limit into soft and hard limits: on hitting the
    soft limit at a program point, go into a less precise mode for states
    at that program point, in the hope that we can fully explore execution
    paths beyond it without hitting the hard limit, but this seems like
    GCC 13 material.
    
    Another possible future fix might be for the analysis plan to make an
    attempt to prioritize parts of the code in an enode budget, rather than
    setting the same hard limit uniformly across all program points.
    
    gcc/analyzer/ChangeLog:
            PR analyzer/103521
            * analyzer.opt (-param=analyzer-max-svalue-depth=): Reduce from 13
            to 12.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/103521
            * gcc.dg/analyzer/pr93032-mztools.c: Move to...
            * gcc.dg/analyzer/pr93032-mztools-signed-char.c: ...this, adding
            -fsigned-char to args, and...
            * gcc.dg/analyzer/pr93032-mztools-unsigned-char.c: ...copy to here,
            adding -funsigned-char to args.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

Diff:
---
 gcc/analyzer/analyzer.opt                          |   2 +-
 ...032-mztools.c => pr93032-mztools-signed-char.c} |   1 +
 .../analyzer/pr93032-mztools-unsigned-char.c       | 332 +++++++++++++++++++++
 3 files changed, 334 insertions(+), 1 deletion(-)

diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index d20ac5c34ee..b9d2ece273c 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -43,7 +43,7 @@ Common Joined UInteger Var(param_analyzer_max_recursion_depth) Init(2) Param
 The maximum number of times a callsite can appear in a call stack within the analyzer, before terminating analysis of a call that would recurse deeper.
 
 -param=analyzer-max-svalue-depth=
-Common Joined UInteger Var(param_analyzer_max_svalue_depth) Init(13) Param
+Common Joined UInteger Var(param_analyzer_max_svalue_depth) Init(12) Param
 The maximum depth of a symbolic value, before approximating the value as unknown.
 
 -param=analyzer-min-snodes-for-call-summary=
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-signed-char.c
similarity index 99%
rename from gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c
rename to gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-signed-char.c
index 88ab5bf8c18..1f3df7c211f 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-signed-char.c
@@ -4,6 +4,7 @@
    removed.  */
 
 /* { dg-do "compile" } */
+/* { dg-additional-options "-fsigned-char" } */
 
 /* Minimal replacement of system headers.  */
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-unsigned-char.c b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-unsigned-char.c
new file mode 100644
index 00000000000..db9678d1caa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-unsigned-char.c
@@ -0,0 +1,332 @@
+/* Integration test to ensure we issue FILE * leak diagnostics for
+   this particular non-trivial case.
+   Adapted from zlib/contrib/minizip/mztools.c, with all #includes
+   removed.  */
+
+/* { dg-do "compile" } */
+/* { dg-additional-options "-funsigned-char" } */
+
+/* Minimal replacement of system headers.  */
+
+typedef __SIZE_TYPE__ size_t;
+#define NULL ((void *) 0)
+
+typedef struct _IO_FILE FILE;
+extern FILE *fopen(const char *__restrict __filename,
+		   const char *__restrict __modes);
+extern size_t fread (void *__restrict __ptr, size_t __size,
+		     size_t __n, FILE *__restrict __stream);
+extern size_t fwrite (const void *__restrict __ptr, size_t __size,
+		      size_t __n, FILE *__restrict __s);
+extern int fclose (FILE *__stream);
+extern int remove (const char *__filename)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+extern void *malloc (size_t __size)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__malloc__));
+extern void free (void *__ptr)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+extern size_t strlen (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__pure__))
+  __attribute__ ((__nonnull__ (1)));
+
+/* Minimal replacement of zlib headers.  */
+
+#define ZEXPORT
+typedef unsigned long  uLong; /* 32 bits or more */
+#define Z_OK            0
+#define Z_ERRNO        (-1)
+#define Z_STREAM_ERROR (-2)
+#define Z_MEM_ERROR    (-4)
+
+/*
+  Additional tools for Minizip
+  Code: Xavier Roche '2004
+  License: Same as ZLIB (www.gzip.org)
+*/
+
+/* Code */
+
+#define READ_8(adr)  ((unsigned char)*(adr))
+#define READ_16(adr) ( READ_8(adr) | (READ_8(adr+1) << 8) )
+#define READ_32(adr) ( READ_16(adr) | (READ_16((adr)+2) << 16) )
+
+#define WRITE_8(buff, n) do { \
+  *((unsigned char*)(buff)) = (unsigned char) ((n) & 0xff); \
+} while(0)
+#define WRITE_16(buff, n) do { \
+  WRITE_8((unsigned char*)(buff), n); \
+  WRITE_8(((unsigned char*)(buff)) + 1, (n) >> 8); \
+} while(0)
+#define WRITE_32(buff, n) do { \
+  WRITE_16((unsigned char*)(buff), (n) & 0xffff); \
+  WRITE_16((unsigned char*)(buff) + 2, (n) >> 16); \
+} while(0)
+
+extern int ZEXPORT unzRepair(file, fileOut, fileOutTmp, nRecovered, bytesRecovered)
+const char* file;
+const char* fileOut;
+const char* fileOutTmp;
+uLong* nRecovered;
+uLong* bytesRecovered;
+{
+  int err = Z_OK;
+  FILE* fpZip = fopen(file, "rb");
+  FILE* fpOut = fopen(fileOut, "wb");
+  FILE* fpOutCD = fopen(fileOutTmp, "wb");
+  if (fpZip != NULL && fpOut != NULL) {
+    int entries = 0;
+    uLong totalBytes = 0;
+    char header[30];
+    char filename[1024];
+    char extra[1024];
+    int offset = 0;
+    int offsetCD = 0;
+    while ( fread(header, 1, 30, fpZip) == 30 ) {
+      int currentOffset = offset;
+
+      /* File entry */
+      if (READ_32(header) == 0x04034b50) {
+        unsigned int version = READ_16(header + 4);
+        unsigned int gpflag = READ_16(header + 6);
+        unsigned int method = READ_16(header + 8);
+        unsigned int filetime = READ_16(header + 10);
+        unsigned int filedate = READ_16(header + 12);
+        unsigned int crc = READ_32(header + 14); /* crc */
+        unsigned int cpsize = READ_32(header + 18); /* compressed size */
+        unsigned int uncpsize = READ_32(header + 22); /* uncompressed sz */
+        unsigned int fnsize = READ_16(header + 26); /* file name length */
+        unsigned int extsize = READ_16(header + 28); /* extra field length */
+        filename[0] = extra[0] = '\0';
+
+        /* Header */
+        if (fwrite(header, 1, 30, fpOut) == 30) {
+          offset += 30;
+        } else {
+          err = Z_ERRNO;
+          break;
+        }
+
+        /* Filename */
+        if (fnsize > 0) {
+          if (fnsize < sizeof(filename)) {
+            if (fread(filename, 1, fnsize, fpZip) == fnsize) {
+                if (fwrite(filename, 1, fnsize, fpOut) == fnsize) {
+                offset += fnsize;
+              } else {
+                err = Z_ERRNO;
+                break;
+              }
+            } else {
+              err = Z_ERRNO;
+              break;
+            }
+          } else {
+            err = Z_ERRNO;
+            break;
+          }
+        } else {
+          err = Z_STREAM_ERROR;
+          break;
+        }
+
+        /* Extra field */
+        if (extsize > 0) {
+          if (extsize < sizeof(extra)) {
+            if (fread(extra, 1, extsize, fpZip) == extsize) {
+              if (fwrite(extra, 1, extsize, fpOut) == extsize) {
+                offset += extsize;
+                } else {
+                err = Z_ERRNO;
+                break;
+              }
+            } else {
+              err = Z_ERRNO;
+              break;
+            }
+          } else {
+            err = Z_ERRNO;
+            break;
+          }
+        }
+
+        /* Data */
+        {
+          int dataSize = cpsize;
+          if (dataSize == 0) {
+            dataSize = uncpsize;
+          }
+          if (dataSize > 0) {
+            char* data = malloc(dataSize);
+            if (data != NULL) {
+              if ((int)fread(data, 1, dataSize, fpZip) == dataSize) {
+                if ((int)fwrite(data, 1, dataSize, fpOut) == dataSize) {
+                  offset += dataSize;
+                  totalBytes += dataSize;
+                } else {
+                  err = Z_ERRNO;
+                }
+              } else {
+                err = Z_ERRNO;
+              }
+              free(data);
+              if (err != Z_OK) {
+                break;
+              }
+            } else {
+              err = Z_MEM_ERROR;
+              break;
+            }
+          }
+        }
+
+        /* Central directory entry */
+        {
+          char header[46];
+          char* comment = "";
+          int comsize = (int) strlen(comment);
+          WRITE_32(header, 0x02014b50);
+          WRITE_16(header + 4, version);
+          WRITE_16(header + 6, version);
+          WRITE_16(header + 8, gpflag);
+          WRITE_16(header + 10, method);
+          WRITE_16(header + 12, filetime);
+          WRITE_16(header + 14, filedate);
+          WRITE_32(header + 16, crc);
+          WRITE_32(header + 20, cpsize);
+          WRITE_32(header + 24, uncpsize);
+          WRITE_16(header + 28, fnsize);
+          WRITE_16(header + 30, extsize);
+          WRITE_16(header + 32, comsize);
+          WRITE_16(header + 34, 0);     /* disk # */
+          WRITE_16(header + 36, 0);     /* int attrb */
+          WRITE_32(header + 38, 0);     /* ext attrb */
+          WRITE_32(header + 42, currentOffset);
+          /* Header */
+          if (fwrite(header, 1, 46, fpOutCD) == 46) {
+            offsetCD += 46;
+
+            /* Filename */
+            if (fnsize > 0) {
+              if (fwrite(filename, 1, fnsize, fpOutCD) == fnsize) {
+                offsetCD += fnsize;
+              } else {
+                err = Z_ERRNO;
+                break;
+              }
+            } else {
+              err = Z_STREAM_ERROR;
+              break;
+            }
+
+            /* Extra field */
+            if (extsize > 0) {
+              if (fwrite(extra, 1, extsize, fpOutCD) == extsize) {
+                offsetCD += extsize;
+              } else {
+                err = Z_ERRNO;
+                break;
+              }
+            }
+
+            /* Comment field */
+            if (comsize > 0) {
+              if ((int)fwrite(comment, 1, comsize, fpOutCD) == comsize) {
+                offsetCD += comsize;
+              } else {
+                err = Z_ERRNO;
+                break;
+              }
+            }
+
+
+          } else {
+            err = Z_ERRNO;
+            break;
+          }
+        }
+
+        /* Success */
+        entries++;
+
+      } else {
+        break;
+      }
+    }
+
+    /* Final central directory  */
+    {
+      int entriesZip = entries;
+      char header[22];
+      char* comment = ""; // "ZIP File recovered by zlib/minizip/mztools";
+      int comsize = (int) strlen(comment);
+      if (entriesZip > 0xffff) {
+        entriesZip = 0xffff;
+      }
+      WRITE_32(header, 0x06054b50);
+      WRITE_16(header + 4, 0);    /* disk # */
+      WRITE_16(header + 6, 0);    /* disk # */
+      WRITE_16(header + 8, entriesZip);   /* hack */
+      WRITE_16(header + 10, entriesZip);  /* hack */
+      WRITE_32(header + 12, offsetCD);    /* size of CD */
+      WRITE_32(header + 16, offset);      /* offset to CD */
+      WRITE_16(header + 20, comsize);     /* comment */
+
+      /* Header */
+      if (fwrite(header, 1, 22, fpOutCD) == 22) {
+
+        /* Comment field */
+        if (comsize > 0) {
+          if ((int)fwrite(comment, 1, comsize, fpOutCD) != comsize) {
+            err = Z_ERRNO;
+          }
+        }
+
+      } else {
+        err = Z_ERRNO;
+      }
+    }
+
+    /* Final merge (file + central directory) */
+    fclose(fpOutCD);
+    if (err == Z_OK) {
+      fpOutCD = fopen(fileOutTmp, "rb");
+      if (fpOutCD != NULL) {
+        int nRead;
+        char buffer[8192];
+        while ( (nRead = (int)fread(buffer, 1, sizeof(buffer), fpOutCD)) > 0) {
+          if ((int)fwrite(buffer, 1, nRead, fpOut) != nRead) {
+            err = Z_ERRNO;
+            break;
+          }
+        }
+        fclose(fpOutCD);
+      }
+    }
+
+    /* Close */
+    fclose(fpZip);
+    fclose(fpOut);
+
+    /* Wipe temporary file */
+    (void)remove(fileOutTmp);
+
+    /* Number of recovered entries */
+    if (err == Z_OK) {
+      if (nRecovered != NULL) {
+        *nRecovered = entries;
+      }
+      if (bytesRecovered != NULL) {
+        *bytesRecovered = totalBytes;
+      }
+    }
+  } else {
+    err = Z_STREAM_ERROR;
+  }
+  return err; /* { dg-warning "leak of FILE 'fpZip'" "leak of fpZip" } */
+  /* { dg-warning "leak of FILE 'fpOut'" "leak of fpOut" { target *-*-* } .-1 } */
+  /* { dg-warning "leak of FILE 'fpOutCD'" "leak of fpOutCD" { target *-*-* } .-2 } */
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-03-04 18:57 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-04 18:57 [gcc r12-7494] analyzer: reduce svalue depth limit from 13 to 12 [PR103521] David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).