Forum: PC-Programmierung Problem mit Hex2bix. (cypress Tool)


Announcement: there is an English version of this forum on EmbDev.net. Posts you create there will be displayed on Mikrocontroller.net and EmbDev.net.
von Thomas (Gast)


Angehängte Dateien:

Bewertung
0 lesenswert
nicht lesenswert
Ich benutze das Tool schon sehr lange und kenne die Aufrufparameter. 
Unter XP funktioniert es immer noch auch in einer VM. Auf einem Win10 
System geht es leider nicht.
Das Tool würde ursprünglich mit MSC 6 übersetzt. Auch ein Neuübersetzung 
wie von cypress in einem Thread empfohlen hat nicht weitergeholfen.
Da ich mir das Fehlverhalten (es kommt ein Fehler bezüglich der 
Speicherzugriffe) nicht so recht erklären konnte habe ich den Debugger 
angeworfen und das näher angeschaut.

Letztendlich könnte ich das Problem auf malloc bei den Filenamen 
eingrenzen.
Im Code wird per malloc Speicher für die Filenamen angefordert
1
Outfilename=(char*)malloc(strlen(argv[i]+EXT_LENGTH));
2
strcopy(Outfilename,argv[i];
soweit so gut oder auch nicht ich kann da erst Mal nichts falsches 
erkennen und wie gesagt unter XP geht ja auch alles wie erwartet.
Wenn ich nun den Speicher nicht per malloc anfordern sondern die 
Filenamen als globale Variablen mit char[256] definiere funktioniert das 
Tool auch unter W10.

Nun zur eigentlichen Frage:
was passiert unter W10?  Wieso arbeitet malloc jetzt auf einmal anders?
Ist das vielleicht ein Emulation Problem von WOW64?
Allgemein gefragt :
Wie kann ich sowas für mich verhindern?

Noch ein Hinweis. Das Tool hat noch ein Problem mit Intel Hex 
Checksummen die werden nicht korrekt behandelt. Das ist aber eine andere 
Baustelle und sehr einfach zu fixen. In alten Versionen von Hex2bix 
wurde die checksum Prüfung einfach auskommentiert

falls von Interesse hier meine Aufrufparameter
..\..\tools\hex2bix -IR -O .\obj\outf.eep .\obj\infile.hex

Thomas

von mh (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Thomas schrieb:
1
Outfilename=(char*)malloc(strlen(argv[i]+EXT_LENGTH));

Sicher dass es nicht
1
Outfilename=(char*)malloc(strlen(argv[i])+EXT_LENGTH);
sein sollte?

von Thomas (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Ja ich bin mir sicher und das ist wohl auch das Problem .... Manchmal 
sieht man halt den Wald vor lauter Bäumen nicht. Der Code ist ja im 
Anhang.
Die Frage bleibt trotzdem warum geht sowas unter XP?
 Danke für den Hinweis.
Thomas

von Kaj (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Ach du... das sollte man aber patchen...
1
[...]
2
3
case 'O':
4
  i++;
5
  OutFilename = (char *)malloc(strlen(argv[i]+EXT_LENGTH+1));
6
  strcpy(OutFilename, argv[i]);
7
  cont = FALSE;
8
  break;
9
10
[...]
11
12
if (OutFilename == NULL)
13
{
14
  root_length = strcspn(InFilename,".");
15
  OutFilename = (char *)malloc(root_length+EXT_LENGTH);
16
  memcpy(OutFilename,InFilename,root_length);
17
  OutFilename[root_length] = 0;
18
  strcat(OutFilename,Extension[OutFileType]);
19
}
20
21
[...]
22
23
Image = (BYTE *)malloc(MEMORY_BUFFER_SIZE);
24
ImageMap = (BYTE *)malloc(MEMORY_BUFFER_SIZE+2);
25
memset(Image,MEM_FILL,MEMORY_BUFFER_SIZE);
26
memset(ImageMap,FALSE,MEMORY_BUFFER_SIZE);
27
28
[...]

Soweit ich das beim ueberfliegen sehe, wird nie geprueft, ob man 
ueberhaupt speicher bekommen hat. Da gehoert zu jedem malloc noch ein
1
if (ptr == NULL) {
2
  // FEHLER! Mach was sinnvolles, z.B. Fehlerbehandlung...
3
}

von Dr. Sommer (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Thomas schrieb:
> Die Frage bleibt trotzdem warum geht sowas unter XP?

Vielleicht gibt der Kernel unter XP größere Adressbereiche zurück sodass 
der dann folgende Buffer Overflow Fehler unentdeckt bleibt. So kann man 
sich natürlich beliebige (Sicherheits-)Probleme einfangen. Es "geht" 
also nur zufällig. Also froh sein dass Win10 hier strenger ist und 
solche Fehler auffallen...

von Kaj (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Und sowas
1
memset(ImageMap,FALSE,MEMORY_BUFFER_SIZE);
2
                ^^^^^
ist auch ganz dolle pfui! Wenn man den Speicher mit 0 fuellen will, dann 
sollte man auch hinschreiben, dass man 0 meint, und nicht 'false'...

Der ganze Code sollte mal ueberarbeitet werden. Ist ja gruselig...

von Dr. Sommer (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Kaj schrieb:
> Der ganze Code sollte mal ueberarbeitet werden. Ist ja gruselig...

So ist C doch immer - gehört zum guten Ton! Gute alte 
Programmier-Wertarbeit :-)

von Thomas (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Das der Code nicht so dolle ist war mir schon klar. Um ehrlich zu sein 
habe ich mir das nie angeschaut da ja alles bisher funktioniert hat. Ich 
habe halt immer die exe von Cypress verwendet.
Das ganze ist mir ja erst aufgefallen als ich die Entwicklermaschine 
getauscht hatte.
Ich finde das ganze schon ziemlich interessant. Hätte cypress nicht den 
Quellcode offengelegt, hätte ich das Tool selber nachprogrammieren 
müssen.

Meine Frage wie ich das für mich verhindern kann hat sich aber erledigt.

Thomas

von Kaj (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Auch das hier:
1
    if (compressIIC)
2
        while((ImageMap[addr] || ImageMap[addr+1] || ImageMap[addr+2]) && (addr < MemSize) && (i++ < 0x3ff))
3
            ++addr;
4
    else
5
        while(ImageMap[addr] && (addr < MemSize) && (i++ < 0x3ff))
6
            ++addr;

Was CppCheck dazu sagt:
1
Summary: Array index 'addr' is used before limits check.
2
3
Message: Defensive programming:
4
The variable 'addr' is used as an array index before it is checked that
5
is within limits. This can mean that the array might be accessed out of
6
bounds. Reorder conditions such as '(a[i] && i < 10)' to '(i < 10 && a[i])'.
7
That way the array will not be accessed if the index is out of limits.
Das dieses Programm jemals (ansatzweise) korrekt funktioniert hat grenzt 
an Zauberei...

von Thomas (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Naja an dem Tool wurde sehr oft rumgeflickt. Ich habe aleine 4 Versionen 
davon und bin mir nicht sicher ab ich die aktuellste Version habe. Da 
ich nur eine Teilmenge der Optionen bisher genutzt habe waren mir 
Optionen wie das compressed iic bisher egal.
Bei Cypress ist da alles auf obsolet gestellt. Ich will gar nicht wissen 
wie viele Programme noch im Umlauf sind die ähnliche Probleme haben.
Thomas

von Rufus Τ. F. (rufus) (Moderator) Benutzerseite


Bewertung
0 lesenswert
nicht lesenswert
Man kann Dateinamen in char-Arrays fester Länge unterbringen; dafür kann 
man das Macro MAX_PATH verwenden, das mit #include <windows.h> zur 
Verfügung steht.

Weder ist es für ein paar Dateinamen nötig, mit malloc zu arbeiten, noch 
ist es nötig, hier ein paar Bytes zu sparen und weniger als MAX_PATH 
anzufordern.

Um windows.h einbinden zu können, sollte man allerdings das enum "BOOL" 
entfernen, sonst dürfte es mit den enum-Werten TRUE und FALSE zu 
Konflikten kommen.

: Bearbeitet durch Moderator
von Dr. Sommer (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Wenn man natürlich ein Programm unbedingt unportabel machen will... 
Außerdem können Pfade teilweise doch länger sein, siehe:

https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file

von Christian R. (supachris)


Bewertung
0 lesenswert
nicht lesenswert
Seltsam, das scheint dann aber nicht immer aufzutreten. Ich nutze das 
Tool auch um die EEPROM Files für den FX2 zu erzeugen, und das klappt 
auch unter Win 10 x64 einwandfrei.
Version müsste ich mal nachschauen...

von Dr. Sommer (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Das ist das schöne an "Undefined behaviour" (hier: das Schreiben auf 
nicht allokierten Speicher dank zu kleiner Allokation) - mal 
funktioniert's, mal stürzt es ab, mal produziert es klammheimlich 
falsche Ergebnisse, mal formatiert es deine Festplatte. Deswegen ist es 
wichtig, korrekte Programme zu schreiben, und nicht nur solche, die 
"bei mir ja funktionieren".

von Thomas (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Christian R. schrieb:
> Seltsam, das scheint dann aber nicht immer aufzutreten

Das Problem existiert sobald man mit -O das Ausgabefile erstellt. Bei 
der automatischen Erzeugung ist alles okay

Thomas

von Steffen R. (steffen_rose)


Bewertung
0 lesenswert
nicht lesenswert
Kaj schrieb:
> Und sowas
>
1
> memset(ImageMap,FALSE,MEMORY_BUFFER_SIZE);
2
>                 ^^^^^
3
>
> ist auch ganz dolle pfui! Wenn man den Speicher mit 0 fuellen will, dann
> sollte man auch hinschreiben, dass man 0 meint, und nicht 'false'...
>
> Der ganze Code sollte mal ueberarbeitet werden. Ist ja gruselig...

Da das Array ImageMap Booleans enthält, erscheint es mir richtig.
Es soll mit False initialisiert werden, nicht mit 0.
1
ImageMap[rec_addr+i] = TRUE;

und an anderer Stelle wird
1
addr < MemSize
als Schleifenabbruch genutzt und nicht zur Prüfung, dass der 
Eingabeparameter addr zu groß ist. Der wird durch den Kontext maximal 
addr == MemSize.
Nicht toll, aber dass es nur zufällig funktioniert... Na ja.

Kaj schrieb:
> Soweit ich das beim ueberfliegen sehe, wird nie geprueft, ob man
> ueberhaupt speicher bekommen hat.

Auch nicht toll. Aber wenn man die paar Byte nicht bekommt, dann hat man 
ein größeres Problem.

Natürlich ist der Source nicht das gelbe vom Ei. Aber der TO will es 
auch nur wieder zum Laufen bekommen. Nicht das Ziel aus den Augen 
verlieren.

von Thomas (Gast)


Angehängte Dateien:

Bewertung
0 lesenswert
nicht lesenswert
Um das ganze abzuschließen im Anhang eine geänderte Version im Quelltext 
und compiliert
Sowie eine kleine Beschreibung dazu.
Es sind zwei Bugs behoben:
zum einen die Sache mit den Intel Hex checksum zum andern der Bug mit 
malloc bei -@O Filename.
Danke an alle die geholfen haben.

Thomas

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.
Hinweis: der ursprüngliche Beitrag ist mehr als 6 Monate alt.
Bitte hier nur auf die ursprüngliche Frage antworten,
für neue Fragen einen neuen Beitrag erstellen.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.