Re: [avr-libc-dev] [RFC] catching misspeeled signal names

[Theodore A. Roth]

On Tue, 20 Apr 2004, Joerg Wunsch wrote:

> As Dmitry K. wrote:
>
> > MOST OF INTERRUPT SERVICE ROUTINES ARE ASSEMBLER ONES.

Well, I didn't catch the asm breakage since all my interrupt handlers to
date have been pure C (not that that won't change in the future in
special cases).

>
> While I seriously doubt that statement ;-), I agree that asm must not
> be broken.  However, that should not be too hard.  In case
> __ASSEMBLER__ is set, just revert the macros to the old behaviour.
> This doesn't get spelling checks in assembler code, but we didn't used
> to have anything before, so it's better than nothing.

It's not that simple. I changed the patch to do this:

+#if defined (__ASSEMBLER__)
+#  define _SIG_VECTOR(sig_num, attr) _VECTOR(sig_num)
+#else
+#  ifdef __cplusplus
+#    define _SIG_VECTOR(sig_num, attr)                \
+extern "C" void _VECTOR(sig_num) (void);              \
+void _VECTOR(sig_num) (void) __attribute__ ((attr));  \
+void _VECTOR(sig_num) (void)
+#  else
+#    define _SIG_VECTOR(sig_num, attr)                \
+void _VECTOR(sig_num) (void) __attribute__ ((attr));  \
+void _VECTOR(sig_num) (void)
+#  endif
+#endif
+

And it still breaks assembler handlers. The problem is that I added args
to the signal names in the io*.h and defining a handler like this:

  .global SIG_INTERRUPT0
  SIG_INTERRUPT0:
        reti

The preprocessor seems to ignore SIG_INTERRUPT0 because it doesn't have
the arg list with parens. Dmitry noted this, but I had to play with it
to understand what he was taking about. My updated patch does work it
you change the handler to this:

  .global SIG_INTERRUPT0(foo,bar)
  SIG_INTERRUPT0(foo,bar):
        reti

Now that's _really_ ugly. So I reworked <avr/signal.h> so that you can
do this in an asm file (irq.S in my test case):

  #include <avr/io.h>
  #include <avr/signal.h>

  SIGNAL(fooooooo_SIG_INTERRUPT0)
        reti

Which when assembling, yeilds this error:

  $ avr-gcc -Wall -x assembler-with-cpp -mmcu=atmega128 -save-temps -c -o irq.o 
irq.S
  irq.S: Assembler messages:
  irq.S:4: Error: too many positional arguments

The temp file, irq.s now shows this:

  # 34 
"/home/roth/local/avr-20040322/bin/../lib/gcc/avr/3.4.0/../../../../avr/include/avr/signal.h"
 3
  .macro _ASM_SIGNAL signame
  .global \signame
  \signame:
  .endm
  # 3 "irq.S" 2

  _ASM_SIGNAL fooooooo_SIG_INTERRUPT0(("<- Misspelled SIG name"), signal)
   reti

If the sig name is spelled correctly, you end up with the __vector_N in
the final elf as expected.

>
> The hack is really ugly :), but I've been thinking about the issue
> myself, and so far didn't get any better idea.  I'm all for it since
> misspelled vector names are one of the major sources of errors that
> often remain unnoticed.

Ok. How's the attached revised patch look then? Sorry to make it a
little more ugly and this one requires asm handlers to be changed (which
I'm not convinced it the best route).

>
> The only idea that occurred to me was: how about putting all interrupt
> vectors into a section of its own, and then have the linker make check
> that all global symbols arriving in that section are of the form
> _vector_N?  I assume something like that can be had in the linker.

That still doesn't catch mispelling the SIG_* name since the compiler
will still generate a valid C function. I don't see any way around that.

>
> The major drawback of this besides the requirement to also modify
> binutils is that AVR Studio can only handle the standard
> .text/.data/.bss sections (at least in COFF files, I don't know about
> their ELF effort), so it would not load the interrupt vectors.
> Perhaps either the linker or the avr-objcopy performing the COFF
> conversion could coerce .text + the vector section into a final .text
> section though.

I don't have anything positive to say about that, so I'll refrain. ;-)

---
Ted Roth
PGP Key ID: 0x18F846E9
Jabber ID: address@hidden