Re: [avr-libc-dev] [Patch] Include a device info note section in gcrt1.S

[Senthil Kumar Selvaraj]

On Sat, Oct 11, 2014 at 10:35:22AM +0200, Joerg Wunsch wrote:
> I finally had a chance to test all this.
> 
> For reference, here's the result for an ATmega1281:
> 
> % avr-objdump -h -j .note.gnu.avr.deviceinfo -s 
> avr/lib/avr5/atmega1281/crtm1281.o
> 
> avr/lib/avr5/atmega1281/crtm1281.o:     file format elf32-avr
> 
> Sections:
> Idx Name          Size      VMA       LMA       File off  Algn
>  11 .note.gnu.avr.deviceinfo 00000034  00000000  00000000  0000181c  2**2
>                   CONTENTS, READONLY
> Contents of section .note.gnu.avr.deviceinfo:
>  0000 04000000 23000000 01000000 41565200  ....#.......AVR.
>  0010 00000000 00000200 00020000 00200000  ............. ..
>  0020 00000000 00100000 08000000 01000000  ................
>  0030 00000000                             ....            
> 
> 
> As Senthil Kumar Selvaraj wrote:
> 
> > How does the below patch look? Once you guys find it ok, I'll follow it
> > up with a documentation patch. Note that the part above the
> > L__desc_start label (and the .balign directive at the end) are there as
> > per the ELF spec for a note section.
> 
> OK, if that's required, we have to stick with that.
> 
> Wouldn't the note name string also possibly need padding
> for alignment behind it?  (In the case of the "AVR\0",
> it only incidentally aligns the following .long values
> to a long boundary.)

Yes, I figured it's only going to be "AVR\0", so I skipped the align
directive. I'll add it.
> 
> For the documentation, please include a C struct definition
> as explanation:
> 
> #define __NOTE_NAME_LEN 4
> struct __note_gnu_avr_deviceinfo
> {
>     struct
>     {
>         uint32_t note_name_len;          /* = __NOTE_NAME_LEN */
>         uint32_t desc_len;
>         char note_name[__NOTE_NAME_LEN]; /* = "AVR\0" */
>     }
>     elfinfo_hdr;
>     struct
>     {
>         uint32_t flash_start;
>         uint32_t flash_size;
>         uint32_t sram_start;
>         uint32_t sram_size;
>         uint32_t eeprom_start;
>         uint32_t eeprom_size;
>         uint32_t offset_table[2];  /* index 0: ... */
>         // ???
>     }
>     avr_desc;
> };

Ok, will do.
> 
> > +.L__stroffsettab_end:
> > +    /* String table for storing arbitrary strings.
> > +    Offsets are stored in the string offset table above */
> > +.L__strtab_start:
> > +    .byte 0
> > +.L__device_name_start:
> > +    .asciz DEVICE_NAME
> > +.L__device_name_end:
> > +    .byte 0
> > +.L__strtab_end:
> 
> I don't quite understand the purpose of the two ".byte 0" definitions.

I used the standard ELF string table convention - the first and last
bytes are null. From http://man7.org/linux/man-pages/man5/elf.5.html,

"String table sections hold null-terminated character sequences,
commonly called strings.  The object file uses these strings to
represent symbol and section names.  One references a string as an
index into the string table section.  The first byte, which is index
zero, is defined to hold a null byte ('\0').  Similarly, a string
table's last byte is defined to hold a null byte, ensuring null
termination for all strings."

There's no need to follow that convention though - the descriptor's
layout/interpretation can be arbitrary. If you think it is redundant,
I'll drop it.

I'll send a new patch with all the changes.

Regards
Senthil