[DONE] Compiler Bug gefunden?

Vielleicht habe ich einen Compiler Bug gefunden.
Auf jeden Fall ein Verhalten, welches mir total quer kommt!

Reduziert habe ich es jetzt mal auf ein BlinkTestProgramm.

Verwendet werden meine Task Macros.
Und da scheint der Hase auch im Pfeffer zu liegen.
Die Macros verführen den Compiler zu fehlerhaftem/seltsamen Verhalten.

Beschreibung des Fehlers:

Im Testprogramm werden 2 Objekte erstellt.
a und b
Soweit, so gut.

In der run Methode von a wird auch der Kontext, also "this" von a verwendet.
Auch gut so. Tuts.

In der run Methode von b wird abwechselnd der Kontext von b und dann von a genutzt.
Das ist falsch.
Nochmal: In run() von b, zeigt "this" manchmal auf a
Und damit stimmt die Pause nicht, und mit blinken ist auch nichts.

Testcode:

// https://forum.arduino.cc/index.php?topic=415229.0
#include <TaskMacro.h>

class Blinky 
{
  private:
  byte buttonPin;
  byte led;
  unsigned long pause;


  public:
  Blinky(byte buttonPin,byte led,unsigned long pause): buttonPin(buttonPin),led(led),pause(pause){}
  
  void init()
  {
    pinMode(buttonPin,INPUT_PULLUP);
    pinMode(led,OUTPUT);
    digitalWrite(led,LOW);
  }
  
  void run() // Task
  {
  
    taskBegin();
    while(1)
    {
      taskWaitFor(!digitalRead(buttonPin));
       
      Serial.print("this "); Serial.println(int(this));
      digitalWrite(led,HIGH);
       
      taskPause(pause);
      
      Serial.print("this "); Serial.println(int(this));
      digitalWrite(led,LOW);
       
      taskPause(pause);
    }
    taskEnd();    
  }

};



// 2 Instanzen erzeugen
Blinky a(2,12,500UL); // Pinzuordnung Button=2 Led=12 pause=500ms(Blinkinterval)
Blinky b(3,13,1000UL);



void setup() 
{
  Serial.begin(9600);
  Serial.println("Start");
  a.init();
  b.init();
}

void loop() 
{
  a.run();
  b.run();
}

Weiß einer, warum das so ist?
Eine Idee, wie man das vermeiden kann?

Wenn Du abwechselnd a.run() und b.run() aufrufst, woher willst Du dann wissen, ob eine Ausgabe von a oder b stammt?

Wenn man keine Brücke zu Pin 2 oder 3 schlägt, erfolgen keine Ausgaben.
Wenn man eine Brücke von Pin 2 zu GND schlägt erfolgt das erwartete Verhalten.
Wenn man eine Brücke von Pin 3 zu GND schlägt erfolgt das entartete Verhalten.

woher willst Du dann wissen, ob eine Ausgabe von a oder b stammt?

Nur eine Brücke schlagen.

Versuche mal, den Parametern und internen Variablen unterschiedliche Namen zu geben.

DrDiettrich:
Versuche mal, den Parametern und internen Variablen unterschiedliche Namen zu geben.

Das ist nicht nötig

Die Ausgabe:

Hier der Test mit Brücke zwischen GND und Pin 2

Start
this 468
this 468
this 468
this 468
this 468
this 468
this 468

"this" beinhaltet immer die gleiche Adresse
Wie es auch sein sollte.

Hier der Test mit Brücke zwischen GND und Pin 3

Start
this 462
this 468
this 462
this 468
this 462
this 468
this 462
this 468

"this" wechselt zwischen 2 Adressen
Damit werden die Variablen(Instanz Eigenschaften) aus dem falschen Kontext verwendet.

Versuche mal, den Parametern und internen Variablen unterschiedliche Namen zu geben.

Gerade getestet.
Ändert nichts.
(Hätte mich auch ganz schwer gewundert)

DrDiettrich:
Versuche mal, den Parametern und internen Variablen unterschiedliche Namen zu geben.

Wie soll das bei 2 Instanzen einer Klasse überhaupt gehen?

Wenn man die Makros mal manuell einsetzt, erkennt man, dass da recht seltsame Programme entstehen, die man eigentlich so nie schreiben würde. Ich weis auch nicht ob das überhaupt wirklich zulässig ist, und was da bei der Optimierung des Compilers passiert.
Z.B.:

     switch (Task_mark)
      {
        case 0:
          while (1)
          {
            while (!(digitalRead(buttonPin)))  do {
                Task_mark = 35;
                return ;
              case 35: ;
              } while (0);
            Serial.print (buttonPin); Serial.print(" this "); Serial.println(int(this));
            digitalWrite(led, HIGH);

Da springt der switch mitten in die while-Schleife. Das scheint mir schon sehr verdächtig.

Die while-Schleife sollte eigentlich weg-optimiert werden. Die existiert nur damit das Semi-Kolon am Ende verarbeitet wird.

Vielleicht mal mit den Optimierungs-Optionen des Compilers spielen. Und sehen ob sich da was ändert.

combie:
"this" wechselt zwischen 2 Adressen
Damit werden die Variablen(Instanz Eigenschaften) aus dem falschen Kontext verwendet.

Wenn man da noch ein bisschen mit zusätzlichen Debug-Ausgaben jongliert, erkennt man, dass der this-Pointer schon immer richtig ist. Die unterschiedlichen Pointer-Ausgaben entstammen der Tatsache, dass er sowohl beim Aufruf der Instanz a, als auch beim Aufruf der Instanz b jeweils Ausgaben macht. Er kommt mit den switch-Sprüngen durcheinander, und ich kann mir schon vorstellen, dass das daran liegt, dass in Funktionsblöcke { ... } hineingesprungen wird. Mich wundert schon fast, dass der Compiler das akzeptiert.

Was macht eigentlich print(), wenn man ihm einen Pointer übergibt?

Ein Zeiger ist nur eine Adresse. Wenn man den auf int castet zeigt er die an

Vielleicht mal mit den Optimierungs-Optionen des Compilers spielen. Und sehen ob sich da was ändert.

Die Ausgabe ändert sich nicht.
Leider.
Aber die Codegröße.

Da springt der switch mitten in die while-Schleife. Das scheint mir schon sehr verdächtig.

Mich wundert schon fast, dass der Compiler das akzeptiert.

Die TaskMacros beruhen auf den Protothreads von Adam Dunkel.
Die sind recht weit verbreitet, z.B. Putty ist voll davon.

In Funktionen, also C artigem Code funktioniert das perfekt.
In Methoden nicht.
Verwende ich für jede Instanz eine eigene Klasse, geht auch alles.

Aber bei der Verwendung von mehreren Instanzen einer Klasse läuft es aus dem Ruder.

Kannst Du auch mal meine Task Makros ausprobieren?

DrDiettrich:
Kannst Du auch mal meine Task Makros ausprobieren?

Ja, werde ich machen.
Wird sich aber etwas strecken.
Komme z. Zt. nicht an mein Testequipment ran.

DrDiettrich:
Kannst Du auch mal meine Task Makros ausprobieren?

Getestet!
Das macht leider genau den gleichen Mist.

Aber ich bin dem Problem, an sich, näher auf die Pelle gerückt.
Die Sorge wird nicht vom Quereinstieg in Schleifen Blöcke verursacht.
Das habe ich einzeln getestet.

Das Problem tritt auf, sobald statische Variablen in Methoden genutzt werden.
UND Quereinstiege genutzt werden.

Da muss man nur in die Sprachdefinition schauen, dann ist klar, dass es keinen Sinn macht, statische Variablen dort zu verwenden. Das war also an dieser Stelle sowieso ein Fehler.

Denn diese sind für alle Instanzen gleich. (wenn ich das richtig verstanden habe)
Wenn man das will, dann ok.
Aber hier werden darin Zeiten und Sprungmarken gespeichert, und die müssen sich von Instanz zu Instanz unterscheiden.
Gut.
Mit static bekommt man also Probleme.

Sprungmarken und Zeiten durcheinander hauen, ist eine Sache.
Aber leider sagt mir das noch nicht, warum "this" so unkultiviert wechselt.

Dieses funktioniert:

#include "ExpireTimer.h"
#include "ProtoThread.h"

 class Blinki : public ProtoThread
 {

   private:
     ExpireTimer timer;
     const byte led;
     const byte button;
     const unsigned long interval;

  
   public:
       Blinki(byte led,byte button,unsigned long interval):led(led),button(button),interval(interval){}
       virtual void run()
       {
           taskBegin();
           for(;;)
           {

               taskWaitFor(!digitalRead(button));

               digitalWrite(led,HIGH);
               Serial.print("Led ");Serial.print(led);Serial.println(" HIGH");
               timer.start(interval);
               taskWaitFor(timer.expired());
      
               digitalWrite(led,LOW);
               Serial.print("Led ");Serial.print(led);Serial.println(" LOW");
               timer.start(interval);
               taskWaitFor(timer.expired());
           }
           taskEnd();
       }
       
       virtual void init()
       {
          pinMode(led,OUTPUT);
          pinMode(button,INPUT_PULLUP);
       }
      
 };


Blinki a(12,2,500);
Blinki b(13,3,1000);

void setup() 
{
  Serial.begin(9600);
  Serial.println("Start");
  ProtoThread::initAll();
}

void loop() 
{
  ProtoThread::runAll();
}

Der ganze Code im Anhang. Eine Lib habe ich noch nicht daraus gemacht.

ProtoThread.zip (1.86 KB)

"static" sollte erst mal geklärt werden. Hilfsvariablen sollten bei Verwendung von Task Makros immer static sein, damit sie zwischen den vielen Aufrufen ihre Werte behalten. Da kann man sich mit auto Variablen (in for Schleifen...) schnell Ärger einhandeln. Im vorliegenden Code kann ich allerdings keine solchen Variablen entdecken.

Auch run() als virtuelle Methode ist nicht besonders sinnvoll, das macht allenfalls bei mehreren abgeleiteten Klassen Sinn, in denen die Methode unterschiedlich überschrieben wird.

Möglicherweise liegt das Problem bei Methoden an sich, die ja einen versteckten "this" Pointer übergeben bekommen. Aber wirklich erklären kann ich mir die unterschiedliche Ausgabe nicht. Der Code liegt ja nur einmal im Speicher, wieso sollte der bei a.run() anders laufen als bei b.run()? Genaueres kann man nur dem Assembler-Listing des erzeugten Codes entnehmen.

"static" sollte erst mal geklärt werden. Hilfsvariablen sollten bei Verwedung von Task Makros immer static sein, damit sie zwischen den vielen Aufrufen ihre Werte behalten.

Was für Funktionen richtig ist, ist für Methoden falsch.
Offensichtlich.

Im vorliegenden Code kann ich allerdings keine solchen Variablen entdecken.

Die stecken in den Makros.
Bei dir heißen sie:

static int _mark = 0; static millis_t __attribute__((unused)) time_Stamp = 0;

Und damit ist ein Knall, bei mehr als einer Instanz, vorprogrammiert.
Jede Instanz braucht ihre eigenen.
Sie müssen also ausgelagert werden.
Eigenschaften des Objektes, sie werden müssen, static geht nicht.

Auch run() als virtuelle Methode ist nicht besonders sinnvoll, das macht allenfalls bei mehreren abgeleiteten Klassen Sinn, in denen die Methode unterschiedlich überschrieben wird.

Wird bei mir ja...
Eine abstrakte Basisklasse.
Und dann davon abgeleitet, eine konkrete Implementierung.
Werden sicherlich noch mehr Implementierungen.
Nee, das ist schon ok so...

Den Timer habe ich in eine eigene Klasse ausgelagert, aber dein _mark findet sich jetzt in der abstrakten Basisklasse. Welches auch der Hauptgrund für diese Basisklasse ist.

Die Iteration über alle Kinder, ist nur ein zusätzliches Feature, welches damit möglich wird.

Genaueres kann man nur dem Assembler-Listing des erzeugten Codes entnehmen.

Sehr unübersichtlich.

Der Code liegt ja nur einmal im Speicher, wieso sollte der bei a.run() anders laufen als bei b.run()?

AHA!

:o :o :o :o :o :o :o :o :o

Der Groschen ist gefallen!

DANKE!

Natürlich wird in Methode run von b _mark irgendwann weiter gesetzt.
Diese _mark gilt auch für a, darum macht a genau diesen Schritt.

Der Compiler ist völlig unschuldig!
Und ich habe verstanden, wie statische Variablen in Methoden funktionieren.
Unter Schmerzen.

Tipp: (natürlich auch an die eigene Adresse!)
Vorsicht, mit statischen Variablen in Methoden.
(und, wenn man die dann auch noch in Makros verbirgt, kann man lange suchen)


Dass sich da irgendwelche Symmetrien ergeben haben, von wegen, a tuts immer und b nicht, liegt einfach an der Reihenfolge und Anzahl der Kinder, und schritte.

Ich bin etwas erstaunt dass dir nicht bewusst war dass static in Methoden etwas ganz anderes macht als in Funktionen :o

Serenifly:
Ich bin etwas erstaunt dass dir nicht bewusst war dass static in Methoden etwas ganz anderes macht als in Funktionen :o

Ja, ich auch!
Tut auch ein bisschen weh!

Aber, so ganz anders ist das ja auch nicht.

Sicherlich habe ich das mal gelesen.
Aber es war nicht fest genug verankert.
Die Makro Verschleierung hat gereicht um es aus dem Blickfeld raus zu transportieren.

Wenn ich so nachdenke:
Dann hatte ich bis zu den "PseudoThreads" noch nie das Bedürfnis statische Variablen in Methoden anzulegen.


Der Lohn ist, dass ich das jetzt wohl nie wieder vergessen werde.

Und die einzige Entschuldigung, welche ich habe, ist:
Mein C++ ist so alt, wie meine Anwesenheit hier im Forum.
Die C++ Lernphase ist also noch lange nicht abgeschlossen.
Das merke ich auch an einigen anderen Ecken.