Re: [2/3] RFC - reimplement common block support

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Tue, 21 Aug 2012 18:32:20 +0200, Tom Tromey wrote:
> +      for (child_die = die->child;
> +	   child_die && child_die->tag;
> +	   child_die = sibling_die (child_die))
> +	++n_entries;
> +
> +      size = (sizeof (struct fortran_common_block)
> +	      + (n_entries - 1) * sizeof (struct symbol *));
> +      common_block = obstack_alloc (&objfile->objfile_obstack, size);
> +      memset (common_block->contents, 0, n_entries * sizeof (struct symbol *));
> +      common_block->n_entries = n_entries;
> +
> +      count = 0;

A nitpick but why not to just use common_block->n_entries instead of count.


> +      for (child_die = die->child;
> +	   child_die && child_die->tag;
> +	   child_die = sibling_die (child_die))
> +	{
> +	  /* Create the symbol in the DW_TAG_common_block block in the current
> +	     symbol scope.  */
>  	  sym = new_symbol (child_die, NULL, cu);
> -	  if (sym != NULL
> -	      && handle_data_member_location (child_die, cu, &offset))
> -	    {
> -	      SYMBOL_VALUE_ADDRESS (sym) = base + offset;
> -	      add_symbol_to_list (sym, &global_symbols);
> -	    }
> -	  child_die = sibling_die (child_die);
> +	  if (sym)
> +	    common_block->contents[count++] = sym;

And then to just do
	common_block->contents[common_block->n_entries]++ = sym;


>  	}
> +
> +      sym = new_symbol (die, objfile_type (objfile)->builtin_void, cu);
> +      SYMBOL_VALUE_COMMON_BLOCK (sym) = common_block;
>      }
>  }
>  
[...]
> --- a/gdb/f-lang.h
> +++ b/gdb/f-lang.h
[...]
> +struct fortran_common_block
> +{
> +  /* The number of entries in the block.  */
> +  unsigned int n_entries;

With the pending "bitpos expansion patches" patches I would just find easier
in these cases in general to put here 'size_t' and no longer think how much
possible is it to overflow 2GB (it will get signed somewhere) or not.
It would sure expect other variables iterating it to also use size_t.

OTOH I agree it is very unreal to overflow this one so OK with it.


>  
> -extern SAVED_F77_COMMON_PTR find_common_for_function (const char *,
> -						      const char *);
> +  /* The contents of the block, allocated using the struct hack.  */
> +  struct symbol *contents[1];

Otherwise here should be a comment it may be NULL.  With proposed "count"
change here could even be a comment it must not be NULL.


> +};
[...]
> +	for (index = 0; index < common->n_entries; index++)
> +	  {
> +	    struct value *val = NULL;
> +	    volatile struct gdb_exception except;
> +
> +	    if (!common->contents[index])
> +	      continue;

And with the "count" change one can then drop this statement.


[...]
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/common-block.exp
[...]
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
> +
> +# This file was written by Jan Kratochvil <jan.kratochvil@redhat.com>.

Here is missing:

# It requires fortran.
if {[skip_fortran_tests]} {
    return 0
}


> +
> +standard_testfile .f90
> +
> +if {[prepare_for_testing ${testfile}.exp ${testfile} \
> +	 $srcfile {debug f90 quiet}]} {
> +    return -1
> +}


Thanks,
Jan