Skip to content

Commit

Permalink
Fix segmentation fault in Rocky Linux for ReadSparseFormatCKeysError
Browse files Browse the repository at this point in the history
Le probleme provient de la classe ValueBlock, hautement optimisee

KWValueBlock
- La classe utilise une fonctionnalite avancee du C++, le 'placement new operator',
  pour allouer prealablement la memoire donnee au constructeur, qui usuellement
  d'une part alloue la memoire, d'autre part l'initialise
- Ici, on aura une sequence de trois methodes dans les 'placement new' exploites dans
  les methodes NewValueBlock des sous classes:
  - appel de GenericAllocValueBlock pour creer un bloc memoire
  - appel du placement new du C++ avec ce bloc, qui declenche un appel standard du C++
    (implemente en ne faisant rien)
  - appel de GenericInitValueBlock pour initialiser le bloc memoire

Auparavant, on avait une seule methode GenericNewValueBlock qui allouait la memoire et
l'initialisait. Le constructeur etait appele ensuite, mais il semble qu'il modifiait (a tort)
le contenu de la memoire, entrainant un bug.

Impact etudie sur les autre utilisations du "placement new"
- uniquement la classe KWTuple: ok (utilisation plus basique)

Correction au passage d'une typo dans un message d'erreur
SNBPredictorSNBTrainingTask::MasterInitializeDataTableBinarySliceSet
- le premier caractere est mis en majuscule dans le message "Not enough memory to run the task"
- pour suivre le meme pattern que partout ailleurs dans le code
- pour etre traite systematiquement dans les pattern de resilience au messages utilisateurs
  dans le scripts de LearningTest

Note: la modification de IrisLight est a ignorer
- il n'y a en fait aucune difference (peut-etre les fins de lignes?)
  • Loading branch information
marcboulle committed Feb 1, 2024
1 parent f0a1b8c commit 5683f1d
Show file tree
Hide file tree
Showing 13 changed files with 4,817 additions and 4,776 deletions.
47 changes: 35 additions & 12 deletions src/Learning/KWData/KWValueBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,11 @@
/////////////////////////////////////////////////////////////////////
// Classe KWValueBlock

void* KWValueBlock::GenericNewValueBlock(int nSize)
void* KWValueBlock::GenericAllocValueBlock(int nSize)
{
KWValueBlock* newValueBlock;
int nMemorySize;
void* pValueBlockMemory;
int nSegmentNumber;
KWValueIndexPair* valueIndexPairs;
int i;

require(nSize >= 0);

Expand All @@ -26,9 +23,39 @@ void* KWValueBlock::GenericNewValueBlock(int nSize)
{
// Calcul de la taille a allouer
nMemorySize = sizeof(int) + nSize * sizeof(KWValueIndexPair);
}
// Cas multi-segment: il faut plusieurs segments pour stocker le bloc
// On passe par un tableau de segments
else
{
// Calcul du nombre de segment
nSegmentNumber = (nSize - 1) / nSegmentSize + 1;

// Calcul de la taille a allouer
nMemorySize = sizeof(int) + nSegmentNumber * sizeof(KWValueIndexPair*);
}

// Creation des donnees d'un bloc
pValueBlockMemory = NewMemoryBlock(nMemorySize);
// Creation du bloc memoire
pValueBlockMemory = NewMemoryBlock(nMemorySize);
return pValueBlockMemory;
}

void KWValueBlock::GenericInitValueBlock(void* pValueBlockMemory, int nSize)
{
KWValueBlock* newValueBlock;
int nMemorySize;
int nSegmentNumber;
KWValueIndexPair* valueIndexPairs;
int i;

require(pValueBlockMemory != NULL);
require(nSize >= 0);

// Cas mono-segment: le bloc entier peut tenir dans un segment memoire
if (nSize <= nSegmentSize)
{
// Calcul de la taille allouee
nMemorySize = sizeof(int) + nSize * sizeof(KWValueIndexPair);

// Initialisation avec des zero
memset(pValueBlockMemory, 0, nMemorySize * sizeof(char));
Expand All @@ -42,7 +69,7 @@ void* KWValueBlock::GenericNewValueBlock(int nSize)

// On verifie par assertion que le packing des classe utilise est correct
assert(sizeof(KWValueIndexPair) == sizeof(KWValue) + sizeof(int));
assert(&(newValueBlock->cStartBlock) - (char*)newValueBlock == sizeof(int));
assert(nSize == 0 or &(newValueBlock->cStartBlock) - (char*)newValueBlock == sizeof(int));
}
// Cas multi-segment: il faut plusieurs segments pour stocker le bloc
// On passe par un tableau de segments
Expand All @@ -51,12 +78,9 @@ void* KWValueBlock::GenericNewValueBlock(int nSize)
// Calcul du nombre de segment
nSegmentNumber = (nSize - 1) / nSegmentSize + 1;

// Calcul de la taille a allouer
// Calcul de la taille allouee
nMemorySize = sizeof(int) + nSegmentNumber * sizeof(KWValueIndexPair*);

// Creation des donnees du bloc, avec un tableau de pointeurs vers des blocs
pValueBlockMemory = NewMemoryBlock(nMemorySize);

// On caste la memoire allouee pour pouvoir utiliser les methodes de la classe
newValueBlock = (KWValueBlock*)pValueBlockMemory;

Expand Down Expand Up @@ -85,7 +109,6 @@ void* KWValueBlock::GenericNewValueBlock(int nSize)
memset(valueIndexPairs, 0, nSegmentSize * sizeof(KWValueIndexPair));
}
}
return pValueBlockMemory;
}

KWValueBlock::~KWValueBlock()
Expand Down
37 changes: 27 additions & 10 deletions src/Learning/KWData/KWValueBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,18 @@ class KWValueBlock : public SystemObject
protected:
// Creation et initialisation de la memoire necessaire au stockage d'un bloc
// Permet d'implementer les methodes de creation de bloc dans les sous-classes
static void* GenericNewValueBlock(int nSize);
//
// La classe utilise une fonctionnalite avancee du C++, le "placement new operator",
// pour allouer prealablement la memoire donnee au constructeur, qui usuellement
// d'une part alloue la memoire, d'autre part l'initialise
// Ici, on aura une sequence de trois methodes dans les "placement new" exploites dans
// les methodes NewValueBlock des sous-classes:
// - appel de GenericAllocValueBlock pour creer un bloc memoire
// - appel du placement new du C++ avec ce bloc, qui declenche un appel standard du C++
// (en utilisant un constructeur ne faisant rien)
// - appel de GenericInitValueBlock pour initialiser le bloc memoire a la place du constructeur
static void* GenericAllocValueBlock(int nSize);
static void GenericInitValueBlock(void* pValueBlockMemory, int nSize);

// Constructeur prive
KWValueBlock();
Expand Down Expand Up @@ -603,10 +614,12 @@ inline KWContinuousValueBlock* KWContinuousValueBlock::NewValueBlock(int nSize)
KWContinuousValueBlock* newValueBlock;
void* pValueBlockMemory;

// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee (attention, C++
// avance)
pValueBlockMemory = GenericNewValueBlock(nSize);
// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee
// (attention, C++ avance)
pValueBlockMemory = GenericAllocValueBlock(nSize);
newValueBlock = new (pValueBlockMemory) KWContinuousValueBlock;
GenericInitValueBlock(pValueBlockMemory, nSize);
assert(newValueBlock->nValueNumber == nSize);
return newValueBlock;
}

Expand Down Expand Up @@ -660,10 +673,12 @@ inline KWSymbolValueBlock* KWSymbolValueBlock::NewValueBlock(int nSize)
KWSymbolValueBlock* newValueBlock;
void* pValueBlockMemory;

// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee (attention, C++
// avance)
pValueBlockMemory = GenericNewValueBlock(nSize);
// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee
// (attention, C++ avance)
pValueBlockMemory = GenericAllocValueBlock(nSize);
newValueBlock = new (pValueBlockMemory) KWSymbolValueBlock;
GenericInitValueBlock(pValueBlockMemory, nSize);
assert(newValueBlock->nValueNumber == nSize);
return newValueBlock;
}

Expand Down Expand Up @@ -710,10 +725,12 @@ inline KWObjectArrayValueBlock* KWObjectArrayValueBlock::NewValueBlock(int nSize
KWObjectArrayValueBlock* newValueBlock;
void* pValueBlockMemory;

// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee (attention, C++
// avance)
pValueBlockMemory = GenericNewValueBlock(nSize);
// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee
// (attention, C++ avance)
pValueBlockMemory = GenericAllocValueBlock(nSize);
newValueBlock = new (pValueBlockMemory) KWObjectArrayValueBlock;
GenericInitValueBlock(pValueBlockMemory, nSize);
assert(newValueBlock->nValueNumber == nSize);
return newValueBlock;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Learning/KWDataPreparation/KWTupleTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,8 @@ KWTuple* KWTupleTable::NewTuple() const
// en plus des donnees du symbol (le caractere fin de chaine '\0' est deja prevu)
pTupleMemory = NewMemoryBlock(nMemorySize);

// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee (attention, C++
// avance)
// On utilise le "placement new" pour appeler un constructeur avec de la memoire preallouee
// (attention, C++ avance)
newTuple = new (pTupleMemory) KWTuple;
assert((void*)newTuple == pTupleMemory);

Expand Down
1 change: 1 addition & 0 deletions src/Learning/MODL/MODL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ int main(int argc, char** argv)

// Choix du repertoire de lancement pour le debugage sous Windows (a commenter apres fin du debug)
// SetWindowsDebugDir("Standard", "IrisLight");
SetWindowsDebugDir("SparseData", "ReadSparseFormatCKeysError");

// Parametrage des logs memoires depuis les variables d'environnement, pris en compte dans KWLearningProject
// KhiopsMemStatsLogFileName, KhiopsMemStatsLogFrequency, KhiopsMemStatsLogToCollect
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ boolean SNBPredictorSNBTrainingTask::MasterInitializeDataTableBinarySliceSet()
}
// Message d'erreur si pas assez de memoire, meme avec le nombre maximal de slices
else
AddError("not enough memory to run the task" +
AddError("Not enough memory to run the task" +
RMResourceManager::BuildMissingMemoryMessage(lSlaveNecessaryMemory - lGrantedSlaveMemory));

// Trace de deboggage
Expand Down
22 changes: 11 additions & 11 deletions test/LearningTest/TestKhiops/Standard/IrisLight/IrisSelection.kdic
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#Khiops 7.7.2i
Dictionary Iris
{
Numerical SepalLength ;
Numerical SepalWidth ;
Numerical PetalLength ;
Numerical PetalWidth ;
Categorical Class ;
Unused Numerical NoSetosa = NEQc(Class, "Iris-setosa") ;
};
#Khiops 7.7.2i

Dictionary Iris
{
Numerical SepalLength ;
Numerical SepalWidth ;
Numerical PetalLength ;
Numerical PetalWidth ;
Categorical Class ;
Unused Numerical NoSetosa = NEQc(Class, "Iris-setosa") ;
};
Loading

0 comments on commit 5683f1d

Please sign in to comment.