sanitizer::address unlogik?

EL europ

Folgender Code:

for(int x=0; x<picWidth; x++){
	for(int y=0; y<picHeight; y++){
		if(x<i_xres && y<i_yres){
			myApple.pic[x][y] = ci->apple->colormatrix[ x*divisor/xfak ][ y*divisor/yfak ];
		}
		else if(x<i_xres){ myApple.pic[x][y] = 0; }
//		else if(x<picWidth){ myApple.pic[x][y] = 0xFF0000; }
//		else if(x<picWidth){ myApple.pic[x][y] = 0x00FF0000; }
	}
}

wird ausgeführt. Wenn ich jedoch eine der beiden auskommentierten Zeilen "aktiviere" bricht der sanitizer::adress mit dem Hinweis, das die Adresse eines Lesezugriffs auf eine "zero-page" verweist, ab. (myApple.pic[x][y] ist vom Typ int)

Ich kann die Meldung nicht copy n pasten weil die umleitung vom virtuellen Terminal nicht funktioniert, und bin mir nicht sicher ob ich die sanitizer Flags sinngemäß nutze. Kann mir bitte jemand auf die Sprünge helfen.

DocShoe

@EL-europ sagte in sanitizer::address unlogik?:

...
Kann mir bitte jemand auf die Sprünge helfen.

Ja, gerne. Lass die Rumfrickelei mit deiner Speicherverwaltung sein und modellier' deine Daten vernünftig. Wurde dir im anderen Thread auch schon mehrmals empfohlen. Wenn du merkst, dass du ein totes Pferd reitest, dann steig ab.

EL europ

@DocShoe
Kanst du deine Ausführung bitte etwas detaillierter darlegen?

DocShoe

@EL-europ
Noch ausführlicher als hier hier? Nein, kann ich nicht, ich kann höchstens wiederholen, was dir da gesagt wurde. Aber da verweise ich lieber auf's Original.

EL europ

@DocShoe
Nur es fehlt hier der Bezug, an was ich arbeite kann dir doch egal sein

Schlangenmensch

@EL-europ Wenn du so Code Auszüge mit solchen Fragen kombinierst: Stell ein minimales compilierbares Beispiel zur Verfügung, mit dem man das Problem rekonstruieren kann. Das ist unter umständen arbeit, aber ganz häufig findet man so selbst seine Probleme und man lernt eine Menge.

Mutmaßlich greifst du irgendwo im Speicher daneben. Aber, woher sollen wir wissen, was pic ist. Nichtmal wenn ich in die Apple Klasse in deinem Githubrepository gucke, finde ich das.

Aber, selbst wenn es da stünde, mit der public Sichtbarkeit von den meisten Membern und myApple als globale Variable, ist es kaum möglich nachzuvollziehen, wo da was wie gesetzt wird.

EL europ

@Schlangenmensch
Hey Ich hab die Speicherlöcher aus Apple->calc() rausbekommen. Jetz hab ich noch kleinere in Apple->sort() vermutlich durch **iterMembers. Jetzt funzt auch der von mir hier gepostete Methodenaufruf mit allen Anweisungen. Ich hab die Klasse Apple leicht modifiziert so das die mandel.cpp aufgeräumter ist. Vielleicht kannst du dir die Apple->sort() in Anbetracht kleinerer Speicherlöcher durch **iterMembers mal Anschauen. Ich bin da mit meinem Wissen und Recherchen am Ende.

Schlangenmensch

@EL-europ Ganz ehrlich, beherzige die Tipps die du zu genüge bekommen hast.
Das ist nicht nur kein C++, das ist auch schlechter C Code.

iterMembers = (int**)malloc(1 * sizeof(int *));

f(! inarray){
   if((countsOfIter % 199) == 0){
	if( !(iterMembers = (int**)realloc(iterMembers, (countsOfIter +200) * sizeof(int *)))){return;}
		for(int k=0; k<200; k++){
			if( !(iterMembers[countsOfIter+k] = (int*)malloc( (2) * sizeof(int)))){return;}

Wer soll denn da verstehen, wie groß dein iterMembers zu welcher Zeit ist?

Bist du sicher, das countsOfIter häufig genug inkrementiert wird, um hinterher alles frei zu geben.

Noch mal: Ganz ehrlich, schmeiß das weg.
Entweder, mach das in C++ neu.Wenn du dir ins Bein schießen willst und das mit manueller Speicherverwaltung machen willst, und Sachen wie Vektor nachbauen willst, ok, aber google mal RAII.

Wenn du das in C machen willst, kannst du die Speicherverwaltung auch kapseln. Z.b. so ungfähr

typedef struct 
{
   int* values;
   int length;
} MyArray

MyArray* getArray(unsigned n) {

  MyArray* anArray = malloc(sizeof(MyArray));
  if (anArray == NULL) 
    return NULL;
          
  anArray->values = malloc(sizeof(int) * n);
  if (anArray->values == NULL ) {  
    free(anArray );             
    return NULL;                       
  }

  anArray->length= n;
  return anArray ; 
}

void freeArray(MyArray* anArray) {
  if (anArray == NULL) 
    return;               
  
  free(anArray->values); 
  free(anArray );
}

Ich habe bestimmt seit 10 Jahren kein plain C mehr programmiert und ich habe das nicht probiert, daher keinerlei Garantie. Aber so wie du deine Speicherverwaltung machst, ist es kein Wunder, dass dir das um die Ohren fliegt und es ist nur sehr schwer möglich zu sagen, wo genau der Fehler ist.

Das eigentliche Problem ist, dass es mit dem Programmierstil kaum möglich ist, sowas fehlerfrei zu implementieren, egal ob in C oder in C++.

EL europ

@Schlangenmensch Ja, die iterMembers allociere ich in 200er Schritten wegen der Laufzeit. Dort Versuche ich, wenn ich den Fehler nicht finde, eine Version mit std::vector und std::sort(). Ist es möglich mit std::sort() ein zweidimensionales Array naxh x oder y zu sortieren?

EL europ

@Schlangenmensch Den Code von dir oben wer ich mal, aber leider erst nachmittags, versuchen zu verstehen und zu übertragen

Schlangenmensch

@EL-europ std::sort kannst du mitgeben, wie du sortieren möchtest. Ich würde dir die Doku dazu empfehlen: https://en.cppreference.com/w/cpp/algorithm/sort oder, vlt je nach compiler noch etwas einfacher für dich: https://en.cppreference.com/w/cpp/algorithm/ranges/sort

Im Zweifel können wir dir da auch unter die Arme greifen, wenn du soweit bist.

Wenn du wirklich C++ machen willst, dann brauchst du meinen Code nicht, weil das ein std::vector alles schon mit bringt.

EL europ

@Schlangenmensch
Nun ja, es geht mir nicht um c++ sondern um Laufzeit. Aber die hängt an den **thr_... und **iterMembers Funktionen. Vielleicht ist es gar sinvoll die **iterMebers auch über Threads laufen zu lassen.
Das Prog soll mehrere tausend Apfelmänchen in Folge berechnen und als "rohe" Daten auf Platte speichern. Also ist das "Speicherloch" in Apple->sort() untragbar. Es Wäre fatal ne halbe Stunde zu warten um dann einen Abbruch wegen eines Speicherfehlers zu bekommen.

Th69

Wie ich schon schrieb:
@Th69 sagte in Klassenfrage : ):

Du brauchst auch nicht jedesmal wieder neuen Speicher anzufordern, da sich ja xres und yres währenddessen nicht ändert, d.h. bei Umstellung auf vector<...> reicht es diesen einmalig in der passenden Größe zu erzeugen (bzw. direkt in Init(...)).

Und bzgl. Performance kann man deinen Programmcode um einiges noch verbessern, da du viel zu viele unnötige Speicherallokationen und Berechnungen durchführst (und bei einem besseren Design, d.h. Codestruktur, könnte man einiges viel eleganter lösen).

Wenn du wirklich ein performantes Programm erstellen willst, dann laß diese unnötigen Speicherallokationen in jeder Funktion sein, sondern erstelle sie nur bei Bedarf (nur wenn sich die Größe der Bilder ändert o.ä.)! Diese Apfelmännchen werden doch sicherlich alle in derselben Größe erstellt, oder? Und selbst, wenn nicht, brauchst du nicht in jeder Funktion Speicher allozieren und wieder freigeben.

Du solltest wirklich mal eine kurze Programmierpause machen und dir mal ein Konzept (Design) für dein Programm erstellen (z.B. eine Liste der Einzelfunktionalitäten sowie ein Ablaufdiagramm).
Durch die Teilumstellung auf C++ ist dein Code noch undurchschaubarer und quasi unwartbar geworden (u.a. da du, trotz Benutzung von Klassen, OOP nicht sinnvoll anwendest).

Auch hast du meinen Vorschlag (thr_matrix = matrix) genau falschherum umgesetzt!
Die Membervariable matrix muß den Speicher allozieren und dieser wird dann nur an die Threadfunktion weitergereicht, so daß dann auch das Apple-Objekt für die Freigabe zuständig ist (so daß es dann auch bei mehreren Apple-Objekten einwandfrei funktionieren würde - dies ist der Grundgedanke beim Klassendesign).
Aber bei Umstellung auf std::thread bräuchtest du auch keine globalen Variablen mehr und könntest direkt auf matrix zugreifen.

Wenn du aber so, wie die letzten Tage, weitermachst, wirst du dein Programm nie sauber (und performant) umsetzen können.

Was ich dir anbieten kann: wenn du ein Mikro hast, dann könnten wir mal über Discord dies besprechen.

Schlangenmensch

@EL-europ sagte in sanitizer::address unlogik?:

Nun ja, es geht mir nicht um c++ sondern um Laufzeit

Mit ordentlichem C++ wirst du eine sehr gute Laufzeit hinbekommen. Aber, du musst dich für eine Programmiersprache entscheiden: C oder C++.

Für performante Software, unabhängig von der Programmiersprache:

Schritt 1) Eine ordentliche Software schreiben (saubere Algorithmen, Teile mit Unittests getestet, kommentiert)
Schritt 2) Mit Optimierungsflag gesetzt kompilieren
Schritt 3) Performance messen

Wenn das dann nicht performant genug ist, dann kann man anfangen manuell zu optimieren. Wenn man das direkt "mitdenken" will, kann man darauf achten:

Wenige Speicherallokationen
Wenig "rum kopieren" von großen Speicherblöcken
Daten linear im Speicher halten (kein Array von array, sondern am Stück und mit den richtigen Indizes drauf zugreifen)

Wenn das nicht reicht, kann man gucken, ob man das sinnvoll parallelisieren kann. Aber, es wäre nicht das erste mal, dass das Erstellen von Threads teurer ist, als der performance Vorteil durch die Parallelisierung.

wob

Dieser Beitrag wurde gelöscht!

EL europ

@Th69 Die Größe der Apfelmänchen ist variabel und kann per ui geändert werden, aber das ist ja "einfache" Funktionalität. Der *thr_Function muss ich noch Parameter hinzufügen um die Farbgebung zu beeinflussen. Aber das ist noch "Zukunftsmusik".

EL europ

@Schlangenmensch
Das "leak" in sort hab ich auch weg bekommen mit:
std::vector<std::array<int, 2>> iterMembers

firefly

@EL-europ sagte in sanitizer::address unlogik?:

@Schlangenmensch
Das "leak" in sort hab ich auch weg bekommen mit:
std::vector<std::array<int, 2>> iterMembers

Für was das std::array der größe 2? Wäre es da nicht besser eine struct mit zwei member zu haben?
Dadurch ist klarer was was ist.

EL europ

@firefly
Das ist Kosmetik denke ich?

firefly

@EL-europ sagte in sanitizer::address unlogik?:

@firefly
Das ist Kosmetik denke ich?

Ist dir überlassen wenn du immer rätseln musst ob array member 0 oder 1 jetzt das korrekte Datum darstellt.

Und das rätseln wird definitv kommen wenn du mal länger nicht an dem Projekt gearbeitet hast und dann daran zurückkehrst.
Aber vermutlich war mein Hinweis hier vergeudete Zeit wenn ich mir so deine Reaktionen auf gut gemeinte Hinweise hier in diesem Thread und dem anderen so anschaue

EL europ

Ich will einen std::vector<MemApple> in ein File speichern:

struct MemApple{
	struct ApplePara paras;
	int pic[100][45];
	std::vector<struct _CiElement> ci_elements;
};

Bei Folgendem Code:

void AppleMemory::writeToFile(){
	std::ofstream file("mem.Apples", std::ios::out|std::ios::binary);
	int count = memApples.size();
	file.write( (char*)(&count), sizeof(int));
	for(int i=0; i<memApples.size(); i++){
		int elem_size = memApples.at(i).ci_elements.size();
		file.write( (char*)(&elem_size), sizeof(int));
		file.write(reinterpret_cast<char*>(&memApples[i]), sizeof(memApples[i]));
	}
	file.close();
}
AppleMemory::AppleMemory(int x, int y, Colorinterface &_ci){
	xpos = x;
	ypos = y;
	ci = &_ci;
	std::ifstream file("mem.Apples", std::ios::in|std::ios::binary);
	if(!file.fail()){
		int count = 0;
		file.read((char*) &count, sizeof(int));
		memApples.resize(count);
		for(int i=0;i<count; i++){
			int size = 0;
			file.read((char*) &size, sizeof(int));
			memApples.at(i).ci_elements.resize(size);
			file.read(reinterpret_cast<char *> (&memApples[i]), sizeof(memApples[i]));
		}
		file.close();
		drawApples();
	}
}

erkennt der leak-sanitizer fehler mit der meldung "new Operator..." bezüglich der ci_elements. Wo ist hier der Fehler?