Mass filing of clang warnings

[Werner Koch]

Hi!

While looking at the bug tracker, I figured that a few days ago one guy
filed dozens of bugs which in most cases are warnings issued by Clang. I
closed most of them and tagged them as mistaken because pasting just
warnings along with comments like "makes me nervous" do not make good bug
reports.  It took me more than 2 hours to close them and briefly check
some of them.  I might have missed some real things due to that torrent
of reports, though. 

An example of faulty warnings is

  https://bugs.g10code.com/gnupg/issue1912

where one of the warnings is

iobuf.c:1325:3: warning: String copy function overflows destination buffer
  strcpy (fcx->fname, fname);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~

Looking at the code in question:

--8<---------------cut here---------------start------------->8---
  fcx = xmalloc (sizeof *fcx + strlen (fname));
  fcx->fp = fp;
  fcx->print_only_name = print_only;
  strcpy (fcx->fname, fname);
--8<---------------cut here---------------end--------------->8---

the last line is 1325.  It is pretty easy to see that FCX->FNAME has
been allocated 3 lines earlier using a standard C pattern to use a
dynamically sized struct (for FCX):

--8<---------------cut here---------------start------------->8---
  typedef struct
  {
    gnupg_fd_t fp;       /* Open file pointer or handle.  */
    int keep_open;
    int no_cache;
    int eof_seen;
    int print_only_name; /* Flags indicating that fname [...]  */
    char fname[1];       /* Name of the file.  */
  } file_filter_ctx_t;
--8<---------------cut here---------------end--------------->8---

The xmalloc (which is a common name for a malloc wrapper) allocates the
size of the struct including the one byte for fname (from the
definition) and adds the strlen of the SOURCE string to the allocation
size.  Thus the total allocation for FCX->FNAME is

 (1 + strlen(SOURCE))

and a final strcpy works as expected without any overflow.

Now, how useful is an analyzer which does not grok one of the oldest C
coding pattern?  Frankly, I have not seen that before and thus I assume
that this and other warnings are due to a regression in that version of
Clang or due to wrong use.


Salam-Shalom,

   Werner


-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.