From fafbc9b66f81069a048265f4e9a0a76ca7195c70 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Wed, 21 Apr 2021 19:10:19 -0700 Subject: [PATCH] fix: consistently treat entry indices as uint64_t in tclhandle This code intermingled uint64_t and int values for representing indices into an array. Making this consistent removes a huge number of -Wsign-conversion and -Wsign-compare compiler warnings, as well as more serious problem where printing code assumed "%lu" was the format code to print an index. This is not true when uint64_t is not the same type as unsigned long and would have most probably resulted in stack corruption. --- CHANGELOG.md | 1 + tclpkg/tclhandle/tclhandle.c | 37 +++++++++++++++++--------------- tclpkg/tclhandle/tclhandle.h | 20 +++++++++-------- tclpkg/tclpathplan/tclpathplan.c | 7 +++--- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 338d0261b..3d6f698f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Uninitialized variable read in delaunay_tri +- potentially mismatched format string in tclpkg ## [2.47.1] - 2021-04-17 diff --git a/tclpkg/tclhandle/tclhandle.c b/tclpkg/tclhandle/tclhandle.c index ff27bcc23..4210b19db 100644 --- a/tclpkg/tclhandle/tclhandle.c +++ b/tclpkg/tclhandle/tclhandle.c @@ -35,6 +35,7 @@ */ #include +#include #include #include #include "tclhandle.h" @@ -44,7 +45,7 @@ * Variable set to contain the alignment factor (in bytes) for this machine. * It is set on the first table initialization. */ -static int tclhandleEntryAlignment = 0; +static uint64_t tclhandleEntryAlignment = 0; /*============================================================================= * tclhandleLinkInNewEntries -- @@ -56,10 +57,10 @@ static int tclhandleEntryAlignment = 0; * o numEntries (I) - The number of new entries. *----------------------------------------------------------------------------- */ -static void tclhandleLinkInNewEntries(tblHeader_pt tblHdrPtr, int newIdx, - int numEntries) +static void tclhandleLinkInNewEntries(tblHeader_pt tblHdrPtr, uint64_t newIdx, + uint64_t numEntries) { - int entIdx, lastIdx; + uint64_t entIdx, lastIdx; entryHeader_pt entryPtr; lastIdx = newIdx + numEntries - 1; @@ -84,13 +85,13 @@ static void tclhandleLinkInNewEntries(tblHeader_pt tblHdrPtr, int newIdx, * entries specified on table creation. MUST be smaller than this size. *----------------------------------------------------------------------------- */ -static void tclhandleExpandTable(tblHeader_pt tblHdrPtr, int neededIdx) +static void tclhandleExpandTable(tblHeader_pt tblHdrPtr, uint64_t neededIdx) { ubyte_pt oldbodyPtr = tblHdrPtr->bodyPtr; - int numNewEntries; - int newSize; + uint64_t numNewEntries; + uint64_t newSize; - if (neededIdx < 0) + if (neededIdx == NULL_IDX || neededIdx == ALLOCATED_IDX) numNewEntries = tblHdrPtr->tableSize; else numNewEntries = (neededIdx - tblHdrPtr->tableSize) + 1; @@ -124,10 +125,10 @@ entryHeader_pt tclhandleAlloc(tblHeader_pt headerPtr, char *handle, { tblHeader_pt tblHdrPtr = (tblHeader_pt) headerPtr; entryHeader_pt entryPtr; - int entryIdx; + uint64_t entryIdx; if (tblHdrPtr->freeHeadIdx == NULL_IDX) - tclhandleExpandTable(tblHdrPtr, -1); + tclhandleExpandTable(tblHdrPtr, NULL_IDX); entryIdx = tblHdrPtr->freeHeadIdx; entryPtr = TBL_INDEX(tblHdrPtr, entryIdx); @@ -153,7 +154,8 @@ entryHeader_pt tclhandleAlloc(tblHeader_pt headerPtr, char *handle, * A pointer to the table header. *----------------------------------------------------------------------------- */ -tblHeader_pt tclhandleInit(char *prefix, int entrySize, int initEntries) +tblHeader_pt tclhandleInit(char *prefix, uint64_t entrySize, + uint64_t initEntries) { tblHeader_pt tblHdrPtr; @@ -180,9 +182,9 @@ tblHeader_pt tclhandleInit(char *prefix, int entrySize, int initEntries) tblHdrPtr->entrySize = ENTRY_HEADER_SIZE + ROUND_ENTRY_SIZE(entrySize); tblHdrPtr->freeHeadIdx = NULL_IDX; tblHdrPtr->tableSize = initEntries; - tblHdrPtr->handleFormat = malloc(strlen(prefix) + 4); + tblHdrPtr->handleFormat = malloc(strlen(prefix) + strlen("%" PRIu64) + 1); strcpy(tblHdrPtr->handleFormat, prefix); - strcat(tblHdrPtr->handleFormat, "%lu"); + strcat(tblHdrPtr->handleFormat, "%" PRIu64); tblHdrPtr->bodyPtr = malloc(initEntries * tblHdrPtr->entrySize); tclhandleLinkInNewEntries(tblHdrPtr, 0, initEntries); @@ -202,7 +204,7 @@ tblHeader_pt tclhandleInit(char *prefix, int entrySize, int initEntries) */ int tclhandleDestroy(tblHeader_pt tblHdrPtr) { - int entIdx, lastIdx; + uint64_t entIdx, lastIdx; entryHeader_pt entryPtr; lastIdx = tblHdrPtr->tableSize; @@ -230,9 +232,9 @@ int tclhandleDestroy(tblHeader_pt tblHdrPtr) *----------------------------------------------------------------------------- */ -int tclhandleReset(tblHeader_pt tblHdrPtr, int initEntries) +int tclhandleReset(tblHeader_pt tblHdrPtr, uint64_t initEntries) { - int entIdx, lastIdx; + uint64_t entIdx, lastIdx; entryHeader_pt entryPtr; lastIdx = tblHdrPtr->tableSize; @@ -365,7 +367,8 @@ void *tclhandleFreeIndex(tblHeader_pt headerPtr, uint64_t entryIdx) entryPtr = USER_AREA(entryPtr); freeentryPtr = HEADER_AREA(entryPtr); freeentryPtr->freeLink = tblHdrPtr->freeHeadIdx; - tblHdrPtr->freeHeadIdx = (((ubyte_pt) entryPtr) - tblHdrPtr->bodyPtr) / + tblHdrPtr->freeHeadIdx = + ((uint64_t)((uintptr_t)entryPtr - (uintptr_t)tblHdrPtr->bodyPtr)) / tblHdrPtr->entrySize; return entryPtr; diff --git a/tclpkg/tclhandle/tclhandle.h b/tclpkg/tclhandle/tclhandle.h index d425108e5..319d85a71 100644 --- a/tclpkg/tclhandle/tclhandle.h +++ b/tclpkg/tclhandle/tclhandle.h @@ -12,6 +12,8 @@ extern "C" { #endif +#include + #ifndef TCL_ERROR #define TCL_ERROR (1) #endif @@ -29,8 +31,8 @@ extern "C" { #define ROUND_ENTRY_SIZE(size) ((((size) + tclhandleEntryAlignment - 1) / \ tclhandleEntryAlignment) * tclhandleEntryAlignment) -#define NULL_IDX -1 -#define ALLOCATED_IDX -2 +#define NULL_IDX UINT64_MAX +#define ALLOCATED_IDX (UINT64_MAX - 1) typedef unsigned char ubyte_t; typedef ubyte_t *ubyte_pt; @@ -46,16 +48,16 @@ extern "C" { */ typedef struct { - int entrySize; /* Entry size in bytes, including overhead */ - int tableSize; /* Current number of entries in the table */ - int freeHeadIdx; /* Index of first free entry in the table */ + uint64_t entrySize; /* Entry size in bytes, including overhead */ + uint64_t tableSize; /* Current number of entries in the table */ + uint64_t freeHeadIdx; /* Index of first free entry in the table */ char *handleFormat; /* Malloc'ed copy of prefix string + "%lu" */ ubyte_pt bodyPtr; /* Pointer to table body */ } tblHeader_t; typedef tblHeader_t *tblHeader_pt; typedef struct { - int freeLink; + uint64_t freeLink; } entryHeader_t; typedef entryHeader_t *entryHeader_pt; @@ -90,9 +92,9 @@ extern "C" { void *tclhandleFreeIndex(tblHeader_pt headerPtr, uint64_t entryIdx); void *tclhandleFree(tblHeader_pt headerPtr, char *handle); - tblHeader_pt tclhandleInit(char *prefix, int entrySize, - int initEntries); - int tclhandleReset(tblHeader_pt tblHdrPtr, int initEntries); + tblHeader_pt tclhandleInit(char *prefix, uint64_t entrySize, + uint64_t initEntries); + int tclhandleReset(tblHeader_pt tblHdrPtr, uint64_t initEntries); int tclhandleDestroy(tblHeader_pt tblHdrPtr); void *tclhandleXlateIndex(tblHeader_pt headerPtr, uint64_t entryIdx); diff --git a/tclpkg/tclpathplan/tclpathplan.c b/tclpkg/tclpathplan/tclpathplan.c index 10029b202..3fb048d9e 100644 --- a/tclpkg/tclpathplan/tclpathplan.c +++ b/tclpkg/tclpathplan/tclpathplan.c @@ -25,6 +25,7 @@ #define _GNU_SOURCE 1 #include +#include #include #include #include @@ -195,9 +196,9 @@ void triangle_callback(void *vgparg, point pqr[]) /* TBL_ENTRY((tblHeader_pt)vgpaneTable, (ubyte_pt)vgp));*/ if (vgp->triangle_cmd) { - snprintf(vbuf, sizeof(vbuf), "vgpane%lu", - (uint64_t) (((ubyte_pt) vgp - (vgpaneTable->bodyPtr)) - / (vgpaneTable->entrySize))); + snprintf(vbuf, sizeof(vbuf), "vgpane%" PRIu64, + ((uint64_t)((uintptr_t)vgp - (uintptr_t)vgpaneTable->bodyPtr)) + / vgpaneTable->entrySize); expandPercentsEval(vgp->interp, vgp->triangle_cmd, vbuf, 3, pqr); } } -- 2.40.0