GCC (STM32) problem mit Optimierer

Gast #6482887
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)

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

7
  if (aFlow > 0)
8
  {
9
    while ((aFlow >= mVxMapUp[tIdx].Flow) && (tIdx < VX_MAP_ELEMENTS ))
10
    {
11
      tRet = mVxMapUp[tIdx].VCmd;
12
      tIdx++;
13
    }
14
  }
15
  else if (aFlow < 0)
16
  {
17
    while ((-aFlow >= mVxMapDn[tIdx].Flow) && (tIdx < VX_MAP_ELEMENTS ))
18
    {
19
      tRet = mVxMapDn[tIdx].VCmd;
20
      tIdx++;
21
    }
22
  }
23
  else
24
  {
25
    tRet = 0;
26
  }
27

28
  return tRet;
29
}
Gast #6482909
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.
#6483297
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.
#6483950
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.
Gast #6485471
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):
1
-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
Angehängte Dateien:
Gast #6485502
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
Persönliche Seite #6485525
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.
Gast #6485534
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.
Gast #6485565
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

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren