Merged Michael's valgrind branch

documentation/RELEASE_NOTES.html

@@ -20,6 +20,29 @@
 
 -->
 
+<h3>Valgrind Instrumentation</h3>
+
+<p>Valgrind is a software debugging suite provided by many Linux distributions.
+The header valgrind/valgrind.h is now included in, and installed by, Base.
+When included by a C or C++ source file this header defines some macros which
+expand to provide hints to the Valgrind runtime.
+These have no effect on normal operation of the software, but when run using the
+valgrind tool they can help to find memory leaks and buffer overflows.
+Suitable hints have been added to several free-lists within libCom, including
+freeListLib, allowing valgrind to provide more accurate information about the
+source of potential leaks.</p>
+
+<p>valgrind.h automatically disables itself when the build target is not
+supported by the valgrind tool.
+It can also explicitly be disabled by defining the macro <em>NVALGRIND</em>.
+See <em>src/libCom/Makefile</em> for a commented-out example.</p>
+
+<p>As a matter of policy valgrind.h should never be included by any header file
+installed by Base, so its use will remain purely an internal implementation
+detail and not be directly visible to application software.
+Support modules which choose to use valgrind.h are advised to avoid to do
+likewise.</p>
+
 <h3>Database Multi-locking</h3>
 
 <p>dbLock.c is re-written with an expanded API, and the removal of global mutex locks.</p>

src/libCom/Makefile

@@ -9,9 +9,14 @@
 TOP = ../..
 include $(TOP)/configure/CONFIG
 
+# Uncomment this to remove the (benign) valgrind helper stubs
+#USR_CFLAGS += -DNVALGRIND
+
 SRC = $(TOP)/src
 LIBCOM = $(SRC)/libCom
 
+INC += valgrind/valgrind.h
+
 include $(LIBCOM)/as/Makefile
 include $(LIBCOM)/bucketLib/Makefile
 include $(LIBCOM)/calc/Makefile

src/libCom/dbmf/dbmf.c

@@ -19,6 +19,15 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "valgrind/valgrind.h"
+
+#ifndef NVALGRIND
+/* buffer around allocations to detect out of bounds access */
+#define REDZONE sizeof(double)
+#else
+#define REDZONE 0
+#endif
+
 #define epicsExportSharedSymbols
 #include "epicsMutex.h"
 #include "ellLib.h"
@@ -70,13 +79,17 @@ int epicsShareAPI dbmfInit(size_t size, int chunkItems)
     pdbmfPvt->lock = epicsMutexMustCreate();
     /*allign to at least a double*/
     pdbmfPvt->size = size + size%sizeof(double);
-    pdbmfPvt->allocSize = pdbmfPvt->size + sizeof(itemHeader);
+    /* layout is
+     * | itemHeader | REDZONE | size | REDZONE |
+     */
+    pdbmfPvt->allocSize = pdbmfPvt->size + sizeof(itemHeader) + 2*REDZONE;
     pdbmfPvt->chunkItems = chunkItems;
     pdbmfPvt->chunkSize = pdbmfPvt->allocSize * pdbmfPvt->chunkItems;
     pdbmfPvt->nAlloc = 0;
     pdbmfPvt->nFree = 0;
     pdbmfPvt->nGtSize = 0;
     pdbmfPvt->freeList = NULL;
+    VALGRIND_CREATE_MEMPOOL(pdbmfPvt, REDZONE, 0);
     return(0);
 }
 
@@ -124,7 +137,7 @@ void* epicsShareAPI dbmfMalloc(size_t size)
         pitemHeader = (itemHeader *)pnextFree;
         pitemHeader->pchunkNode->nNotFree += 1;
     } else {
-        pmem = malloc(sizeof(itemHeader) + size);
+        pmem = malloc(sizeof(itemHeader) + 2*REDZONE + size);
         if(!pmem) {
             epicsMutexUnlock(pdbmfPvt->lock);
             printf("dbmfMalloc malloc failed\n");
@@ -133,12 +146,14 @@ void* epicsShareAPI dbmfMalloc(size_t size)
         pdbmfPvt->nAlloc++;
         pdbmfPvt->nGtSize++;
         pitemHeader = (itemHeader *)pmem;
-        pitemHeader->pchunkNode = NULL;
+        pitemHeader->pchunkNode = NULL; /* not part of free list */
         if(dbmfDebug) printf("dbmfMalloc: size %lu mem %p\n",
                              (unsigned long)size,pmem);
     }
     epicsMutexUnlock(pdbmfPvt->lock);
-    return((void *)(pmem + sizeof(itemHeader)));
+    pmem += sizeof(itemHeader) + REDZONE;
+    VALGRIND_MEMPOOL_ALLOC(pdbmfPvt, pmem, size);
+    return((void *)pmem);
 }
 
 char * epicsShareAPI dbmfStrdup(unsigned char *str)
@@ -160,7 +175,8 @@ void epicsShareAPI dbmfFree(void* mem)
         printf("dbmfFree called but dbmfInit never called\n");
         return;
     }
-    pmem -= sizeof(itemHeader);
+    VALGRIND_MEMPOOL_FREE(pdbmfPvt, mem);
+    pmem -= sizeof(itemHeader) + REDZONE;
     epicsMutexMustLock(pdbmfPvt->lock);
     pitemHeader = (itemHeader *)pmem;
     if(!pitemHeader->pchunkNode) {

src/libCom/freeList/freeListLib.c

@@ -15,6 +15,15 @@
 #include <stdlib.h>
 #include <stddef.h>
 
+#include "valgrind/valgrind.h"
+
+#ifndef NVALGRIND
+/* buffer around allocations to detect out of bounds access */
+#define REDZONE sizeof(double)
+#else
+#define REDZONE 0
+#endif
+
 #define epicsExportSharedSymbols
 #include "cantProceed.h"
 #include "epicsMutex.h"
@@ -47,6 +56,7 @@ epicsShareFunc void epicsShareAPI
     pfl->nBlocksAvailable = 0u;
     pfl->lock = epicsMutexMustCreate();
     *ppvt = (void *)pfl;
+    VALGRIND_CREATE_MEMPOOL(pfl, REDZONE, 0);
     return;
 }
 
@@ -78,7 +88,14 @@ epicsShareFunc void * epicsShareAPI freeListMalloc(void *pvt)
     epicsMutexMustLock(pfl->lock);
     ptemp = pfl->head;
     if(ptemp==0) {
-        ptemp = (void *)malloc(pfl->nmalloc*pfl->size);
+        /* layout of each block. nmalloc+1 REDZONEs for nmallocs.
+         * The first sizeof(void*) bytes are used to store a pointer
+         * to the next free block.
+         *
+         * | RED | size0 ------ | RED | size1 | ... | RED |
+         * |     | next | ----- |
+         */
+        ptemp = (void *)malloc(pfl->nmalloc*(pfl->size+REDZONE)+REDZONE);
         if(ptemp==0) {
             epicsMutexUnlock(pfl->lock);
             return(0);
@@ -89,15 +106,17 @@ epicsShareFunc void * epicsShareAPI freeListMalloc(void *pvt)
             free(ptemp);
             return(0);
         }
-        pallocmem->memory = ptemp;
+        pallocmem->memory = ptemp; /* real allocation */
+        ptemp += REDZONE; /* skip first REDZONE */
         if(pfl->mallochead)
             pallocmem->next = pfl->mallochead;
         pfl->mallochead = pallocmem;
         for(i=0; i<pfl->nmalloc; i++) {
             ppnext = ptemp;
+            VALGRIND_MEMPOOL_ALLOC(pfl, ptemp, sizeof(void*));
             *ppnext = pfl->head;
             pfl->head = ptemp;
-            ptemp = ((char *)ptemp) + pfl->size;
+            ptemp = ((char *)ptemp) + pfl->size+REDZONE;
         }
         ptemp = pfl->head;
         pfl->nBlocksAvailable += pfl->nmalloc;
@@ -106,6 +125,8 @@ epicsShareFunc void * epicsShareAPI freeListMalloc(void *pvt)
     pfl->head = *ppnext;
     pfl->nBlocksAvailable--;
     epicsMutexUnlock(pfl->lock);
+    VALGRIND_MEMPOOL_FREE(pfl, ptemp);
+    VALGRIND_MEMPOOL_ALLOC(pfl, ptemp, pfl->size);
     return(ptemp);
 #   endif
 }
@@ -119,6 +140,9 @@ epicsShareFunc void epicsShareAPI freeListFree(void *pvt,void*pmem)
 #   else
     void	**ppnext;
 
+    VALGRIND_MEMPOOL_FREE(pvt, pmem);
+    VALGRIND_MEMPOOL_ALLOC(pvt, pmem, sizeof(void*));
+
     epicsMutexMustLock(pfl->lock);
     ppnext = pmem;
     *ppnext = pfl->head;
@@ -134,6 +158,8 @@ epicsShareFunc void epicsShareAPI freeListCleanup(void *pvt)
     allocMem	*phead;
     allocMem	*pnext;
 
+    VALGRIND_DESTROY_MEMPOOL(pvt);
+
     phead = pfl->mallochead;
     while(phead) {
         pnext = phead->next;

src/libCom/misc/epicsExit.c

@@ -35,6 +35,8 @@
 #include "cantProceed.h"
 #include "epicsExit.h"
 
+void epicsMutexCleanup(void);
+
 typedef struct exitNode {
     ELLNODE         node;
     epicsExitFunc   func;
@@ -113,6 +115,8 @@ epicsShareFunc void epicsExitCallAtExits(void)
         epicsExitCallAtExitsPvt ( pep );
         destroyExitPvt ( pep );
     }
+    /* Handle specially to avoid circular reference */
+    epicsMutexCleanup();
 }
 
 epicsShareFunc void epicsExitCallAtThreadExits(void)

src/libCom/osi/epicsMutex.cpp

@@ -28,6 +28,7 @@
 #define epicsExportSharedSymbols
 #include "epicsStdio.h"
 #include "epicsThread.h"
+#include "valgrind/valgrind.h"
 #include "ellLib.h"
 #include "errlog.h"
 #include "epicsMutex.h"
@@ -85,6 +86,7 @@ epicsMutexId epicsShareAPI epicsMutexOsiCreate(
         firstTime=0;
         ellInit(&mutexList);
         ellInit(&freeList);
+        VALGRIND_CREATE_MEMPOOL(&freeList, 0, 0);
         epicsMutexGlobalLock = epicsMutexOsdCreate();
     }
     id = epicsMutexOsdCreate();
@@ -98,9 +100,11 @@ epicsMutexId epicsShareAPI epicsMutexOsiCreate(
         reinterpret_cast < epicsMutexParm * > ( ellFirst(&freeList) );
     if(pmutexNode) {
         ellDelete(&freeList,&pmutexNode->node);
+        VALGRIND_MEMPOOL_FREE(&freeList, pmutexNode);
     } else {
         pmutexNode = static_cast < epicsMutexParm * > ( calloc(1,sizeof(epicsMutexParm)) );
     }
+    VALGRIND_MEMPOOL_ALLOC(&freeList, pmutexNode, sizeof(epicsMutexParm));
     pmutexNode->id = id;
 #   ifdef LOG_LAST_OWNER
         pmutexNode->lastOwner = 0;
@@ -127,6 +131,8 @@ void epicsShareAPI epicsMutexDestroy(epicsMutexId pmutexNode)
     assert ( lockStat == epicsMutexLockOK );
     ellDelete(&mutexList,&pmutexNode->node);
     epicsMutexOsdDestroy(pmutexNode->id);
+    VALGRIND_MEMPOOL_FREE(&freeList, pmutexNode);
+    VALGRIND_MEMPOOL_ALLOC(&freeList, &pmutexNode->node, sizeof(pmutexNode->node));
     ellAdd(&freeList,&pmutexNode->node);
     epicsMutexOsdUnlock(epicsMutexGlobalLock);
 }
@@ -162,6 +168,26 @@ epicsMutexLockStatus epicsShareAPI epicsMutexTryLock(
     return status;
 }
 
+/* Empty the freeList.
+ * Called from epicsExit.c, but not via epicsAtExit()
+ * to avoid the possibility of a circular reference.
+ */
+extern "C"
+void epicsMutexCleanup(void)
+{
+    ELLNODE *cur;
+    epicsMutexLockStatus lockStat =
+        epicsMutexOsdLock(epicsMutexGlobalLock);
+    assert ( lockStat == epicsMutexLockOK );
+
+    while((cur=ellGet(&freeList))!=NULL) {
+        VALGRIND_MEMPOOL_FREE(&freeList, cur);
+        free(cur);
+    }
+
+    epicsMutexOsdUnlock(epicsMutexGlobalLock);
+}
+
 void epicsShareAPI epicsMutexShow(
     epicsMutexId pmutexNode, unsigned  int level)
 {

src/libCom/taskwd/taskwd.c

@@ -25,6 +25,7 @@
 #include "epicsStdioRedirect.h"
 #include "epicsThread.h"
 #include "epicsMutex.h"
+#include "valgrind/valgrind.h"
 #include "errlog.h"
 #include "ellLib.h"
 #include "errMdef.h"
@@ -130,9 +131,15 @@ static void twdTask(void *arg)
 
 static void twdShutdown(void *arg)
 {
+    ELLNODE *cur;
     twdCtl = twdctlExit;
     epicsEventSignal(loopEvent);
     epicsEventWait(exitEvent);
+    while ((cur = ellGet(&fList)) != NULL) {
+        VALGRIND_MEMPOOL_FREE(&fList, cur);
+        free(cur);
+    }
+    VALGRIND_DESTROY_MEMPOOL(&fList);
 }
 
 static void twdInitOnce(void *arg)
@@ -142,6 +149,8 @@ static void twdInitOnce(void *arg)
     tLock = epicsMutexMustCreate();
     mLock = epicsMutexMustCreate();
     fLock = epicsMutexMustCreate();
+    ellInit(&fList);
+    VALGRIND_CREATE_MEMPOOL(&fList, 0, 0);
 
     twdCtl = twdctlRun;
     loopEvent = epicsEventMustCreate(epicsEventEmpty);
@@ -388,14 +397,16 @@ static union twdNode *newNode(void)
     union twdNode *pn;
 
     epicsMutexMustLock(fLock);
-    pn = (union twdNode *)ellFirst(&fList);
+    pn = (union twdNode *)ellGet(&fList);
     if (pn) {
-        ellDelete(&fList, (void *)pn);
-        epicsMutexUnlock(fLock);
-        return pn;
+        VALGRIND_MEMPOOL_FREE(&fList, pn);
     }
     epicsMutexUnlock(fLock);
-    return calloc(1, sizeof(union twdNode));
+    if (!pn)
+        pn = calloc(1, sizeof(union twdNode));
+    if (pn)
+        VALGRIND_MEMPOOL_ALLOC(&fList, pn, sizeof(*pn));
+    return pn;
 }
 
 static union twdNode *allocNode(void)
@@ -411,6 +422,8 @@ static union twdNode *allocNode(void)
 
 static void freeNode(union twdNode *pn)
 {
+    VALGRIND_MEMPOOL_FREE(&fList, pn);
+    VALGRIND_MEMPOOL_ALLOC(&fList, pn, sizeof(ELLNODE));
     epicsMutexMustLock(fLock);
     ellAdd(&fList, (void *)pn);
     epicsMutexUnlock(fLock);

src/std/filters/arr.c

@@ -18,6 +18,7 @@
 #include <db_field_log.h>
 #include <dbLock.h>
 #include <recSup.h>
+#include <epicsExit.h>
 #include <special.h>
 #include <chfPlugin.h>
 #include <epicsExport.h>
@@ -200,6 +201,11 @@ static chfPluginIf pif = {
     NULL /* channel_close */
 };
 
+static void arrShutdown(void* ignore)
+{
+    freeListCleanup(myStructFreeList);
+}
+
 static void arrInitialize(void)
 {
     static int firstTime = 1;
@@ -211,6 +217,7 @@ static void arrInitialize(void)
         freeListInitPvt(&myStructFreeList, sizeof(myStruct), 64);
 
     chfPluginRegister("arr", &pif, opts);
+    epicsAtExit(arrShutdown, NULL);
 }
 
 epicsExportRegistrar(arrInitialize);

src/std/filters/dbnd.c

@@ -18,6 +18,7 @@
 #include <dbConvertFast.h>
 #include <chfPlugin.h>
 #include <recGbl.h>
+#include <epicsExit.h>
 #include <db_field_log.h>
 #include <epicsExport.h>
 
@@ -121,6 +122,11 @@ static chfPluginIf pif = {
     NULL /* channel_close */
 };
 
+static void dbndShutdown(void* ignore)
+{
+    freeListCleanup(myStructFreeList);
+}
+
 static void dbndInitialize(void)
 {
     static int firstTime = 1;
@@ -132,6 +138,7 @@ static void dbndInitialize(void)
         freeListInitPvt(&myStructFreeList, sizeof(myStruct), 64);
 
     chfPluginRegister("dbnd", &pif, opts);
+    epicsAtExit(dbndShutdown, NULL);
 }
 
 epicsExportRegistrar(dbndInitialize);

src/std/filters/sync.c

@@ -16,6 +16,7 @@
 #include "db_field_log.h"
 #include "chfPlugin.h"
 #include "dbState.h"
+#include "epicsExit.h"
 #include "epicsAssert.h"
 #include "epicsExport.h"
 
@@ -174,6 +175,11 @@ static chfPluginIf pif = {
     NULL /* channel_close */
 };
 
+static void syncShutdown(void* ignore)
+{
+    freeListCleanup(myStructFreeList);
+}
+
 static void syncInitialize(void)
 {
     static int firstTime = 1;
@@ -185,6 +191,7 @@ static void syncInitialize(void)
         freeListInitPvt(&myStructFreeList, sizeof(myStruct), 64);
 
     chfPluginRegister("sync", &pif, opts);
+    epicsAtExit(syncShutdown, NULL);
 }
 
 epicsExportRegistrar(syncInitialize);