Re: Bug in avr-gdb? was:Re: [avr-gcc-list] AVR simulator and interrupts

[Klaus Rudolph]

Hi Theodore, hi James and the list :-)

Theodore A. Roth schrieb:

> On Fri, 16 Jul 2004, Klaus Rudolph wrote:
>
>  
>
> > Hi James,
> >
> > I could rproduce the problem also with actual simulavr.
> > After traceing through the gdb output I could found
> > a senseless continue command which is send from gdb to simulator.
> > The result is endless running of simulavr which is exact what the
> > gdb wants but what is not what the user want from gdb :-)
> >
> >
> > I think we have a bug in gdb for avr.
> >    
>
> I don't think this is a bug in gdb and it's not something that can be
> easily worked around in gdb.
>
> The problem is gdb doesn't have enough information to step through the
> "__vectors" function. In reality, __vectors is not a function at all,
> but a jump table. The assembler marks it as a function because of this
> (from gcrt1.S):
>
>        .section .vectors,"ax",@progbits
>        .global __vectors
>        .func   __vectors
> __vectors:
>
> I think it's done that way to allow locating the .vectors section at
> flash addr 0x00 (someone correct me if I'm wrong). This confuses gdb
> since it thinks it's looking at a function expecting it to return at
> some point. Instead, we jmp/rjmp out of __vectors directly in the
> interrupt handler.
>  
You are right if you say that we could not find correct line numbers and 
so we are not able to
do a high level language step because we could not calculate the pc for 
the next line number.
But the gdb print a message that he will do single stepping until 
another line number is reached
but the gdb do a continue! That is the bug here.

I fixed that in file:
gdb-6.1.1/gdb/infcmd.c

   629   if (!single_inst)
   630         {
   631           find_pc_line_pc_range (stop_pc, &step_range_start, 
&step_range_end);
   632           if (step_range_end == 0)
   633         {
   634           char *name;
   635           if (find_pc_partial_function (stop_pc, &name, 
&step_range_start,
   636                         &step_range_end) == 0)
   637             error ("Cannot find bounds of current function");
   638
   639           target_terminal_ours ();
   640           printf_filtered ("\
   641 Single stepping until exit from function %s, \n\
   642 which has no line number information.\n", name);
   643           step_range_start = step_range_end = 1;               
<<<<<<<<<<<
Here I simly add that functionality which the gdb speak over :-)
Let step_range_start & step_range_end = 1 and we will do a "stepi". That
is what I understood from the message which gdb prints and now
gdb do exactly that, a stepi.

My problem is solved for that special problem and I think that this a 
bug in gdb.
If we want a stepi here we must set the vars to stepi :-)


   644         }
   645         }
   646       else
   647         {
   648           /* Say we are stepping, but stop after one insn 
whatever it does.  */
   649           step_range_start = step_range_end = 1;
   650           if (!skip_subroutines)
   651         /* It is stepi.
   652            Don't step over function calls, not even to functions 
lacking
   653            line numbers.  */
   654         step_over_calls = STEP_OVER_NONE;
   655         }



> I just hacked simulavr-0.1.2.1 to automatically perform an extra step
> when it vectors into the __vectors table (see attached patch). This
> makes the 'step' command step right into the interrupt when it fires off
> which is what I would expect the user to expect. This is similar in idea
> to avarice's --ignore-intr option.
>
> There's still a problem with the 'next' command. In my modified example
> program (see test.c attached), if I tell gdb to 'next' when it's at line
> 28 in this code,
>
>     25     while (1)
>     26     {
>     27         /* Chew up some MCU cycles. */
>     28         clk += TCNT0;
>     29         //dummy -= TCNT0;
>     30     }
>
> it goes into an infinite loop of stepping (but not jumping into
> __vectors). This falls under the "don't expect gdb to do the right thing
> if you compile with optimizations"  category. A simple work around in
> this case is to just set a breakpoint at line 28.
>
> If I uncomment line 29, then 'next' works fine and I never jump int
> __vectors or the interrupt.
>
>  
Your workaround seems not to solve the problem totally, because there is 
not ever a
reti after a irq handler. And I think that we have also a break if we 
want a "cont" in the __vectors.
That is not what I expect, but I could be wrong here because I have not 
tested that  :-)

Is there a problem with my solution, maybe with other targets as well?
If not it would be nice if this fix could be included to gdb.

Thanks
   Klaus






> ---
> Ted Roth
> PGP Key ID: 0x18F846E9
> Jabber ID: address@hidden
>
> ------------------------------------------------------------------------
>
> #include <inttypes.h>
> #include <avr/io.h>
> #include <avr/signal.h>
> #include <avr/interrupt.h>
> #include <avr/eeprom.h>
>
> volatile char flag;
> char temp;
> volatile uint8_t clk;
> volatile uint8_t dummy;
>
> int
> main (void)
> {
>    clk = TCNT0 = 0;
>  
>    /* Enable the Timer0 overflow interrupt. */
>    TIMSK = _BV (TOIE0);
>
>    /* Start the timer/counter running at the CPU clock rate. */
>    TCCR0 = _BV (CS00);
>    
>    sei ();
>  
>    while (1)
>    {
>        /* Chew up some MCU cycles. */
>        clk += TCNT0;
>        //dummy -= TCNT0;
>    }
>
>    return 0;
> }
>
>
> SIGNAL(SIG_OVERFLOW0)
> {
>         flag = ~flag;
> }
>
>  
>
> ------------------------------------------------------------------------
>
> diff -ur simulavr-0.1.2.1/src/avrcore.c simulavr-0.1.2.1-step-vect/src/avrcore.c
> --- simulavr-0.1.2.1/src/avrcore.c      2004-01-01 19:01:01.000000000 -0800
> +++ simulavr-0.1.2.1-step-vect/src/avrcore.c    2004-07-16 12:23:48.000000000 
> -0700
> @@ -926,10 +926,11 @@
>  * Ted, what do you think we should do?
>  */
>
> -static void
> +static int
> avr_core_check_interrupts (AvrCore *core)
> {
>     IntVect *irq;
> +    int res = 0;
>
>     if (core->irq_pending)
>     {
> @@ -966,9 +967,13 @@
>                 avr_core_PC_set (core, irq->addr + core->irq_offset);
>
>                 avr_core_irq_clear (core, irq);
> +
> +                res = 1;
>             }
>         }
>     }
> +
> +    return res;
> }
>
> /**
> @@ -983,6 +988,8 @@
>     int res = 0;
>     int state;
>
> + STEP_AGAIN:
> +
>     /* The MCU is stopped when in one of the many sleep modes */
>     state = avr_core_get_state (core);
>     if (state != STATE_SLEEP)
> @@ -1012,7 +1019,13 @@
>        need to delay handling of any pending IRQs until after the next
>        instruction is executed. */
>     if (res != opcode_RETI)
> -        avr_core_check_interrupts (core);
> +    {
> +        /* If we vectored into the vector jump table, step again so we get out
> +           of the vector table to avoid confusing gdb. */
> +
> +        if (avr_core_check_interrupts (core) > 0)
> +            goto STEP_AGAIN;
> +    }
>
>     return res;
> }
>