array/pointer/bit-shifting bare avr-gcc problem

Forum 2005-2010 (read only) Software Syntax & Programs
1

I've written some simple code that works if I compile it using the IDE. Trying to understand more of the process I looked at the verbose build log and used these commands to compile the code myself. As I strictly used non-arduino functions I didn't link against core.a.

The problem:

for(ctr=0; etc. etc.) {
  PORTB = 0xFF;
  PORTB &= ~( (1<<ctr) );
}

works just fine and the LEDs light up. For sake of simplicity I've left out the _delay_ms().

Now as I've got a small bug on my PCB, I need to remap 2 LEDs to make the sequence correct. Thus:

uint8_t fix_led_numbering[8] = {0,1,2,3,4,6,5,7};

for(ctr=0; etc. etc.) {
  PORTB = 0xFF;
  PORTB &= ~( (1<<fix_led_numbering[ctr]) );
}

This doesn't work if compiled by hand. I've verified that the array actually exists. It shows up in the disassembled elf file just fine.

Behaviour:

using no arrays: LEDs light up one after another, but wrong sequence (PCB bug).

with array: just 3 LEDs light up in a strange order. I have a feeling that it doesn't use the contents of the array, but maybe the pointer address (wild guess).

I've looked at the assembler code, but I'm not skilled enough to spot the error.

Here's the code + disassembled elf

http://pastebin.com/m71dbb5a5

It was compiled with this set of commands:

avr-g++ -c -g -Os -w -fno-exceptions -ffunction-sections -fdata-sections $1 -o $1.o
avr-gcc -Os -Wl,--gc-sections -o $1.elf $1.o -lm
avr-objcopy -O ihex -R .eeprom $1.elf $1.hex
avr-size $1.hex

I really don't understand why using an array confuses the little cpu that much.

2

Any difference if the array is a const (as I assume it should be)?

3

No, same thing.

4

Code like this is somewhat dangerous:

 PORTB = 0xFF;
 PORTB &= ~( (1<<ctr) );

The issue here is that PORTB is directlty mapped to digitial output pins and depending on the direction of output (DDRB) you will either enable pullup or set output high for all PORTB pins with the PORTB=0xFF statement. PORTB includes crystal pins as well and this is perhaps not what you intended.

5

It is absolutely what I intend :slight_smile:

This code doesn't run on any arduino board, I have a custom PCB. Here's the schematic. And the chip runs on internal oscillator. It's impossible to short anything, as PORTB is connected to 8 RGB LEDs (common cathodes) and the corresponding resistors are there too.

What completely dumbfounds me is the fact that it works just fine if compiled with the IDE. Using the very same commands by hand... well I've already said that it doesn't work as soon as I use the array. My understanding of C is that using arrays is such a fundamental thing that it doesn't require inclusion of any type of library. Basically it should just work.

I really want to understand what is going on.

6

There is definitely something strange with this:

00000026 <__do_copy_data>:
  26:   10 e0           ldi     r17, 0x00       ; 0
  28:   a0 e6           ldi     r26, 0x60       ; 96
  2a:   b0 e0           ldi     r27, 0x00       ; 0
  2c:   e8 e6           ldi     r30, 0x68       ; 104
  2e:   f4 e0           ldi     r31, 0x04       ; 4
  30:   03 c0           rjmp    .+6             ; 0x38 <__do_copy_data+0x12>
  32:   c8 95           lpm
  34:   31 96           adiw    r30, 0x01       ; 1
  36:   0d 92           st      X+, r0
  38:   a8 36           cpi     r26, 0x68       ; 104
  3a:   b1 07           cpc     r27, r17
  3c:   d1 f7           brne    .-12            ; 0x32 <__do_copy_data+0xc

versus:

00000074 <__do_copy_data>:
  74:   11 e0           ldi     r17, 0x01       ; 1
  76:   a0 e0           ldi     r26, 0x00       ; 0
  78:   b1 e0           ldi     r27, 0x01       ; 1
  7a:   ec e4           ldi     r30, 0x4C       ; 76
  7c:   f2 e0           ldi     r31, 0x02       ; 2
  7e:   02 c0           rjmp    .+4             ; 0x84 <__do_copy_data+0x10>
  80:   05 90           lpm     r0, Z+
  82:   0d 92           st      X+, r0
  84:   a8 30           cpi     r26, 0x08       ; 8
  86:   b1 07           cpc     r27, r17
  88:   d9 f7           brne    .-10            ; 0x80 <__do_copy_data+0xc>

The bottom one is created by the IDE (works), the top one by "hand" (doesn't work). As far as I understand, it should copy the data (array of length 8) into RAM using the Z register in both cases.

In the code that shifts the one according to the array data, the address used for the array matches 0x01,0x00 or 0x00,0x96 respectively. So if I'm not mistaken it looks at the right address in ram and the bit-shifting code looks identical to me as well. I only don't know if this "lpm" does work without any arguments. So it could be that the array data contains crap.

7

Looking at your schematic, the code makes sense. If you have an issue with the PCB, one possibilitity might be that the board resets (and perhaps not consistenly) in which case it is difficult to trace the output.

The following is the makefile I use for a pure AVR single source (sonar.cpp) project file. You may want to check this against your own command lines if you're running out of ideas.

##################################
## Makefile for project: Sonar
##################################

## General Flags
PROJECT = Sonar
MCU = atmega328p
TARGET = output/$(PROJECT).elf
CC = avr-gcc
CCXX = avr-g++

## Flags common to C, ASM, and Linker
COMMON = -mmcu=$(MCU)

## Flags common to C only
CFLAGS = $(COMMON)
CONLYFLAGS = -std=gnu99
CFLAGS += -Wall -gdwarf-2 -std=gnu99-DF_CPU=8000000UL -Os -funsigned-bitfields -fpack-struct -fshort-enums -ffunction-sections -fdata-sections
CFLAGS += -MD -MP -MT $(*F).o

## Flags common to ASM only
ASMFLAGS = $(COMMON)
ASMFLAGS += $(CFLAGS)
ASMFLAGS += -x assembler-with-cpp -Wa,-gdwarf2
ASMFLAGS += 

## Flags common to CPP/CXX only
CXXFLAGS = $(COMMON)
CXXFLAGS += $(CFLAGS)
CXXFLAGS += -std=c99

## Flags common to Linker only
LDFLAGS = $(COMMON)
LDFLAGS += -Wl,-Map=output/$(PROJECT).map
LDFLAGS += -Wl,--gc-sections

## Flags for Intel HEX file production
HEX_FLASH_FLAGS = -R .eeprom -R .fuse -R .lock -R .signature

HEX_EEPROM_FLAGS = -j .eeprom
HEX_EEPROM_FLAGS += --set-section-flags=.eeprom="alloc,load"
HEX_EEPROM_FLAGS += --change-section-lma .eeprom=0 --no-change-warnings

## Link these object files to be made
OBJECTS = Sonar.o

## Link objects specified by users
LINKONLYOBJECTS = 

## Compile

all: $(TARGET)

Sonar.o: ./Sonar.cpp
       $(CCXX) $(INCLUDES) $(CXXFLAGS) -c  $<



## Link
$(TARGET): $(OBJECTS)
      -rm -rf $(TARGET) output/$(PROJECT).map
       $(CC) $(LDFLAGS) $(OBJECTS) $(LINKONLYOBJECTS) $(LIBDIRS) $(LIBS) -o $(TARGET)
      -rm -rf $(OBJECTS) Sonar.d 
      -rm -rf output/$(PROJECT).hex output/$(PROJECT).eep output/$(PROJECT).lss
      avr-objcopy -O ihex $(HEX_FLASH_FLAGS) $(TARGET) output/$(PROJECT).hex
      avr-objcopy $(HEX_FLASH_FLAGS) -O ihex $(TARGET) output/$(PROJECT).eep || exit 0
      avr-objdump -h -S $(TARGET) >> output/$(PROJECT).lss
      @avr-size -C --mcu=${MCU} ${TARGET}

## Clean target
.PHONY: clean
clean:
      -rm -rf $(OBJECTS) Sonar.d  output/$(PROJECT).elf output/$(PROJECT).map output/$(PROJECT).lss output/$(PROJECT).hex output/$(PROJECT).eep
8

Thanks!

It compiles with your Makefile and it works too ;D

Only the code is about 2x as big as with the IDE. That is not really an issue with an ATmega168 and just a few LEDs though. Still very strange. Garbage collect is in there too... hmmm.

9

The code size issue was caused by using _delay_ms() with non constants. This seems to pull in some quite large chunk of code.

This wrapper here is a workaround:

void __delay_ms(uint16_t delay_time) {
  uint16_t counter;
  for(counter = 0; counter < delay_time; counter++) {
    _delay_ms(1);
}
10 
32:   c8 95           lpm

vs
80:   05 90           lpm     r0, Z+


I only don't know if this "lpm" does work without any arguments.

Ah hah! A significant clue... Looking at the AVR instruction set reference (NOT the "summary" in each data sheet), I see:

Not all variants of the LPM instruction are available in all devices. Refer to the device specific instruction set summary.

And then we have

avr-g++ -c -g -Os -w -fno-exceptions -ffunction-sections -fdata-sections $1 -o $1.o
avr-gcc -Os -Wl,--gc-sections -o $1.elf $1.o -lm
avr-objcopy -O ihex -R .eeprom $1.elf $1.hex
avr-size $1.hex

Where is your compiler option that tells the compiler which cpu you are compiling for? It should look like -mmcu=atmega328p ?? I think your manual build is compiling for some default CPU that doesn't match your hardware!

The code size issue was caused by using _delay_ms() with non constants. This seems to pull in some quite large chunk of code.

The _delay_ms macro uses floating point, and REALLY counts on a combination of "use constants only" and compiler optimization to make sure that nothing of floating point is left by the time the final code is generated. Otherwise your code bloats because of inclusion of the Floating point libraries AND the delays get wildly inaccurate because the timing of the FP calculations is NOT included in the generated code.

11

I have (and had) these defines in the code:

#define F_CPU 8000000UL
#define AVR_ATmega168

Apparently that doesn't do the job. It only seems to pull in the right include files. But the compiler still doesn't know about mcu type.

Well, the datasheet for the ATmega168 lists many forms of LPM. If not there, where else should it say it doesn't work.

But most importantly, it compiles correctly when explicitly using -mmcu=<whaterver_you_have>. I might have guessed :wink:

12

Apparently [#define correct_cpu]doesn't do the job. It only seems to pull in the right include files. But the compiler still doesn't know about mcu type.

Exactly correct.

The datasheet for the ATmega168 lists many forms of LPM. If not there, where else should it say it doesn't work.

I don't claim that I recognized the correct or incorrect form of LPM as the exact cause of your problems; just that the fact that they were apparently different in working vs non-working binaries was a pointer to the compiler thinking the CPU type was different for the two cases. No smoking gun, but perhaps the lingering smell of cordite in the air :slight_smile:

13

I'm humbled and from now on I shall walk with my head bowed down.