lili/OCCT
1
0
Fork 0
mirror of https://github.com/Open-Cascade-SAS/OCCT.git synced 2026-08-09 07:20:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

Coding - Fix critical CodeQL static analysis warnings (#1074)

Browse Source 
Interface_ParamSet:
- Eliminate use-after-free in Append() by deleting old buffer through
  a temp variable after reassigning the member pointer (CodeQL #5132/#2684)

delabella.cpp:
- Fix upcast array pointer arithmetic by parenthesizing cast to ensure
  pointer arithmetic uses derived class (Vert) size rather than base
  class (DelaBella_Vertex) size (CodeQL #5131)

NCollection_SparseArrayBase:
- Rework to replace virtual dispatch (createItem/destroyItem/copyItem)
  with function pointers passed as arguments to protected methods
- Store only DestroyItemFunc in base class to enable safe cleanup
  in destructor without virtual dispatch
- Pass CreateItemFunc and CopyItemFunc as arguments with zero
  per-instance storage overhead
- Move Clear() and UnsetValue() from base public API to protected
  clearItems()/unsetValue() with function pointer parameters;
  template class provides public wrappers
- Remove vtable entirely (no virtual methods remain)
- This eliminates the pure virtual call during base class destruction
  (CodeQL #5012)

AdvApp2Var_MathBase:
- Rewrite comparison to avoid potential signed integer overflow:
  *ncfnew + 1 > ncut becomes *ncfnew >= ncut (CodeQL #2692)
This commit is contained in:
Pasukhin Dmitry
2026-02-13 08:48:17 +00:00
committed by GitHub
parent 833c86f176
commit 56e162c480
6 changed files with 118 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/DataExchange/TKXSBase/Interface/Interface_ParamSet.cxx
+4 -3
View File
@@ -86,9 +86,10 @@ int Interface_ParamSet::Append(const char* const val,
OFP.SetEntityNumber(onum);
}
// Confirm the new reservation
delete[] theval;
theval = newval;
thelnres = newres;
char* anOldVal = theval;
theval = newval;
thelnres = newres;
delete[] anOldVal;
}
// Register this parameter
for (i = 0; i < lnval; i++)
src/FoundationClasses/TKernel/NCollection/NCollection_SparseArray.hxx
+17 -20
View File
@@ -49,7 +49,7 @@ class NCollection_SparseArray : public NCollection_SparseArrayBase
public:
//! Constructor; accepts size of blocks
explicit NCollection_SparseArray(size_t theIncrement) noexcept
: NCollection_SparseArrayBase(sizeof(TheItemType), theIncrement)
: NCollection_SparseArrayBase(sizeof(TheItemType), theIncrement, destroyItemImpl)
{
}
@@ -58,7 +58,7 @@ public:
{
if (this == &theOther)
return *this;
this->assign(theOther);
this->assign(theOther, createItemImpl, destroyItemImpl, copyItemImpl);
return *this;
}
@@ -67,8 +67,8 @@ public:
//! in a fast way (without creation of duplicated data)
void Exchange(NCollection_SparseArray& theOther) noexcept { this->exchange(theOther); }
//! Destructor
~NCollection_SparseArray() override { Clear(); }
//! Clears all the data
void Clear() { clearItems(destroyItemImpl); }
public:
//!@name Array-like interface (in addition to inherited methods)
@@ -95,9 +95,13 @@ public:
//! Set a value at specified index method
TheItemType& SetValue(const size_t theIndex, const TheItemType& theValue)
{
return *(TheItemType*)this->setValue(theIndex, (void*)&theValue);
return *(TheItemType*)this->setValue(theIndex, (void*)&theValue, createItemImpl, copyItemImpl);
}
//! Deletes the item from the array;
//! returns True if that item was defined
bool UnsetValue(const size_t theIndex) { return this->unsetValue(theIndex, destroyItemImpl); }
//!@}
public:
@@ -126,7 +130,7 @@ public:
bool IsBound(const size_t theIndex) const { return this->HasValue(theIndex); }
//! Remove the item from array
bool UnBind(const size_t theIndex) { return this->UnsetValue(theIndex); }
bool UnBind(const size_t theIndex) { return UnsetValue(theIndex); }
//!@}
@@ -190,29 +194,22 @@ public:
};
private:
// Implementation of virtual methods providing type-specific behaviour
// Static functions providing type-specific item operations
//! Create new item at the specified address with default constructor
// virtual void createItem (void* theAddress)
// {
// new (theAddress) TheItemType;
// }
//! Create new item at the specified address with copy constructor
//! from existing item
void createItem(void* theAddress, void* theOther) override
//! Copy-construct a new item from existing item
static void createItemImpl(void* theAddress, void* theOther)
{
new (theAddress) TheItemType(*(const TheItemType*)theOther);
}
//! Call destructor to the item at given address
void destroyItem(void* theAddress) override
//! Call destructor on the item at given address
static void destroyItemImpl(void* theAddress)
{
((TheItemType*)theAddress)->TheItemType::~TheItemType();
}
//! Call assignment operator to the item
void copyItem(void* theAddress, void* theOther) override
//! Call assignment operator on the item
static void copyItemImpl(void* theAddress, void* theOther)
{
(*(TheItemType*)theAddress) = *(const TheItemType*)theOther;
}
src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.cxx
+36 -40
View File
@@ -14,13 +14,12 @@
// commercial license or contractual agreement.
#include <NCollection_SparseArrayBase.hxx>
#include <Standard_ProgramError.hxx>
#include <Standard.hxx>
#include <algorithm>
#include <cstring>
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::allocData(const size_t iBlock)
{
@@ -43,9 +42,9 @@ void NCollection_SparseArrayBase::allocData(const size_t iBlock)
myNbBlocks = newNbBlocks;
}
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::freeBlock(const size_t iBlock)
void NCollection_SparseArrayBase::freeBlock(const size_t iBlock, DestroyItemFunc theDestroyItem)
{
void*& anAddr = myData[iBlock];
if (!anAddr)
@@ -57,7 +56,7 @@ void NCollection_SparseArrayBase::freeBlock(const size_t iBlock)
{
if (aBlock.IsSet(anInd))
{
destroyItem(getItem(aBlock, anInd));
theDestroyItem(getItem(aBlock, anInd));
mySize--;
}
}
@@ -65,16 +64,16 @@ void NCollection_SparseArrayBase::freeBlock(const size_t iBlock)
anAddr = nullptr;
}
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::Clear()
void NCollection_SparseArrayBase::clearItems(DestroyItemFunc theDestroyItem)
{
// free block data
for (size_t iBlock = 0; iBlock < myNbBlocks; iBlock++)
{
if (myData[iBlock])
{
freeBlock(iBlock);
freeBlock(iBlock, theDestroyItem);
}
}
@@ -83,23 +82,21 @@ void NCollection_SparseArrayBase::Clear()
myData = nullptr;
myNbBlocks = 0;
mySize = 0;
// consistency check
Standard_ProgramError_Raise_if(
mySize != 0,
"NCollection_SparseArrayBase: Implementation error: inconsistent items count")
}
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theOther)
void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theOther,
CreateItemFunc theCreateItem,
DestroyItemFunc theDestroyItem,
CopyItemFunc theCopyItem)
{
if (this == &theOther)
return;
// if block size is different, clear all data
if (myBlockSize != theOther.myBlockSize)
Clear();
clearItems(theDestroyItem);
myBlockSize = theOther.myBlockSize;
// iterate by blocks in theOther
@@ -110,7 +107,7 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO
{
// if other block is empty, just make sure to empty that block in "this"
if (iBlock < myNbBlocks && myData[iBlock])
freeBlock(iBlock);
freeBlock(iBlock, theDestroyItem);
continue;
}
@@ -133,7 +130,7 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO
aBlock.Set(anInd);
(*aBlock.Count)++;
mySize++;
createItem(anItem, getItem(anOtherBlock, anInd));
theCreateItem(anItem, getItem(anOtherBlock, anInd));
}
}
// else perform copying item-by-item
@@ -148,14 +145,14 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO
void* anOtherItem = getItem(anOtherBlock, anInd);
if (aBlock.IsSet(anInd)) // copy
{
copyItem(anItem, anOtherItem);
theCopyItem(anItem, anOtherItem);
}
else // create
{
aBlock.Set(anInd);
(*aBlock.Count)++;
mySize++;
createItem(anItem, getItem(anOtherBlock, anInd));
theCreateItem(anItem, getItem(anOtherBlock, anInd));
}
}
else if (aBlock.IsSet(anInd)) // delete
@@ -163,7 +160,7 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO
aBlock.Set(anInd);
(*aBlock.Count)--;
mySize--;
destroyItem(anItem);
theDestroyItem(anItem);
}
}
}
@@ -172,15 +169,10 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO
// clear any remaining blocks in this
for (; iBlock < myNbBlocks; iBlock++)
if (myData[iBlock])
freeBlock(iBlock);
// consistency check
Standard_ProgramError_Raise_if(
mySize != theOther.mySize,
"NCollection_SparseArrayBase: Implementation error: inconsistent items count")
freeBlock(iBlock, theDestroyItem);
}
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::exchange(NCollection_SparseArrayBase& theOther) noexcept
{
@@ -193,11 +185,15 @@ void NCollection_SparseArrayBase::exchange(NCollection_SparseArrayBase& theOther
std::swap(myNbBlocks, theOther.myNbBlocks);
std::swap(mySize, theOther.mySize);
std::swap(myData, theOther.myData);
std::swap(myDestroyItem, theOther.myDestroyItem);
}
//=================================================================================================
//==================================================================================================
void* NCollection_SparseArrayBase::setValue(const size_t theIndex, void* const theValue)
void* NCollection_SparseArrayBase::setValue(const size_t theIndex,
void* const theValue,
CreateItemFunc theCreateItem,
CopyItemFunc theCopyItem)
{
size_t iBlock = theIndex / myBlockSize;
@@ -225,18 +221,18 @@ void* NCollection_SparseArrayBase::setValue(const size_t theIndex, void* const t
{
(*aBlock.Count)++;
mySize++;
createItem(anItem, theValue);
theCreateItem(anItem, theValue);
}
else
{
// Item already exists, just copy the value
copyItem(anItem, theValue);
theCopyItem(anItem, theValue);
}
return anItem;
}
//=================================================================================================
//==================================================================================================
bool NCollection_SparseArrayBase::HasValue(const size_t theIndex) const
{
@@ -246,9 +242,9 @@ bool NCollection_SparseArrayBase::HasValue(const size_t theIndex) const
return getBlock(myData[iBlock]).IsSet(theIndex % myBlockSize) != 0;
}
//=================================================================================================
//==================================================================================================
bool NCollection_SparseArrayBase::UnsetValue(const size_t theIndex)
bool NCollection_SparseArrayBase::unsetValue(const size_t theIndex, DestroyItemFunc theDestroyItem)
{
// check that the item is defined
size_t iBlock = theIndex / myBlockSize;
@@ -261,18 +257,18 @@ bool NCollection_SparseArrayBase::UnsetValue(const size_t theIndex)
return false;
// destroy the item
destroyItem(getItem(aBlock, anInd));
theDestroyItem(getItem(aBlock, anInd));
(*aBlock.Count)--;
mySize--;
// free block if it becomes empty
if (!(*aBlock.Count))
freeBlock(iBlock);
freeBlock(iBlock, theDestroyItem);
return true;
}
//=================================================================================================
//==================================================================================================
NCollection_SparseArrayBase::Iterator::Iterator(const NCollection_SparseArrayBase* theArray)
: myArr((NCollection_SparseArrayBase*)theArray),
@@ -284,7 +280,7 @@ NCollection_SparseArrayBase::Iterator::Iterator(const NCollection_SparseArrayBas
init(theArray);
}
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::Iterator::Next()
{
@@ -318,7 +314,7 @@ void NCollection_SparseArrayBase::Iterator::Next()
}
}
//=================================================================================================
//==================================================================================================
void NCollection_SparseArrayBase::Iterator::init(const NCollection_SparseArrayBase* theArray)
{
src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx
+59 -39
View File
@@ -24,16 +24,30 @@
* Base class for NCollection_SparseArray;
* provides non-template implementation of general mechanics
* of block allocation, items creation / deletion etc.
*
* Type-specific item operations (construction, destruction, copy)
* are provided by the derived template class via function pointers
* passed as arguments to the protected methods.
*/
class NCollection_SparseArrayBase
{
public:
//!@name Type-independent public interface
//!@name Function pointer types for type-specific item operations
//!@{
//! Clears all the data
Standard_EXPORT void Clear();
//! Copy-construct a new item at theAddress from theOther
using CreateItemFunc = void (*)(void* theAddress, void* theOther);
//! Destroy the item at theAddress
using DestroyItemFunc = void (*)(void* theAddress);
//! Copy-assign the item at theAddress from theOther
using CopyItemFunc = void (*)(void* theAddress, void* theOther);
//!@}
public:
//!@name Type-independent public interface
//!@{
//! Returns number of currently contained items
size_t Size() const noexcept { return mySize; }
@@ -41,10 +55,6 @@ public:
//! Check whether the value at given index is set
Standard_EXPORT bool HasValue(const size_t theIndex) const;
//! Deletes the item from the array;
//! returns True if that item was defined
Standard_EXPORT bool UnsetValue(const size_t theIndex);
//!@}
private:
@@ -180,18 +190,23 @@ private:
protected:
// Object life
//! Constructor; initialized by size of item and of block (in items)
NCollection_SparseArrayBase(size_t theItemSize, size_t theBlockSize) noexcept
//! Constructor; initialized by size of item, block size, and item destructor function.
//! @param theDestroyItem is stored to enable proper item destruction in the base destructor
NCollection_SparseArrayBase(size_t theItemSize,
size_t theBlockSize,
DestroyItemFunc theDestroyItem) noexcept
: myItemSize(theItemSize),
myBlockSize(theBlockSize),
myNbBlocks(0),
mySize(0),
myData(nullptr)
myData(nullptr),
myDestroyItem(theDestroyItem)
{
}
//! Destructor
virtual ~NCollection_SparseArrayBase() { Clear(); }
//! Destructor; properly destroys all items and frees all memory.
//! Uses the stored DestroyItemFunc, so no virtual dispatch is needed.
~NCollection_SparseArrayBase() { clearItems(myDestroyItem); }
protected:
// Data access interface for descendants
@@ -219,48 +234,53 @@ protected:
+ myItemSize * (theIndex % myBlockSize);
}
//! Set a value to the specified item; returns address of the set item
Standard_EXPORT void* setValue(const size_t theIndex, void* const theValue);
//! Clears all items and frees all memory.
//! @param theDestroyItem function to call destructor on each item
Standard_EXPORT void clearItems(DestroyItemFunc theDestroyItem);
//! Deletes the item at theIndex from the array;
//! returns True if the item was defined.
//! @param theDestroyItem function to call destructor on the item
Standard_EXPORT bool unsetValue(const size_t theIndex, DestroyItemFunc theDestroyItem);
//! Set a value to the specified item; returns address of the set item.
//! @param theCreateItem function to copy-construct a new item
//! @param theCopyItem function to copy-assign an existing item
Standard_EXPORT void* setValue(const size_t theIndex,
void* const theValue,
CreateItemFunc theCreateItem,
CopyItemFunc theCopyItem);
//! Copy contents of theOther to this;
//! assumes that this and theOther have exactly the same type of arguments
Standard_EXPORT void assign(const NCollection_SparseArrayBase& theOther);
//! assumes that this and theOther have exactly the same type of arguments.
//! @param theCreateItem function to copy-construct a new item
//! @param theDestroyItem function to call destructor on an item
//! @param theCopyItem function to copy-assign an existing item
Standard_EXPORT void assign(const NCollection_SparseArrayBase& theOther,
CreateItemFunc theCreateItem,
DestroyItemFunc theDestroyItem,
CopyItemFunc theCopyItem);
//! Exchange contents of theOther and this;
//! assumes that this and theOther have exactly the same type of arguments
Standard_EXPORT void exchange(NCollection_SparseArrayBase& theOther) noexcept;
protected:
// Methods to be provided by descendant
//! Create new item at the specified address with default constructor
// virtual void createItem (void* theAddress) = 0;
//! Create new item at the specified address with copy constructor
//! from existing item
virtual void createItem(void* theAddress, void* theOther) = 0;
//! Call destructor to the item
virtual void destroyItem(void* theAddress) = 0;
//! Call assignment operator to the item
virtual void copyItem(void* theAddress, void* theOther) = 0;
private:
// Implementation of memory allocation/deallocation and access mechanics
//! Allocate space for at least iBlock+1 blocks
void allocData(const size_t iBlock);
//! Free specified block
void freeBlock(const size_t iBlock);
//! Free specified block, destroying all items via theDestroyItem
void freeBlock(const size_t iBlock, DestroyItemFunc theDestroyItem);
protected:
size_t myItemSize; //!< size of item
size_t myBlockSize; //!< block size (in items)
size_t myNbBlocks; //!< allocated size of blocks table
size_t mySize; //!< number of currently defined items
void** myData; //!< array of pointers to data blocks
size_t myItemSize; //!< size of item
size_t myBlockSize; //!< block size (in items)
size_t myNbBlocks; //!< allocated size of blocks table
size_t mySize; //!< number of currently defined items
void** myData; //!< array of pointers to data blocks
DestroyItemFunc myDestroyItem; //!< function to call destructor on items
};
#endif
src/ModelingAlgorithms/TKMesh/BRepMesh/delabella.cpp
+1 -1
View File
@@ -247,7 +247,7 @@ struct CDelaBella : IDelaBella
errlog_proc(errlog_file,
"[WRN] all input points are colinear, returning single segment!\n");
first_hull_vert = vert_alloc + 0;
vert_alloc[0].next = (DelaBella_Vertex*)vert_alloc + 1;
vert_alloc[0].next = (DelaBella_Vertex*)(vert_alloc + 1);
vert_alloc[1].next = nullptr;
}
else
src/ModelingData/TKGeomBase/AdvApp2Var/AdvApp2Var_MathBase.cxx
+1 -1
View File
@@ -535,7 +535,7 @@ int mmaper0_(integer* ncofmx,
/* ------ Minimum that can be reached : Stop at 1 or NCFNEW ------ */
ncut = 1;
if (*ncfnew + 1 > ncut)
if (*ncfnew >= ncut)
{
ncut = *ncfnew + 1;
}