Re: [patch] .comm & alignment bug

[Nick Clifton <nickc at redhat dot com>]

Hi Denis,

: I want to define common variable without alignment, so I wrote:
: 
: .comm var,23,1
: 
: Is this right ? (IMHO:yes)

Yup.

: IMHO: bug in elflink.h:elf_link_add_object_symbols
: 
:       /* Set the alignment of a common symbol.  */
:       if (sym.st_shndx == SHN_COMMON
: 	  && h->root.type == bfd_link_hash_common)
: 	{
: 	  unsigned int align;
: 
: >	  align = bfd_log2 (sym.st_value);
: >	  if (align > old_alignment)
: >	    h->root.u.c.p->alignment_power = align;

: Index: elflink.h
: ===================================================================
:   
:   	  align = bfd_log2 (sym.st_value);
: ! 	  if (align > old_alignment)
:   	    h->root.u.c.p->alignment_power = align;
:   	}
:   
: --- 1527,1533 ----
:   
:   	  align = bfd_log2 (sym.st_value);
: ! 	  if (align >= old_alignment)
:   	    h->root.u.c.p->alignment_power = align;
:   	}

I agree that this is a bug, but I think that the patch you are
proposing is wrong since if no alignment was specified for the symbol
'sym.st_value' will be 0, which will result in 'align' being zero (*),
which will cause 'alignment_power' to be set to zero (assuming
old_alignment is also zero).  This is incorrect since the spec says
that if no alignment is specified its alignment will be:

  ... the largest power of two less than or equal to the size of the
  symbol, up to a maximum of 16.

Instead I would suggest the following, which is only slightly
different, and which should have the desired effect whilst making it
clear that 'align' is overriding the default alignment because an
alignment of 1 was requested and there are no other definitions of the
symbol with greater alignment requirements.

Please could you test this and let me know if it works for you.  If it
does then I will check the patch in.

Cheers
	Nick

(*) bfd_log2 (0) == bfd_log2 (1) == 0 !!!


2000-08-22  Denis Chertykov  <denisc@overta.ru> & Nick Clifton  <nickc@redhat.com>

	* elflink.h (elf_link_add_object_symbols): Allow common
	symbols to have an alignment of 1 if explicitly requested, and
	not overridden by other definitions.

Index: bfd/elflink.h
===================================================================
RCS file: /cvs/src//src/bfd/elflink.h,v
retrieving revision 1.70
diff -p -r1.70 elflink.h
*** elflink.h	2000/08/22 19:33:16	1.70
--- elflink.h	2000/08/22 21:42:26
*************** elf_link_add_object_symbols (abfd, info)
*** 1615,1621 ****
  	  unsigned int align;
  
  	  align = bfd_log2 (sym.st_value);
! 	  if (align > old_alignment)
  	    h->root.u.c.p->alignment_power = align;
  	}
  
--- 1615,1624 ----
  	  unsigned int align;
  
  	  align = bfd_log2 (sym.st_value);
! 	  if (align > old_alignment
! 	      /* Permit an alignment power of zero if an alignment of one
! 		 is specified and no other alignments have been specified.  */
! 	      || (sym.st_value == 1 && old_alignment == 0))
  	    h->root.u.c.p->alignment_power = align;
  	}