Forum: Mikrocontroller und Digitale Elektronik GCC (STM32) problem mit Optimierer


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 Hugo (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Hallo miteinander

Bei meinem Projekt bin ich bisher im Debug-Build mit Optimierung -Og 
(Debug) unterwegs und es funktioniert soweit wie erwartet. Kürzlich 
versuchte ich den Release-Build, bei welchem ich -Os also auf Grösse 
optimieren möchte.
Nun, das Programm funktionierte nicht mehr ganz korrekt. Ich konnte das 
Problem auf eine Funktion eingrenzen.

Meine Feststellung ist, das wenn ich die Laufvariable tIdx als volatile 
deklariere, die Funktion sowohl bei -Og als auch bei -Os korrekt 
funktioniert. Ohne volatile funktioniert sie nur bei -Og.

Aus meiner Sicht braucht es hier aber kein volatile. Wo klemmt es da?

Compiler: gcc version 7.3.1 20180622 (release) [ARM/embedded-7-branch 
revision 261907] (GNU Tools for Arm Embedded Processors 
7-2018-q2-update)

uint16_t TestFunction(const float aFlow)
{
  uint16_t  tRet = 1;
//  uint16_t  tIdx = 0;           // das funktioniert nicht mit Optimierung -Os (size)
  volatile uint16_t  tIdx = 0;  // das funktioniert mit Optimierung -Os (size)

  if (aFlow > 0)
  {
    while ((aFlow >= mVxMapUp[tIdx].Flow) && (tIdx < VX_MAP_ELEMENTS ))
    {
      tRet = mVxMapUp[tIdx].VCmd;
      tIdx++;
    }
  }
  else if (aFlow < 0)
  {
    while ((-aFlow >= mVxMapDn[tIdx].Flow) && (tIdx < VX_MAP_ELEMENTS ))
    {
      tRet = mVxMapDn[tIdx].VCmd;
      tIdx++;
    }
  }
  else
  {
    tRet = 0;
  }

  return tRet;
}

von Niklas G. (erlkoenig) Benutzerseite


Bewertung
0 lesenswert
nicht lesenswert
Wie sind mVxMapUp, Flow, mVxMapDn und VX_MAP_ELEMENTS definiert? Zeige 
auch den disassemblierten Code der Funktion mit und ohne "volatile".

: Bearbeitet durch User
von foobar (Gast)


Bewertung
6 lesenswert
nicht lesenswert
> while ((aFlow >= mVxMapUp[tIdx].Flow) && (tIdx < VX_MAP_ELEMENTS ))

Das kann zu einem Out-of-Bounds-Zugriff führen.  Besser die beiden 
Vergleiche vertauschen - erst Index überprüfen, dann aufs Arrray 
zugreifen.

von M.K. B. (mkbit)


Bewertung
2 lesenswert
nicht lesenswert
foobar schrieb:
> while ((aFlow >= mVxMapUp[tIdx].Flow) && (tIdx < VX_MAP_ELEMENTS
> ))
>
> Das kann zu einem Out-of-Bounds-Zugriff führen.  Besser die beiden
> Vergleiche vertauschen - erst Index überprüfen, dann aufs Arrray
> zugreifen.

Das wäre dann Undefined Behavior. Dabei können beim Optimieren "Fehler" 
auftreten, wobei bei Undefined Behavior alles erlaubt ist.

Je nach Größe des Arrays könnte z.B. die Schleife aufgelöst und tIdx 
wegoptimiert werden. Mit volatile verbietest du das.

von Markus F. (mfro)


Bewertung
0 lesenswert
nicht lesenswert
ich kann da auf Anhieb - bis auf die Tatsache, dass der direkte 
Vergleich von Integer-Literalen mit floats meist nicht genau das ist, 
was man will - keinen Fehler entdecken.

Wie äussert sich denn das Problem?

Können wir mal die Compiler-Optionen sehen?

: Bearbeitet durch User
von S. R. (svenska)


Bewertung
1 lesenswert
nicht lesenswert
M.K. B. schrieb:
>> Das kann zu einem Out-of-Bounds-Zugriff führen.  Besser die beiden
>> Vergleiche vertauschen - erst Index überprüfen, dann aufs Arrray
>> zugreifen.
>
> Das wäre dann Undefined Behavior.

Die Auswertungsreihenfolge logischer Operatoren ist von links nach 
rechts definiert. Wenn also der linke Ausdruck garantiert, dass der 
rechte Ausdruck kein UB mehr auslösen kann, dann ist alles in Ordnung.

Hier ist das umgekehrt. Ich würde daher empfehlen, das mal umzudrehen 
und zu schauen, ob es dann funktioniert. Im Zweifelsfall auch mal einen 
Blick ins Assembly werfen.

von M.K. B. (mkbit)


Bewertung
0 lesenswert
nicht lesenswert
Ich habe mich unklar ausgedrückt.
Der Out of bound Zugriff wäre Undefined Behavior. Wenn man es 
vertauchscht, dann ist es natürlich nicht mehr Undefined und sollte wie 
erwartet funktionieren.

von S. R. (svenska)


Bewertung
0 lesenswert
nicht lesenswert
Dann sind wir uns ja einig. :-)

von Hugo (Gast)


Angehängte Dateien:

Bewertung
0 lesenswert
nicht lesenswert
Vielen Dank für die zahlreichen Antworten.

Der Fehler äussert sich so, dass der Rückgabewert immer 1 ist. Die 
Schleifen werden also gar nicht ausgeführt.

In der Tat funktioniert es, wenn ich die Abfrage umdrehe: Zuerst den 
Index, dann den anderen Wert.
Ich weiss jetzt nicht so recht, ob mich das beruhigt oder nicht, denn 
das Projekt ist sehr gross...

Ich lege die ergänzenden Daten bei: Deklarationen, Disassembly mit und 
ohne volatile, sowie mit umgedrehter Abfrage.

Die Compileroptionen (includes gekürzt):
-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -D__weak=__attribute__((weak)) -DDEBUG -D__packed=__attribute__((__packed__)) -DUSE_HAL_DRIVER -DSTM32F446xx -I../Drivers/CMSIS/Include -Os -g3 -Wall -fmessage-length=0 -ffunction-sections -c -fmessage-length=0

von foobar (Gast)


Bewertung
1 lesenswert
nicht lesenswert
Ohne volatile hat er das "tIdx < VX_MAP_ELEMENTS" einfach wegoptimiert, 
nach dem Motto: "er greift ja auf das Array zu, dann muß der Index ja 
stimmen".

von foobar (Gast)


Bewertung
1 lesenswert
nicht lesenswert
Btw, wenn ich irgendwo "attribute(packed)" sehe, gehen bei mir die 
Alarmglocken an - das ist so gut wie immer unportabler Pfusch-Code ...

von Nop (Gast)


Bewertung
2 lesenswert
nicht lesenswert
Hugo schrieb:

> In der Tat funktioniert es, wenn ich die Abfrage umdrehe: Zuerst den
> Index, dann den anderen Wert.
> Ich weiss jetzt nicht so recht, ob mich das beruhigt oder nicht, denn
> das Projekt ist sehr gross...

Im Gegensatz zu GCC-Warnungen findet CppCheck das und sagt (als 
Style-Warnung):
"Array index tIdx is used before limits check."

Da dieses Tool frei, kostenlos und sehr benutzerfreundlich ist, kannst 
Du damit das Projekt mal eben durchprüfen. Unter Linux direkt aus dem 
Paketmanager, für Windows von hier:

http://cppcheck.sourceforge.net

von Niklas G. (erlkoenig) Benutzerseite


Bewertung
0 lesenswert
nicht lesenswert
Hugo schrieb:
> Ich weiss jetzt nicht so recht, ob mich das beruhigt oder nicht, denn
> das Projekt ist sehr gross...

Wenn du überall im Code über Array-Grenzen hinweg zugreifst, sollte dich 
das in der Tat beunruhigen, denn das wird früher oder später alle 
möglichen Fehler, gern auch Sicherheitslücken, bewirken.

foobar schrieb:
> Ohne volatile hat er das "tIdx < VX_MAP_ELEMENTS" einfach wegoptimiert,
> nach dem Motto: "er greift ja auf das Array zu, dann muß der Index ja
> stimmen".

Absolut korrekt. Der Compiler darf annehmen, dass Undefined Behaviour 
(UB) niemals auftritt, und da der Zugriff über die Array-Grenze hinweg 
UB ist, nimmt der Compiler an, dass dieser Fall nicht eintritt.

foobar schrieb:
> Btw, wenn ich irgendwo "attribute(packed)" sehe, gehen bei mir die
> Alarmglocken an - das ist so gut wie immer unportabler Pfusch-Code ...

Das wird vermutlich von einer Library gebraucht, ich glaube die HAL 
nutzt das.

von Nop (Gast)


Bewertung
0 lesenswert
nicht lesenswert
foobar schrieb:
> Btw, wenn ich irgendwo "attribute(packed)" sehe, gehen bei mir die
> Alarmglocken an - das ist so gut wie immer unportabler Pfusch-Code ...

Ich nutze das manchmal auf Cortex-M, aber mit einem Pack-Alignment von 4 
Bytes, da auf einem 32-Bitter Datentypen wie uint64_t keine 8 Bytes 
Alignment brauchen.

von Hugo (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Niklas G schrieb:
> Wenn du überall im Code über Array-Grenzen hinweg zugreifst, sollte dich
> das in der Tat beunruhigen, denn das wird früher oder später alle
> möglichen Fehler, gern auch Sicherheitslücken, bewirken.

Ja da hast du absolut recht: Asche auf mein Haupt...

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]
  • [avrasm]AVR-Assembler-Code[/avrasm]
  • [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.

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