Forum: PC-Programmierung Stack corruption, aber valgrind und asan finden nichts, was kann ich noch tun?


von Daniel A. (daniel-a)


Angehängte Dateien:

Lesenswert?

Ich habe wiedermal einer dieser Fehler...  Im Grunde habe ich alle 
möglichen Fehlerursachen bereits ausgeschlossen, also könnte das Problem 
wieder überall sein.

Die Funktion, die im eigentlichen Programm nicht tut, was sie soll, habe 
ich mal direkt isoliert in einem Minimalprogramm aufgerufen, mit den 
gleichen Daten, und dort kommt das erwartete Resultat raus. Aber im 
richtigen Programm, wird eine if Bedingung ausgeführt, dessen Bedingung 
ich direkt vorher und nachher ausgebe, und die mit den werten eigentlich 
nicht ausgeführt werden dürfte. Der generierte Assemblercode im 
Minimalprogramm und im richtigen Programm ist der selbe. Da ich eine 
Form von Stack Korruption vermutete, habe ich Valgrind und ASAN darüber 
laufen lassen. Diese haben absolut nichts gefunden. Nur ein paar 
bekannte memory leaks in der curses library, aber absolut nichts in 
meinem Programm. Ich kompiliere es auch noch mit -fstack-protector-all, 
das hat auch nichts bemerkt. Der Fehler ist relativ robust, immerhin 
haben ein paar printf ausgaben ihn nicht verändert.

Ich bin mit meinem Latein gerade etwas am ende, was kann ich noch 
versuchen?

Ich sehe den Fehler im in der Funktion "tym_i_pane_set_cursor_position", 
bei der Zeile "if(y >= bottom)":
https://github.com/Daniel-Abrecht/libttymultiplex/blob/work-in-progress/src/pane.c#L320

Die debug ausgaben in Fehlerfall, die so eigentlich nicht möglich sein 
dürften, sind:
1
scroll_region_valid: false origin_mode: false
2
ox->x: 0 0 oy->y: 2 -1
3
sr top: 0 sr bottom: 59
4
1 ax: 0 ay: 1
5
top: 0 bottom: 59
6
2 ax: 0 ay: 1
7
!!!: 1 59 -57
8
tym_i_scroll_def_scrolling_region t 0 b 59 h 59 n -57
9
fx: 0 fy: 58

Wie an der Ausgabe zu erkennen ist, wurde der Ausdruck (y >= bottom) mit 
y=1 und bottom=59 irgendwie als wahr ausgewertet...

Das Minimalprogram liefert die erwartete Ausgabe:
1
scroll_region_valid: false origin_mode: false
2
ox->x: 0 0 oy->y: 2 -1
3
sr top: 0 sr bottom: 59
4
1 ax: 0 ay: 1
5
top: 0 bottom: 59
6
2 ax: 0 ay: 1
7
fx: 0 fy: 1

Die library verwende ich dabei mit folgendem Programmen:
https://github.com/Daniel-Abrecht/console-keyboard-multiplexer
https://github.com/Daniel-Abrecht/console-keyboard-basic

Der Fehler tritt auf, wenn "console-keyboard-multiplexer bash -l" 
aufrufe, und dann in dem Programm "echo -e '\e[1A'" ausführe, das wird 
in "parser.c" geparst, der ruft dann die "tym_i_csq_cursor_up", und die 
wiederum ruft die "tym_i_pane_set_cursor_position" auf, in der das 
Problem sichtbar wird.

Das Minimalprogram, in dem der Fehler, in dem das Problem nicht 
auftritt, und ein disassembly mit gcc der relevanten Funktion, ist im 
Anhang, aber ich glaube nicht, das dort das Problem liegt.

Was kann ich jetzt noch versuchen, um dem Fehler auf die Schliche zu 
kommen?

von Kaj (Gast)


Lesenswert?

Daniel A. schrieb:
> Was kann ich jetzt noch versuchen, um dem Fehler auf die Schliche zu
> kommen?
Wenn es ein Coredump gibt, dann da mal mit gdb reingucken.
Einmal cppcheck ueber das Projekt laufen lassen und das ganze mal mit 
clang und sanitizer-Optionen compilieren und dann ausfuehren.

von DPA (Gast)


Lesenswert?

Kaj schrieb:
> Daniel A. schrieb:
>> Was kann ich jetzt noch versuchen, um dem Fehler auf die Schliche zu
>> kommen?
> Wenn es ein Coredump gibt, dann da mal mit gdb reingucken.

Leider stürzt es nicht ab, sonst könnte ich mit dem Debugger schauen ws 
passiert. Es werden soweit ich sehe auch keine Variablen unerwartet 
überschrieben, sonst könnte ich das mit einem watchpoint im debugger 
weiter analysieren. Ich sehe nur den einen Seiteneffekt, wo an der einen 
stelle eine Bedingung ausgeführt wird, wenn sie nicht ausgeführt werden 
sollte.

> Einmal cppcheck ueber das Projekt laufen lassen

OK, Ich werde das heute Abend mal ausprobieren, mal seen ob das Tool mit 
meinem Codestyle umgehen kann. Es ist zwar (abgesehen von der Verwendung 
von weak und constructor attributen) gültiger C99 Code, aber einige 
Funktionen werden auf eine art und weise referenziert und angesprungen, 
die es für Tools schwierig machen kann zu erkennen, dass diese überhaupt 
verwendet werden, und andere Funktionen werden mit einem weak Symbol 
referenziert, werden aber nur aufgerufen nach einer Überprüfung ob diese 
auch tatsächlich existieren, was nicht immer der Fall ist. Bin schon 
gespannt, wie das Tool darauf reagiert.

> und das ganze mal mit clang und sanitizer-Optionen compilieren und dann
> ausfuehren.

Gute Idee. Clang nutzt zwar auch ASAN, aber andere Compiler und Optionen 
können natürlich andere Probleme finden und das Fehlerbild ändern. Falls 
ich das Programm so dazu bringen kann, nach der eigentlichen 
Problemursache gleich abzustürzen, dann hätte ich wieder einen 
Ansatzpunkt.

von Oliver S. (oliverso)


Lesenswert?

Ohne da groß in die Details eingestiegen zu sein, hilft vielleicht das 
hier weiter:

https://blog.regehr.org/archives/268

Oliver

von DPA (Gast)


Lesenswert?

Oliver S. schrieb:
> Ohne da groß in die Details eingestiegen zu sein, hilft vielleicht das
> hier weiter:
>
> https://blog.regehr.org/archives/268

Danke, dass ein Vergleich "((long)-1) > ((unsigned)1)" auf einigen 
Architekturen das "falsche" Resultat liefern kann, wusste ich nicht. Das 
muss ich bei mir im Code definitiv korrigieren. Eigentlich hab ich beim 
Kompilieren "-Wall -Wextra -pedantic -Werror" mitgegeben, aber der gcc 
scheint mir in letzter zeit recht unzuverlässig damit, Warnungen zu 
generieren.

Den Fehler, den ich Sehe, erklärt es aber trotzdem nicht. Ganz egal, wie 
der Compiler "((long)1) >= ((unsigned)59)" implizit castet, das dürfte 
trotzdem nie wahr sein, und im Minimalbeispiel tut es an der Stelle 
auch, was ich erwarten würde. Der vergleich ist ja kein UB, sondern nur 
im Standard Scheisse definiert.

von CppBert3 (Gast)


Lesenswert?

auf jeden Fall einen aktuellen GCC 8 oder besser 9 mit UBSan probieren
die Warnungen/Prüfungen werden ja staendig verbessert, auch mal den 
clang probieren - moeglicherweise veraendert sich dein Fehlerbild 
dadurch

von Daniel A. (daniel-a)


Lesenswert?

cppcheck 1.76.1 hab ich nun drüber laufen lassen. Eine Funktion, die ich 
falsch benannt habe, habe ich mit dessen unused function information 
gefunden und noch ein paar Kleinigkeiten korrigiert:
https://github.com/Daniel-Abrecht/libttymultiplex/commit/609f4ee014d3df1e4ba6446e9b08ac4ac8e5b947

Ansonsten findet cppcheck jetzt selbst mit den "--enable=all 
--inconclusive" Optionen nichts mehr, abgesehen von ein paar unused 
Funktion Warnungen und ein paar "Redundant assignment of 'X' to itself" 
Warnungen, wo cppcheck die designated initialiser der form "(S){.a=a}" 
nicht richtig interpretiert. Die momentane Ausgabe ist im Anhang.

Die Debugausgabe hat sich geändert, die macht jetzt Sinn:
1
ox->x: 0 0 oy->y: 2 4294967295
2
sr top: 0 sr bottom: 59
3
1 ax: 0 ay: 4294967297
4
top: 0 bottom: 59
5
2 ax: 0 ay: 4294967297
6
!!!: 4294967297 59 4294967239
7
tym_i_scroll_def_scrolling_region t 0 b 59 h 59 n -57
8
fx: 0 fy: 58

Wie da vorher -1 statt 4294967297 zustande kam ist mir schleierhaft. Der 
Fehler ist in der cursor_up Funktion, und genauso vermutlich auch noch 
in ein paar anderen Funktionen. Muss ich jetzt korrigieren.

Damit wäre die Ursache klar. Mein Fehler war das bei so was hier:
1
void test(long x){
2
  printf("%ld\n", x);
3
}
4
5
int main(){
6
  unsigned a = 1;
7
  test(-a);
8
  return 0;
9
}
Ein unsigned integer underflow auftritt.

Einen schlechten Beigeschmack hat es trotzdem noch, ich weiss noch 
nicht, wie es zur ursprünglichen Ausgabe kommen konnte. Das kommt sicher 
irgendwann wieder zurück...

Compilieren mit clang 6 bringt auch keine Warnungen. UBsan und ASan hab 
ich damit jetzt aber nicht ausprobiert.

Wie auch immer.
Danke an alle, das Problem ist gelöst.

von Kaj (Gast)


Lesenswert?

Na siehste mal. Wieder mit einem einfachen Tool den Code besser gemacht 
und wieder was dazu gelernt. :)
Freut mich das wir helfen konnten.

Gruesse

von Daniel A. (daniel-a)


Angehängte Dateien:

Lesenswert?

Ok, eine Sache wäre da doch noch... gcc 6.3.0 und clang 6.0.0 mit 
"-std=c99 -Wall -Wextra -pedantic -Werror", aber auch cppcheck 1.76.1 
haben bei den unsigned vs signed vergleichen und den integer underflows 
komplett versagt, mich zu warnen. Gibt es ein Tool, welches da etwas 
zuverlässiger ist?

Edit: Und hier auch noch die im letzten Beitrag vergessene momentane 
cppcheck Ausgabe.

: Bearbeitet durch User
von Heiko L. (zer0)


Lesenswert?

Daniel A. schrieb:
> Compilieren mit clang 6 bringt auch keine Warnungen. UBsan und ASan hab
> ich damit jetzt aber nicht ausprobiert.

Die hätten vermutlich auch nichts gefunden, da es kein UB ist. Die 
-Wconversion Familie wäre wohl eher das richtige gewesen.

von DPA (Gast)


Lesenswert?

Heiko L. schrieb:
> Daniel A. schrieb:
>> Compilieren mit clang 6 bringt auch keine Warnungen. UBsan und ASan hab
>> ich damit jetzt aber nicht ausprobiert.
>
> Die hätten vermutlich auch nichts gefunden, da es kein UB ist. Die
> -Wconversion Familie wäre wohl eher das richtige gewesen.

Gerade mal mit gcc 8.3.0 und "-Wconversion -Wall -Wextra -pedantic 
-Werror" folgendes Programm kompiliert:
1
#include <stdio.h>
2
3
void test(long x){
4
  printf("%ld\n", x);
5
  unsigned bla = 1;
6
  if(x >= bla)
7
    printf("test\n");
8
}
9
10
int main(){
11
  unsigned a = 1;
12
  test(-a);
13
  return 0;
14
}

Keine Warnung, nichts! Ich meine, klar, technisch gesehen ist das nicht 
falsch, das -a wird nicht zu einem signed int durch das minus, und der 
unsigned int kann sich nicht ändern, wenn er implizit in einen auf der 
Architektur grösseren long konvertiert wird, und UB ist es auch nicht, 
usw.  Aber trotzdem were es schön, wenn man da irgendwie gewarnt werden 
könnte...

von Heiko L. (zer0)


Lesenswert?

DPA schrieb:
> Keine Warnung, nichts!

Clang warnt an der Stelle auf 64bit Plattformen, was aber eigentlich ein 
Fehler zu sein scheint. gcc warnt mit Wconversion auch auf einer 32-bit 
Plattform nicht, was ebenso ein Fehler ist.
Nur msvc gibt schon mit Wall sogar Warnungen, wie du sie gerne hättest.

https://godbolt.org/z/5aZUai

Man könnte also sagen, dass das schon ginge, wenn nicht....

: Bearbeitet durch User
von cppbert3 (Gast)


Lesenswert?

Was sagt UBSan?

von Rolf M. (rmagnus)


Lesenswert?

DPA schrieb:
> Oliver S. schrieb:
>> Ohne da groß in die Details eingestiegen zu sein, hilft vielleicht das
>> hier weiter:
>>
>> https://blog.regehr.org/archives/268
>
> Danke, dass ein Vergleich "((long)-1) > ((unsigned)1)" auf einigen
> Architekturen das "falsche" Resultat liefern kann, wusste ich nicht.

Es ist praktisch immer eine schlechte Idee, signed und unsigned zu 
mischen.

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.