Arduino compilation weirdness

Question

I'm working on OSX Mavericks, standard Arduino.app, and Arduino 2560 board.

This is by far, the single strangest bug I have ever encountered.

Long story short, this code:

  #include "Arduino.h"
  #include "Bitstreamer.h"
  #include "TimerOne.h"
  #include "BusyWaitByteProtocol.h"



  Bitstreamer::Bitstreamer(int pin)
  {
    _pin = pin;
    stage = 0;
    bitholder = new boolean[8];
    bitindex = 0;
    bp = new BusyWaitByteProtocol (128);
  }


  Bitstreamer* Bitstreamer::Daemonholder = NULL;
  void Bitstreamer::RunDaemon()
  {
    Bitstreamer::Daemonholder->Run();
  }

  void Bitstreamer::Listen()
  {
     myTurn = false;
     Init();
  }

  void Bitstreamer::Shout()
  {
     myTurn = true;
     Init();
  }


  void Bitstreamer::Init()
  {
     Daemonholder = this;
     Timer1.initialize(cycleRate);
     Timer1.attachInterrupt(RunDaemon); 
     stage++;
     screamcounter = 0;
     bitcounter = 0;
     lastbit = false;
  }

  void Bitstreamer::Run()
  {
    if (stage == 1)
    {
      //We are initializing the protocol. If we are the shouter, scream for a bit. If we are the listener, wait for scream to end.
      //The scream really just serves as an initial syncronizer.
      if (myTurn)
      {
        pinMode(_pin, OUTPUT);
        digitalWrite(_pin, HIGH);
      }
      else
      {
         pinMode(_pin, INPUT);
      }
      stage++;
    }
    else if (stage == 2)
    {
      if (myTurn)
      {
        screamcounter++;
        if (screamcounter > 100)
        {
          digitalWrite(_pin, LOW);
          stage++;
        }
      }
      else
      {
          int state = digitalRead(_pin);
          if (state == 1)
          {
            screamcounter++;
          }
          else
          {
            if (screamcounter > 30)
            {
              stage++;
            }
            screamcounter = 0;
          }
      }
    }
    else if (stage == 3)
    {
      if (myTurn)
      {
        bitcounter++;
        if (bitcounter == 0)
        {
            digitalWrite(_pin, bitholder[bitindex]?HIGH:LOW);
        }
        else
        {
          if (bitcounter >= resendrate)
          {
            bitcounter = -1;
            bitindex++;
          }
          if (bitindex >= 8)
          {
            bitindex = 0;
            uint8_t nb = bp->RequestByte();
            ReadBits(nb, bitholder);
            //Swap turns as well
          }
        }
      }
      else
      {
        boolean state = (digitalRead(_pin) != 1);
        if (state != lastbit)
        {
          float bits = ((float)bitcounter)/((float)resendrate);
          int finalbits = round(bits);

          debugstring = String(debugstring + String(state) + String(" ") + String(finalbits) + String("\n"));

          //Process received bits
          for (int i = 0; i < finalbits; i++)
          {
            bitholder[bitindex] = state;
            bitindex++;
            if (bitindex >= 8)
            {
              bitindex = 0;
              uint8_t nb = WriteBits(bitholder);
              bp->ProcessByte(nb);
              //Swap turns
            }
          }

          lastbit = state;
          bitcounter = 0;
        }
        else
        {
          bitcounter++;
        }
      }
    }

    /*
    if (myTurn)
    {
      if (hitcount < hits + 3)
      {
        digitalWrite(_pin, ((hitcount++)<hits)?HIGH:LOW);
      }
      else
      {
        hitcount = 0;
        hits++;
        lhits = hits;
      }
    }
    else
    {
      int state = digitalRead(_pin);
      if (state == 0)
      {
        hitcount++;
      }
      else
      {
        hitcount = 0;
        hits++;
      }
      //lhits = hits;
      if (hitcount >= 3)
      {
        lhits = hits;
        hits = 0;
      }
    }
    */
  }

  byte Bitstreamer::Read()
  {
    //Load from incoming byte buffer
    return 0;  
  }

  void Bitstreamer::Write(byte b)
  {
    //Send to outgoing byte buffer
  }

  void Bitstreamer::ReadBits(uint8_t b,  boolean* bitarray)
  {
    //DRAGONHERE: For loop would have been less code, but required initialization of another variable, and slightly more complicated logic for the processor
    //Also, not having it in a for loop makes it easier for others to understand
    bitarray[0] = (b & 1) != 0;    // 1 = 00000001, IE 1st bit. (1 & b) lets only 1st bit through. != 0 checks if zero.
    bitarray[1] = (b & 2) != 0;    // 1 = 00000010, IE 2nd bit. (2 & b) lets only 2nd bit through. != 0 checks if zero.
    bitarray[2] = (b & 4) != 0;    // 1 = 00000100, IE 3rd bit. (4 & b) lets only 3rd bit through. != 0 checks if zero.
    bitarray[3] = (b & 8) != 0;    // 1 = 00001000, IE 4th bit. (8 & b) lets only 4th bit through. != 0 checks if zero.
    bitarray[4] = (b & 16) != 0;   // 1 = 00010000, IE 5th bit. (16 & b) lets only 5th bit through. != 0 checks if zero.
    bitarray[5] = (b & 32) != 0;   // 1 = 00100000, IE 6th bit. (32 & b) lets only 6th bit through. != 0 checks if zero.
    bitarray[6] = (b & 64) != 0;   // 1 = 01000000, IE 7th bit. (64 & b) lets only 7th bit through. != 0 checks if zero.
    bitarray[7] = (b & 128) != 0;  // 1 = 10000000, IE 8th bit. (128 & b) lets only 8th bit through. != 0 checks if zero.

    //PLEASE NOTE: These explainations are based on the assumption that the bit order on Arduino is MSB-Left.
    //Notably, this method of exploding bits will actually dynamically adjust to the bit order of the given system.
  }

 uint8_t Bitstreamer::WriteBits(boolean* b)
 {
    //DRAGONHERE: same as above
    return
    (b[0]? 1 : 0) +
    (b[1]? 2 : 0) +
    (b[2]? 4 : 0) +
    (b[3]? 8 : 0) +
    (b[4]? 16 : 0) +
    (b[5]? 32 : 0) +
    (b[6]? 64 : 0) +
    (b[7]? 128 : 0);
}

compiles just fine, uploads to the Arduino, and works just as expected.

But this code:

#include "Arduino.h"
  #include "Bitstreamer.h"
  #include "TimerOne.h"
  #include "BusyWaitByteProtocol.h"



  Bitstreamer::Bitstreamer(int pin)
  {
    _pin = pin;
    stage = 0;
    bitholder = new boolean[8];
    bitindex = 0;
    bp = new BusyWaitByteProtocol (128);
  }


  Bitstreamer* Bitstreamer::Daemonholder = NULL;
  void Bitstreamer::RunDaemon()
  {
    Bitstreamer::Daemonholder->Run();
  }

  void Bitstreamer::Listen()
  {
     myTurn = false;
     Init();
  }

  void Bitstreamer::Shout()
  {
     myTurn = true;
     Init();
  }


  void Bitstreamer::Init()
  {
     Daemonholder = this;
     Timer1.initialize(cycleRate);
     Timer1.attachInterrupt(RunDaemon); 
     stage++;
     screamcounter = 0;
     bitcounter = 0;
     lastbit = false;
  }

  void Bitstreamer::Run()
  {
    if (stage == 1)
    {
      //We are initializing the protocol. If we are the shouter, scream for a bit. If we are the listener, wait for scream to end.
      //The scream really just serves as an initial syncronizer.
      if (myTurn)
      {
        pinMode(_pin, OUTPUT);
        digitalWrite(_pin, HIGH);
      }
      else
      {
         pinMode(_pin, INPUT);
      }
      stage++;
    }
    else if (stage == 2)
    {
      if (myTurn)
      {
        screamcounter++;
        if (screamcounter > 100)
        {
          digitalWrite(_pin, LOW);
          stage++;
        }
      }
      else
      {
          int state = digitalRead(_pin);
          if (state == 1)
          {
            screamcounter++;
          }
          else
          {
            if (screamcounter > 30)
            {
              stage++;
            }
            screamcounter = 0;
          }
      }
    }
    else if (stage == 3)
    {
      if (myTurn)
      {
        bitcounter++;
        if (bitcounter == 1)
        {
            digitalWrite(_pin, bitholder[bitindex]?HIGH:LOW);
        }
        else
        {
          if (bitcounter >= resendrate)
          {
            bitcounter = 0;
            bitindex++;
          }
          if (bitindex >= 8)
          {
            bitindex = 0;
            uint8_t nb = bp->RequestByte();
            ReadBits(nb, bitholder);
            //Swap turns as well
          }
        }
      }
      else
      {
        boolean state = (digitalRead(_pin) != 1);
        if (state != lastbit)
        {
          float bits = ((float)bitcounter)/((float)resendrate);
          int finalbits = round(bits);

          debugstring = String(debugstring + String(state) + String(" ") + String(finalbits) + String("\n"));

          //Process received bits
          for (int i = 0; i < finalbits; i++)
          {
            bitholder[bitindex] = state;
            bitindex++;
            if (bitindex >= 8)
            {
              bitindex = 0;
              uint8_t nb = WriteBits(bitholder);
              bp->ProcessByte(nb);
              //Swap turns
            }
          }

          lastbit = state;
          bitcounter = 0;
        }
        else
        {
          bitcounter++;
        }
      }
    }

    /*
    if (myTurn)
    {
      if (hitcount < hits + 3)
      {
        digitalWrite(_pin, ((hitcount++)<hits)?HIGH:LOW);
      }
      else
      {
        hitcount = 0;
        hits++;
        lhits = hits;
      }
    }
    else
    {
      int state = digitalRead(_pin);
      if (state == 0)
      {
        hitcount++;
      }
      else
      {
        hitcount = 0;
        hits++;
      }
      //lhits = hits;
      if (hitcount >= 3)
      {
        lhits = hits;
        hits = 0;
      }
    }
    */
  }

  byte Bitstreamer::Read()
  {
    //Load from incoming byte buffer
    return 0;  
  }

  void Bitstreamer::Write(byte b)
  {
    //Send to outgoing byte buffer
  }

  void Bitstreamer::ReadBits(uint8_t b,  boolean* bitarray)
  {
    //DRAGONHERE: For loop would have been less code, but required initialization of another variable, and slightly more complicated logic for the processor
    //Also, not having it in a for loop makes it easier for others to understand
    bitarray[0] = (b & 1) != 0;    // 1 = 00000001, IE 1st bit. (1 & b) lets only 1st bit through. != 0 checks if zero.
    bitarray[1] = (b & 2) != 0;    // 1 = 00000010, IE 2nd bit. (2 & b) lets only 2nd bit through. != 0 checks if zero.
    bitarray[2] = (b & 4) != 0;    // 1 = 00000100, IE 3rd bit. (4 & b) lets only 3rd bit through. != 0 checks if zero.
    bitarray[3] = (b & 8) != 0;    // 1 = 00001000, IE 4th bit. (8 & b) lets only 4th bit through. != 0 checks if zero.
    bitarray[4] = (b & 16) != 0;   // 1 = 00010000, IE 5th bit. (16 & b) lets only 5th bit through. != 0 checks if zero.
    bitarray[5] = (b & 32) != 0;   // 1 = 00100000, IE 6th bit. (32 & b) lets only 6th bit through. != 0 checks if zero.
    bitarray[6] = (b & 64) != 0;   // 1 = 01000000, IE 7th bit. (64 & b) lets only 7th bit through. != 0 checks if zero.
    bitarray[7] = (b & 128) != 0;  // 1 = 10000000, IE 8th bit. (128 & b) lets only 8th bit through. != 0 checks if zero.

    //PLEASE NOTE: These explainations are based on the assumption that the bit order on Arduino is MSB-Left.
    //Notably, this method of exploding bits will actually dynamically adjust to the bit order of the given system.
  }

 uint8_t Bitstreamer::WriteBits(boolean* b)
 {
    //DRAGONHERE: same as above
    return
    (b[0]? 1 : 0) +
    (b[1]? 2 : 0) +
    (b[2]? 4 : 0) +
    (b[3]? 8 : 0) +
    (b[4]? 16 : 0) +
    (b[5]? 32 : 0) +
    (b[6]? 64 : 0) +
    (b[7]? 128 : 0);
}

fails to compile and gives the error:

/Applications/Arduino.app/Contents/Resources/Java/hardware/tools/avr/bin/../lib/gcc/avr/4.3.2/../../../../avr/lib/avr6/crtm2560.o: In function __vector_default': (.vectors+0x90): relocation truncated to fit: R_AVR_13_PCREL against symbol__vector_36' defined in .text.__vector_36 section in core.a(HardwareSerial.cpp.o)

The difference? You'll laugh...

In the working piece of code, I have:

Line #98: if (bitcounter == 0)
Line #106: bitcounter = -1;

In the not-working piece of code, I have:

Line #98: if (bitcounter == 1)
Line #106: bitcounter = 0;

Those are literally the only two differences. I, quite frankly, do not understand why in the name of goodness this does not work. What's even better is that:

Line #98: if (bitcounter == 0)
Line #106: bitcounter = 0;

And:

Line #98: if (bitcounter == 1)
Line #106: bitcounter = -1;

And even:

Line #98: if (bitcounter == 0)
Line #106: bitcounter = 1;

all work. It is the unique combination of 1, and 0, in that order, that causes my compiler to go berserk.

Is someone able to explain what the heck is going wrong here? I am seriously disturbed by this, and I can think of only one reason for this to happen -- Maybe my program is using up too much memory on the arduino? Nope. The original working code takes up, and I quote:

Binary sketch size: 10,178 bytes (of a 258,048 byte maximum)

I'm sorry, but I just don't see how changing two constants in my program can increase the final binary size by a total of 247,870 bytes.

So, what on earth am I doing wrong? Have I stumbled upon some insane bug in the compiler? Is this the result of my code somehow compiling to the same binary pattern as some poorly implemented developer-signal that tweaks the settings on the program-upload-receiver system on the Arduino? Or is there a much more logical answer to this mystery?

score 1 · Answer 1 · edited May 23 '17 at 11:50

1

In a different Arduino question, I saw this kind of blowup happen due to inlining. I'm not certain that that's the sole culprit here, though. (If you think it might be, see this link for the syntax to disable inlining on a function: How can I tell gcc not to inline a function? )

More likely, you're exceeding the relative branch displacement for a conditional branch. You have a huge nest of if-else clauses. You might try breaking those apart or otherwise flattening the hierarchy there. For example, move each of your top-level clauses (stage 1, stage 2, ...) to private methods, and invoke those methods from the body of the if. Perhaps mark those noinline as well.

Why would changing two constants push you over the line? I don't know the vagaries of the underlying assembly language, but often some constants are cheaper to generate or test against than others in terms of codesize. Quite commonly, it's much cheaper work with 0 than any other value, for example.

The following SO question provides much greater detail about relative calls on the AVR platform, as well as a suggested solution: AVR linker error, "relocation truncated to fit"

edited May 23 '17 at 11:50

Community

1
1

answered Dec 24 '13 at 08:05

Joe Z

17,413
3
28
39

Interesting! I am on the fence about the branch displacement theory. Why? If I set line 98 to "bitcounter == 1" and line 106 to "bitcounter = 0;", the compilation fails. But if I leave line 98 alone, and change line 106 to "bitcounter = -1;" then it compiles just fine. In this case, I'm not changing anything about the logic structure of the program. Still, it's good to know that that can become a potential problem! I'll try moving my top-level clauses to separate functions, and let you know how it goes. – Georges Oates Larsen Dec 24 '13 at 08:14
1

@GeorgesOatesLarsen : BTW, this SO question gives much more information about relative vs. long jumps and calls on the AVR, and is directly relevant: http://stackoverflow.com/questions/8188849/avr-linker-error-relocation-truncated-to-fit I will add the link above. – Joe Z Dec 24 '13 at 08:15
@GeorgesOatesLarsen : I guess I should add that linker behavior matters too. Even if the nested-ifs are fine among themselves, the linker will reorder hunks of code based on their size and other factors, and so a slight change in codesize in one function can cause two unrelated functions to be "too far apart". I've hit this many times in other tool chains; it's not unique to Arduino or AVR or even the GNU toolchain. – Joe Z Dec 24 '13 at 08:19
Joe Z ahh, that seems like it might be what's happening. Though why would "bitcounter = 0;" have a different size than "bitcounter = -1;"? – Georges Oates Larsen Dec 24 '13 at 08:24
@GeorgesOatesLarsen: I am not an AVR expert. Looking at the assembly language, though, I see a 1 word instruction to clear a register, and one to set it to all 1s. The only difference seems to be the flags affected. Without seeing the generated code, I can only speculate as to what other effects the change might have had on the code G++ generated. (It does do constant propagation, for example.) The only way to know for sure is to look at the assembly output for both versions and compare. – Joe Z Dec 24 '13 at 08:34
I see! I see. Well I'm trying to use the -mshort-calls command line argument as the link you posted suggests -- but I, sadly, have no idea how to do that with the arduino.app sketch editor. Do you have any suggestions? – Georges Oates Larsen Dec 24 '13 at 08:43
1

@GeorgesOatesLarsen : Actually, you want to do the opposite: Disable that flag. The link I gave above says: _The relevant option is **"Use rjmp/rcall (limited range) on >8k devices (-mshort-calls)"** which needs to be **unchecked** to prevent the named error._ – Joe Z Dec 24 '13 at 08:53

Arduino compilation weirdness

1 Answers1