Re: [Patch, tentative, AVR] Displaying per-device memory usage info

[Senthil Kumar Selvaraj <senthil_kumar dot selvaraj at atmel dot com>]

Ping!

binutils/ChangeLog

2014-11-30  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	* configure.ac: Add od-elf32_avr to build.
	* configure: Regenerate.
	* od-elf32_avr.c: New file.
	* objdump.h: Declare objdump_private_desc_elf32_avr.

Regards
Senthil
On Tue, Nov 25, 2014 at 11:36:25AM +0530, Senthil Kumar Selvaraj wrote:
> On Mon, Nov 24, 2014 at 06:04:52PM +0000, Andrew Burgess wrote:
> > I can't give you commit approval.  I spotted a few minor coding
> > standard issues while taking a look at this patch, comments inline
> > below.
> > 
> > Thanks,
> > Andrew
> > 
> > * Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> [2014-11-17 18:47:56 +0530]:
> > 
> > > diff --git binutils/od-elf32_avr.c binutils/od-elf32_avr.c
> > > new file mode 100644
> > > index 0000000..f74d602
> > > --- /dev/null
> > > +++ binutils/od-elf32_avr.c
> > > @@ -0,0 +1,243 @@
> > > +/* od-avrelf.c -- dump information about an xcoff object file.
> > 
> > That header line seems wrong.
> 
> Fixed - copy paste error :).
> > 
> > > +static char * elf32_avr_get_note_section_contents (bfd *abfd, bfd_size_type *size)
> > 
> > Should have newline after "static char *".
> 
> Done.
> > 
> > > +
> > > +static char* elf32_avr_get_note_desc (bfd *abfd, char *contents,
> > > +        bfd_size_type size)
> > 
> > Should be "static char *" then newline.
> 
> Done.
> > 
> > > +static void
> > > +elf32_avr_get_device_info (bfd *abfd, char *description,
> > > +        deviceinfo *device)
> > > +{
> > > +  if (description == NULL)
> > > +    return;
> > > +
> > > +  const bfd_size_type memory_sizes = 6;
> > > +
> > > +  memcpy (device, description, memory_sizes * sizeof(uint32_t));
> > 
> > Whitespace after sizeof.
> 
> Done.
> > 
> > > +  device->name = NULL;
> > > +
> > > +  uint32_t *stroffset_table = ((uint32_t *)description) + memory_sizes;
> > 
> > Should have whitespace after cast.
> 
> Done.
> > 
> > > +  bfd_size_type stroffset_table_size = bfd_get_32 (abfd, stroffset_table);
> > > +  char *str_table = ((char *)stroffset_table) + stroffset_table_size;
> > 
> > Whitespace after cast.
> 
> Done.
> > 
> > > +static void
> > > +elf32_avr_dump (bfd *abfd)
> > > +{
> > > +  char *description = NULL;
> > > +  bfd_size_type note_section_size = 0;
> > > +
> > > +  deviceinfo device = {0};
> > > +
> > > +  bfd_size_type data_usage = 0;
> > > +  bfd_size_type text_usage = 0;
> > > +  bfd_size_type eeprom_usage = 0;
> > > +
> > > +  if (!options[OPT_MEMUSAGE].selected)
> > > +    return;
> > 
> > In order to allow for future code growth it might be nice to move the
> > core of this function out, into elf32_avr_dump_mem_usage, then write:
> > 
> >     if (options[OPT_MEMUSAGE].selected)
> >     	elf32_avr_dump_mem_usage (abfd);
> > 
> > this way if/when more options are added the code changes required are
> > reduced. What do you think?
> 
> Yes, that looks better. Fixed.
> > 
> > > +
> > > +  device.name = "Unknown";
> > > +
> > > +  char *contents = elf32_avr_get_note_section_contents (abfd,
> > > +    &note_section_size);
> > > +
> > > +  if (contents != NULL)
> > > +    {
> > > +      description = elf32_avr_get_note_desc (abfd, contents, note_section_size);
> > > +      elf32_avr_get_device_info (abfd, description, &device);
> > > +    }
> > > +
> > > +  elf32_avr_get_memory_usage (abfd, &text_usage, &data_usage,
> > > +     &eeprom_usage);
> > > +
> > > +  printf ("AVR Memory Usage\n"
> > > +          "----------------\n"
> > > +          "Device: %s\n\n", device.name);
> > > +
> > > +  /* Text size */
> > > +  printf ("Program:%8ld bytes", text_usage);
> > > +  if (device.flash_size > 0)
> > > +    printf (" (%2.1f%% Full)", ((float)text_usage / device.flash_size) * 100);
> > 
> > Whitespace after cast.
> 
> Done.
> > 
> > > +
> > > +  printf ("\n(.text + .data + .bootloader)\n\n");
> > > +
> > > +  /* Data size */
> > > +  printf ("Data:   %8ld bytes", data_usage);
> > > +  if (device.ram_size > 0)
> > > +  {
> > > +    printf (" (%2.1f%% Full)", ((float)data_usage / device.ram_size) * 100);
> > 
> > Whitespace after cast.
> 
> Done.
> > 
> > > +  }
> > > +  printf ("\n(.data + .bss + .noinit)\n\n");
> > > +
> > > +  /* EEPROM size */
> > > +  if (device.eeprom_size > 0)
> > > +    {
> > > +      printf ("EEPROM: %8ld bytes", eeprom_usage);
> > > +      printf (" (%2.1f%% Full)", ((float)eeprom_usage / device.eeprom_size) * 100);
> > 
> > Whitespace after cast.
> 
> And done.
> > 
> > > +      printf ("\n(.eeprom)\n\n");
> > > +    }
> > > +
> > > +  if (contents != NULL)
> > > +    free (contents);
> > > +}
> 
> 
> diff --git binutils/configure binutils/configure
> index 39b8254..78ad874 100755
> --- binutils/configure
> +++ binutils/configure
> @@ -14218,6 +14218,9 @@ do
>  
>  	# Add objdump private vectors.
>  	case $targ in
> +	avr-*-*)
> +	  od_vectors="$od_vectors objdump_private_desc_elf32_avr"
> +	  ;;
>  	powerpc-*-aix*)
>  	  od_vectors="$od_vectors objdump_private_desc_xcoff"
>  	  ;;
> @@ -14238,6 +14241,8 @@ for i in $od_vectors ; do
>  	f="$f $i"
>  	OBJDUMP_PRIVATE_VECTORS="$OBJDUMP_PRIVATE_VECTORS &$i,"
>  	case $i in
> +	objdump_private_desc_elf32_avr)
> +	    od_files="$od_files od-elf32_avr" ;;
>  	objdump_private_desc_xcoff)
>  	    od_files="$od_files od-xcoff" ;;
>  	objdump_private_desc_mach_o)
> diff --git binutils/configure.ac binutils/configure.ac
> index c5aadd8..84c287c 100644
> --- binutils/configure.ac
> +++ binutils/configure.ac
> @@ -411,6 +411,9 @@ changequote([,])dnl
>  
>  	# Add objdump private vectors.
>  	case $targ in
> +	avr-*-*)
> +	  od_vectors="$od_vectors objdump_private_desc_elf32_avr"
> +	  ;;
>  	powerpc-*-aix*)
>  	  od_vectors="$od_vectors objdump_private_desc_xcoff"
>  	  ;;
> @@ -431,6 +434,8 @@ for i in $od_vectors ; do
>  	f="$f $i"
>  	OBJDUMP_PRIVATE_VECTORS="$OBJDUMP_PRIVATE_VECTORS &$i,"
>  	case $i in
> +	objdump_private_desc_elf32_avr)
> +	    od_files="$od_files od-elf32_avr" ;;
>  	objdump_private_desc_xcoff)
>  	    od_files="$od_files od-xcoff" ;;
>  	objdump_private_desc_mach_o)
> diff --git binutils/objdump.h binutils/objdump.h
> index 5ac00e7..3abb002 100644
> --- binutils/objdump.h
> +++ binutils/objdump.h
> @@ -43,6 +43,8 @@ struct objdump_private_desc
>    /* List of options.  Terminated by a NULL name.  */
>    struct objdump_private_option *options;
>  };
> +/* ELF32_AVR specific target.  */
> +extern const struct objdump_private_desc objdump_private_desc_elf32_avr;
>  
>  /* XCOFF specific target.  */
>  extern const struct objdump_private_desc objdump_private_desc_xcoff;
> diff --git binutils/od-elf32_avr.c binutils/od-elf32_avr.c
> new file mode 100644
> index 0000000..7a0a212
> --- /dev/null
> +++ binutils/od-elf32_avr.c
> @@ -0,0 +1,249 @@
> +/* od-avrelf.c -- dump information about an AVR elf object file.
> +   Copyright (C) 2011-2014 Free Software Foundation, Inc.
> +   Written by Senthil Kumar Selvaraj, Atmel.
> +
> +   This file is part of GNU Binutils.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, 51 Franklin Street - Fifth Floor, Boston,
> +   MA 02110-1301, USA.  */
> +
> +#include "sysdep.h"
> +#include <stddef.h>
> +#include <time.h>
> +#include <stdint.h>
> +#include "safe-ctype.h"
> +#include "bfd.h"
> +#include "objdump.h"
> +#include "bucomm.h"
> +#include "bfdlink.h"
> +#include "bfd.h"
> +#include "elf/external.h"
> +#include "elf/internal.h"
> +
> +/* Index of the options in the options[] array.  */
> +#define OPT_MEMUSAGE 0
> +
> +/* List of actions.  */
> +static struct objdump_private_option options[] =
> +  {
> +    { "mem-usage", 0 },
> +    { NULL, 0 }
> +  };
> +
> +/* Display help.  */
> +
> +static void
> +elf32_avr_help (FILE *stream)
> +{
> +  fprintf (stream, _("\
> +For AVR ELF files:\n\
> +  mem-usage   Display memory usage\n\
> +"));
> +}
> +
> +typedef struct tagDeviceInfo
> +{
> +    uint32_t flash_start;
> +    uint32_t flash_size;
> +    uint32_t ram_start;
> +    uint32_t ram_size;
> +    uint32_t eeprom_start;
> +    uint32_t eeprom_size;
> +    char * name;
> +} deviceinfo;
> +
> +
> +/* Return TRUE if ABFD is handled.  */
> +
> +static int
> +elf32_avr_filter (bfd *abfd)
> +{
> +  return bfd_get_flavour (abfd) == bfd_target_elf_flavour;
> +}
> +
> +static char*
> +elf32_avr_get_note_section_contents (bfd *abfd, bfd_size_type *size)
> +{
> +  asection *section;
> +
> +  if ((section = bfd_get_section_by_name (abfd, ".note.gnu.avr.deviceinfo")) == NULL)
> +    return NULL;
> +
> +  *size = bfd_get_section_size (section);
> +  char *contents = (char *) xmalloc (*size);
> +  bfd_get_section_contents (abfd, section, contents, 0, *size);
> +
> +  return contents;
> +}
> +
> +static char* elf32_avr_get_note_desc (bfd *abfd, char *contents,
> +        bfd_size_type size)
> +{
> +  Elf_External_Note *xnp = (Elf_External_Note *) contents;
> +  Elf_Internal_Note in;
> +
> +  if (offsetof (Elf_External_Note, name) > size)
> +    return NULL;
> +
> +  in.type = bfd_get_32 (abfd, xnp->type);
> +  in.namesz = bfd_get_32 (abfd, xnp->namesz);
> +  in.namedata = xnp->name;
> +  if (in.namesz > contents - in.namedata + size)
> +    return NULL;
> +
> +  in.descsz = bfd_get_32 (abfd, xnp->descsz);
> +  in.descdata = in.namedata + align_power (in.namesz, 2);
> +  if (in.descsz != 0
> +        && (in.descdata >= contents + size
> +            || in.descsz > contents - in.descdata + size))
> +    return NULL;
> +
> +  if (strcmp (in.namedata, "AVR") != 0)
> +    return NULL;
> +
> +  return in.descdata;
> +}
> +
> +static void
> +elf32_avr_get_device_info (bfd *abfd, char *description,
> +        deviceinfo *device)
> +{
> +  if (description == NULL)
> +    return;
> +
> +  const bfd_size_type memory_sizes = 6;
> +
> +  memcpy (device, description, memory_sizes * sizeof(uint32_t));
> +  device->name = NULL;
> +
> +  uint32_t *stroffset_table = ((uint32_t *) description) + memory_sizes;
> +  bfd_size_type stroffset_table_size = bfd_get_32 (abfd, stroffset_table);
> +  char *str_table = ((char *) stroffset_table) + stroffset_table_size;
> +
> +  /* If the only content is the size itself, there's nothing in the table */
> +  if (stroffset_table_size == 4)
> +    return;
> +
> +  /* First entry is the device name index. */
> +  uint32_t device_name_index = bfd_get_32 (abfd, stroffset_table + 1);
> +
> +  device->name = str_table + device_name_index;
> +}
> +
> +static void
> +elf32_avr_get_memory_usage (bfd *abfd,
> +        bfd_size_type *text_usage,
> +        bfd_size_type *data_usage,
> +        bfd_size_type *eeprom_usage)
> +{
> +
> +  bfd_size_type avr_datasize = 0;
> +  bfd_size_type avr_textsize = 0;
> +  bfd_size_type avr_bsssize = 0;
> +  bfd_size_type bootloadersize = 0;
> +  bfd_size_type noinitsize = 0;
> +  bfd_size_type eepromsize = 0;
> +  asection *section;
> +
> +  if ((section = bfd_get_section_by_name (abfd, ".data")) != NULL)
> +    avr_datasize = bfd_section_size (abfd, section);
> +  if ((section = bfd_get_section_by_name (abfd, ".text")) != NULL)
> +    avr_textsize = bfd_section_size (abfd, section);
> +  if ((section = bfd_get_section_by_name (abfd, ".bss")) != NULL)
> +    avr_bsssize = bfd_section_size (abfd, section);
> +  if ((section = bfd_get_section_by_name (abfd, ".bootloader")) != NULL)
> +    bootloadersize = bfd_section_size (abfd, section);
> +  if ((section = bfd_get_section_by_name (abfd, ".noinit")) != NULL)
> +    noinitsize = bfd_section_size (abfd, section);
> +  if ((section = bfd_get_section_by_name (abfd, ".eeprom")) != NULL)
> +    eepromsize = bfd_section_size (abfd, section);
> +
> +  *text_usage = avr_textsize + avr_datasize + bootloadersize;
> +  *data_usage = avr_datasize + avr_bsssize + noinitsize;
> +  *eeprom_usage = eepromsize;
> +}
> +
> +static void
> +elf32_avr_dump_mem_usage (bfd *abfd)
> +{
> +  char *description = NULL;
> +  bfd_size_type note_section_size = 0;
> +
> +  deviceinfo device = {0};
> +  device.name = "Unknown";
> +
> +  bfd_size_type data_usage = 0;
> +  bfd_size_type text_usage = 0;
> +  bfd_size_type eeprom_usage = 0;
> +
> +  char *contents = elf32_avr_get_note_section_contents (abfd,
> +    &note_section_size);
> +
> +  if (contents != NULL)
> +    {
> +      description = elf32_avr_get_note_desc (abfd, contents, note_section_size);
> +      elf32_avr_get_device_info (abfd, description, &device);
> +    }
> +
> +  elf32_avr_get_memory_usage (abfd, &text_usage, &data_usage,
> +     &eeprom_usage);
> +
> +  printf ("AVR Memory Usage\n"
> +          "----------------\n"
> +          "Device: %s\n\n", device.name);
> +
> +  /* Text size */
> +  printf ("Program:%8ld bytes", text_usage);
> +  if (device.flash_size > 0)
> +    printf (" (%2.1f%% Full)", ((float) text_usage / device.flash_size) * 100);
> +
> +  printf ("\n(.text + .data + .bootloader)\n\n");
> +
> +  /* Data size */
> +  printf ("Data:   %8ld bytes", data_usage);
> +  if (device.ram_size > 0)
> +    printf (" (%2.1f%% Full)", ((float) data_usage / device.ram_size) * 100);
> +
> +  printf ("\n(.data + .bss + .noinit)\n\n");
> +
> +  /* EEPROM size */
> +  if (eeprom_usage > 0)
> +    {
> +      printf ("EEPROM: %8ld bytes", eeprom_usage);
> +      if (device.eeprom_size > 0)
> +        printf (" (%2.1f%% Full)", ((float) eeprom_usage / device.eeprom_size) * 100);
> +
> +      printf ("\n(.eeprom)\n\n");
> +    }
> +
> +  if (contents != NULL)
> +    free (contents);
> +
> +}
> +
> +static void
> +elf32_avr_dump (bfd *abfd)
> +{
> +  if (options[OPT_MEMUSAGE].selected)
> +    elf32_avr_dump_mem_usage (abfd);
> +}
> +
> +const struct objdump_private_desc objdump_private_desc_elf32_avr =
> +  {
> +    elf32_avr_help,
> +    elf32_avr_filter,
> +    elf32_avr_dump,
> +    options
> +  };