gearman / gearmand

http://gearman.org/
Other
736 stars 138 forks source link

Clang static analyzer warnings #222

Open esabol opened 5 years ago

esabol commented 5 years ago

References: https://clang-analyzer.llvm.org/scan-build.html https://www.mankier.com/1/scan-build

Over in my fork, I added scan-build -v --use-cc="${CC}" --use-c++="${CXX}" prior to the ./configure and make lines to the clang-5.0 build in Travis CI. Here are the resulting warnings:

libgearman/task.cc:62:5: warning: Potential leak of memory pointed to by 'task'
    return task->shell();
    ^~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman/server_options.cc:61:10: warning: Potential leak of memory pointed to by 'server_options'
  return true;
         ^~~~
1 warning generated.

libgearman/universal.cc:137:22: warning: Attempt to delete released memory
    delete universal.server_options_list;
                     ^
libgearman/universal.cc:216:22: warning: Attempt to delete released memory
    delete universal.con_list;
                     ^
2 warnings generated.

libgearman/worker.cc:1255:11: warning: Potential leak of memory pointed to by 'worker'
          gearman_worker_free(worker->shell());
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman/packet.cc:414:15: warning: Dereference of null pointer (loaded from variable 'ptr')
          *ptr= 0;
           ~~~^
1 warning generated.

libgearman/server_options.cc:61:10: warning: Potential leak of memory pointed to by 'server_options'
  return true;
         ^~~~
1 warning generated.

libgearman/universal.cc:137:22: warning: Attempt to delete released memory
    delete universal.server_options_list;
                     ^
libgearman/universal.cc:216:22: warning: Attempt to delete released memory
    delete universal.con_list;
                     ^
2 warnings generated.

libhashkit/hashkit.cc:95:7: warning: Dereference of null pointer
  if (hashkit_is_allocated(self))
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
./libhashkit/is.h:41:40: note: expanded from macro 'hashkit_is_allocated'
#define hashkit_is_allocated(__object) ((__object)->options.is_allocated)
                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman/add.cc:491:13: warning: Access to field 'type' results in a dereference of a null pointer (loaded from variable 'task')
  task->type= GEARMAN_TASK_KIND_EXECUTE;
  ~~~~      ^
1 warning generated.

libgearman/client.cc:77:5: warning: Potential leak of memory pointed to by 'client'
    return client->shell();
    ^~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman/packet.cc:414:15: warning: Dereference of null pointer (loaded from variable 'ptr')
          *ptr= 0;
           ~~~^
1 warning generated.

libgearman/task.cc:62:5: warning: Potential leak of memory pointed to by 'task'
    return task->shell();
    ^~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman/server_options.cc:61:10: warning: Potential leak of memory pointed to by 'server_options'
  return true;
         ^~~~
1 warning generated.

libgearman/universal.cc:137:22: warning: Attempt to delete released memory
    delete universal.server_options_list;
                     ^
libgearman/universal.cc:216:22: warning: Attempt to delete released memory
    delete universal.con_list;
                     ^
2 warnings generated.

libgearman/worker.cc:1255:11: warning: Potential leak of memory pointed to by 'worker'
          gearman_worker_free(worker->shell());
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libtest/client.cc:367:15: warning: Value stored to 'write_size' is never read
              write_size= SOCKET_ERROR;
              ^           ~~~~~~~~~~~~
1 warning generated.

libgearman/add.cc:491:13: warning: Access to field 'type' results in a dereference of a null pointer (loaded from variable 'task')
  task->type= GEARMAN_TASK_KIND_EXECUTE;
  ~~~~      ^
1 warning generated.

libgearman/client.cc:77:5: warning: Potential leak of memory pointed to by 'client'
    return client->shell();
    ^~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman/packet.cc:414:15: warning: Dereference of null pointer (loaded from variable 'ptr')
          *ptr= 0;
           ~~~^
1 warning generated.
libhashkit/hashkit.cc:95:7: warning: Dereference of null pointer
  if (hashkit_is_allocated(self))
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
./libhashkit/is.h:41:40: note: expanded from macro 'hashkit_is_allocated'
#define hashkit_is_allocated(__object) ((__object)->options.is_allocated)
                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman-server/plugins/queue/libmemcached/queue.cc:162:10: warning: Potential leak of memory pointed to by 'exec_queue'
  return gearmand_gerror("Libmemcached::initialize()", GEARMAND_QUEUE_ERROR);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./libgearman-server/log.h:86:51: note: expanded from macro 'gearmand_gerror'
#define gearmand_gerror(_mesg, _gearmand_errot_t) gearmand_log_gerror(GEARMAN_DEFAULT_LOG_PARAM, (_gearmand_errot_t), (_mesg))
                                                  ^~~~~~~~~~~~~~~~~~~
libgearman-server/plugins/queue/libmemcached/queue.cc:342:3: warning: Potential leak of memory pointed to by 'data'
  return MEMCACHED_SUCCESS;
  ^~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.

libgearman-server/plugins/queue/sqlite/instance.cc:577:9: warning: Potential leak of memory pointed to by 'data'
    gret= Instance::replay_add(server,
    ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman-server/gearmand_con.cc:670:30: warning: Potential leak of memory pointed to by 'dcon'
  gearmand->thread_add_next= gearmand->thread_add_next->next;
                             ^~~~~~~~
1 warning generated.

libgearman-server/io.cc:856:3: warning: Value stored to 'packet' is never read
  packet= connection->recv_packet;
  ^       ~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

libgearman-server/log.cc:245:9: warning: Value stored to 'remaining_size' is never read
        remaining_size= 0;
        ^               ~
1 warning generated.

libgearman-server/packet.cc:201:5: warning: Potential leak of memory pointed to by 'server_packet'
    gearmand_log_fatal_perror(GEARMAN_DEFAULT_LOG_PARAM, error, "pthread_mutex_lock");
    ^~~~~~~~~~~~~~~~~~~~~~~~~
libgearman-server/packet.cc:426:5: warning: Null pointer passed as an argument to a 'nonnull' parameter
    memcpy(packet->args, "\0REQ", 4);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libgearman-server/packet.cc:430:5: warning: Null pointer passed as an argument to a 'nonnull' parameter
    memcpy(packet->args, "\0RES", 4);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.

bin/arguments.cc:104:37: warning: Null pointer passed as an argument to a 'nonnull' parameter
      _count= static_cast<uint32_t>(atoi(optarg));
                                    ^~~~~~~~~~~~
bin/arguments.cc:141:37: warning: Null pointer passed as an argument to a 'nonnull' parameter
      _port= static_cast<in_port_t>(atoi(optarg));
                                    ^~~~~~~~~~~~
bin/arguments.cc:153:17: warning: Null pointer passed as an argument to a 'nonnull' parameter
      _timeout= atoi(optarg);
                ^~~~~~~~~~~~
bin/gearman.cc:624:11: warning: 1st function call argument is an uninitialized value
      if (dup2(out_fds[1], 1) == -1)
          ^~~~~~~~~~~~~~~~~~~
bin/gearman.cc:651:9: warning: 1st function call argument is an uninitialized value
    if (close(out_fds[1]) < 0)
        ^~~~~~~~~~~~~~~~~
2 warnings generated.

bin/error.cc:57:3: warning: Value stored to 'errmsg_ptr' is never read
  errmsg_ptr= errmsg;
  ^           ~~~~~~
1 warning generated.

At first glance, a lot of these warnings seem very spurious. Some of the "Potential leak of memory" warnings might be worth investigating, but I'm not seeing any glaring bugs. Any comments?

esabol commented 5 years ago

The clang static analyzer generates a bunch of HTML output files with more verbose information. Unfortunately, I haven't figured out a way to get them out of Travis CI and put them somewhere that's visible. Any suggestions?

esabol commented 5 years ago

For reference, here is how I modified the .travis.yml file:

--- a/.travis.yml
+++ b/.travis.yml
@@ -26,6 +26,11 @@ matrix:
         artifacts: true
       env:
         - MATRIX_EVAL="CC=clang-5.0 && CXX=clang++-5.0"
+      script:
+        - ./bootstrap.sh -a
+        - scan-build -v --use-cc="${CC}" --use-c++="${CXX}" ./configure --enable-ssl
+        - scan-build -v --use-cc="${CC}" --use-c++="${CXX}" make  
+        - make test
     - compiler: gcc
       addons:
         apt:
esabol commented 5 years ago

Based on some examples I saw, I added -enable-checker core -enable-checker unix -enable-checker cplusplus -enable-checker security arguments to scan-build. That resulted in a few more warnings, mainly about strcpy() usage at first glance. The full build log can be found at: https://travis-ci.org/esabol/gearmand/builds/461609044

esabol commented 5 years ago

Another reference: https://clang-analyzer.llvm.org/faq.html

p-alik commented 5 years ago

Do you think enhancement tag would be enough for the issue, @esabol?

esabol commented 5 years ago

Do you think enhancement tag would be enough for the issue, @esabol?

I'm not sure how to categorize this, but that sounds fine.