[RFC/PATCH] Fix `bfd_{get,set}_*' macros

[RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Sergio Durigan Junior]

Hi,

As a part of an effort to enable `-Wunused-variable' on GDB build
system, I would like to propose this "fix" to the following macros
declared in bfd-in2.h:

 bfd_get_section_name
 bfd_get_section_vma
 bfd_get_section_lma
 bfd_get_section_alignment
 bfd_section_name
 bfd_section_size
 bfd_get_section_flags
 bfd_get_section_userdata

Those macros don't need the first argument (`bfd'), but they still
require it.  However, you can pass anything there and GCC won't complain
about it, because the argument is unused after all.  So, while fixing
GDB to support -Wunused-variable, I saw that there was a declaration of
a `bfd *', which is used as a first argument to those macros.  GCC
started to complain about it (unused argument).  There were two paths
that I could follow:

1) Delete the declaration of `bfd *', so this:

   bfd *abfd = objfile->abfd;
   bfd_get_section_name (abfd, sec);

Would become:

   bfd_get_section_name (objfile->abfd, sec);

or

2) Fix the macros, so that they use the first argument in a safe way
like `(void) bfd', thus silencing GCC warnings.

After having tried the first option, Pedro asked me to actually fix the
macros, so here is the patch to do this, along with small fixes through
the code in order to adapt it.  I am labeling this patch as RFC because
I'm not sure I fixed the code in the right way.  It wasn't clear to me
which `bfd' to use in each situation, so I used my best judgement (which
may not be good).

As you will notice, I did not changed every bfd_*_section_* macro
because some of them are used as lvalue (left side of assignment).

I regtested the patch on Fedora 16 x86_64 with GDB, without
regressions.  Ok to apply?

-- 
Sergio
 
2012-05-03  Sergio Durigan Junior  <sergiodj@redhat.com>

	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
	bfd_section_size, bfd_get_section_flags,
	bfd_get_section_userdata): Rewrite macros in order to use the
	`bfd' argument.
	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
	as the first argument for `bfd_get_section_alignment'.
	* elf32-arm.c (create_ifunc_sections): Likewise, for
	`bfd_set_section_alignment'.
	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
	`bfd_get_section_name'.
	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
	(ppc_elf_relocate_section): Likewise.
	* elf64-mmix.c (mmix_final_link_relocate): Declaring proper `bfd'
	variable.
	* elf64-ppc.c (create_linkage_sections): Pass proper `bfd' as the
	first argument for `bfd_set_section_alignment'.

---
 bfd/bfd-in2.h          |   17 +++++++++--------
 bfd/elf-vxworks.c      |    3 ++-
 bfd/elf32-arm.c        |    4 ++--
 bfd/elf32-m32r.c       |    2 +-
 bfd/elf32-microblaze.c |    8 ++++----
 bfd/elf32-ppc.c        |   14 ++++++++------
 bfd/elf64-mmix.c       |    2 ++
 bfd/elf64-ppc.c        |    2 +-
 8 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 17dbbe1..86a65cc 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -280,18 +280,19 @@ alent;
 
 typedef struct bfd_section *sec_ptr;
 
-#define bfd_get_section_name(bfd, ptr) ((ptr)->name + 0)
-#define bfd_get_section_vma(bfd, ptr) ((ptr)->vma + 0)
-#define bfd_get_section_lma(bfd, ptr) ((ptr)->lma + 0)
-#define bfd_get_section_alignment(bfd, ptr) ((ptr)->alignment_power + 0)
-#define bfd_section_name(bfd, ptr) ((ptr)->name)
-#define bfd_section_size(bfd, ptr) ((ptr)->size)
+#define bfd_get_section_name(bfd, ptr) ((void) bfd, (ptr)->name)
+#define bfd_get_section_vma(bfd, ptr) ((void) bfd, (ptr)->vma)
+#define bfd_get_section_lma(bfd, ptr) ((void) bfd, (ptr)->lma)
+#define bfd_get_section_alignment(bfd, ptr) ((void) bfd, \
+					     (ptr)->alignment_power)
+#define bfd_section_name(bfd, ptr) ((void) bfd, (ptr)->name)
+#define bfd_section_size(bfd, ptr) ((void) bfd, (ptr)->size)
 #define bfd_get_section_size(ptr) ((ptr)->size)
 #define bfd_section_vma(bfd, ptr) ((ptr)->vma)
 #define bfd_section_lma(bfd, ptr) ((ptr)->lma)
 #define bfd_section_alignment(bfd, ptr) ((ptr)->alignment_power)
-#define bfd_get_section_flags(bfd, ptr) ((ptr)->flags + 0)
-#define bfd_get_section_userdata(bfd, ptr) ((ptr)->userdata)
+#define bfd_get_section_flags(bfd, ptr) ((void) bfd, (ptr)->flags)
+#define bfd_get_section_userdata(bfd, ptr) ((void) bfd, (ptr)->userdata)
 
 #define bfd_is_com_section(ptr) (((ptr)->flags & SEC_IS_COMMON) != 0)
 
diff --git a/bfd/elf-vxworks.c b/bfd/elf-vxworks.c
index 06edf8d..ebd8f2b 100644
--- a/bfd/elf-vxworks.c
+++ b/bfd/elf-vxworks.c
@@ -280,7 +280,8 @@ elf_vxworks_finish_dynamic_entry (bfd *output_bfd, Elf_Internal_Dyn *dyn)
     case DT_VX_WRS_TLS_DATA_ALIGN:
       sec = bfd_get_section_by_name (output_bfd, ".tls_data");
       dyn->d_un.d_val
-	= (bfd_size_type)1 << bfd_get_section_alignment (abfd, sec);
+	= (bfd_size_type)1 << bfd_get_section_alignment (output_bfd,
+							 sec);
       break;
       
     case DT_VX_WRS_TLS_VARS_START:
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index f5bad39..ae670f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -3258,7 +3258,7 @@ create_ifunc_sections (struct bfd_link_info *info)
       s = bfd_make_section_with_flags (dynobj, ".iplt",
 				       flags | SEC_READONLY | SEC_CODE);
       if (s == NULL
-	  || !bfd_set_section_alignment (abfd, s, bed->plt_alignment))
+	  || !bfd_set_section_alignment (dynobj, s, bed->plt_alignment))
 	return FALSE;
       htab->root.iplt = s;
     }
@@ -3268,7 +3268,7 @@ create_ifunc_sections (struct bfd_link_info *info)
       s = bfd_make_section_with_flags (dynobj, RELOC_SECTION (htab, ".iplt"),
 				       flags | SEC_READONLY);
       if (s == NULL
-	  || !bfd_set_section_alignment (abfd, s, bed->s->log_file_align))
+	  || !bfd_set_section_alignment (dynobj, s, bed->s->log_file_align))
 	return FALSE;
       htab->root.irelplt = s;
     }
diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
index 7ab7b60..4445e56 100644
--- a/bfd/elf32-m32r.c
+++ b/bfd/elf32-m32r.c
@@ -3007,7 +3007,7 @@ m32r_elf_relocate_section (bfd *output_bfd ATTRIBUTE_UNUSED,
 		const char *name;
 
 		BFD_ASSERT (sec != NULL);
-		name = bfd_get_section_name (abfd, sec);
+		name = bfd_get_section_name (input_bfd, sec);
 
 		if (   strcmp (name, ".sdata") == 0
 		    || strcmp (name, ".sbss") == 0
diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a58f7b3..2d3183c 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -839,7 +839,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 		/* Only relocate if the symbol is defined.  */
 		if (sec)
 		  {
-		    name = bfd_get_section_name (abfd, sec);
+		    name = bfd_get_section_name (input_bfd, sec);
 
 		    if (strcmp (name, ".sdata2") == 0
 			|| strcmp (name, ".sbss2") == 0)
@@ -868,7 +868,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 					       bfd_get_filename (input_bfd),
 					       sym_name,
 					       microblaze_elf_howto_table[(int) r_type]->name,
-					       bfd_get_section_name (abfd, sec));
+					       bfd_get_section_name (input_bfd, sec));
 			/*bfd_set_error (bfd_error_bad_value); ??? why? */
 			ret = FALSE;
 			continue;
@@ -884,7 +884,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 		/* Only relocate if the symbol is defined.  */
 		if (sec)
 		  {
-		    name = bfd_get_section_name (abfd, sec);
+		    name = bfd_get_section_name (input_bfd, sec);
 
 		    if (strcmp (name, ".sdata") == 0
 			|| strcmp (name, ".sbss") == 0)
@@ -913,7 +913,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 					       bfd_get_filename (input_bfd),
 					       sym_name,
 					       microblaze_elf_howto_table[(int) r_type]->name,
-					       bfd_get_section_name (abfd, sec));
+					       bfd_get_section_name (input_bfd, sec));
 			/*bfd_set_error (bfd_error_bad_value); ??? why? */
 			ret = FALSE;
 			continue;
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index ca6df26..de3e492 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -5867,7 +5867,7 @@ ppc_elf_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 	{
 	  /* Strip these too.  */
 	}
-      else if (CONST_STRNEQ (bfd_get_section_name (dynobj, s), ".rela"))
+      else if (CONST_STRNEQ (bfd_get_section_name (ibfd, s), ".rela"))
 	{
 	  if (s->size != 0)
 	    {
@@ -7886,8 +7886,10 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      unresolved_reloc = TRUE;
 	      break;
 	    }
-	  BFD_ASSERT (strcmp (bfd_get_section_name (abfd, sec), ".got") == 0
-		      || strcmp (bfd_get_section_name (abfd, sec), ".cgot") == 0);
+	  BFD_ASSERT (strcmp (bfd_get_section_name (input_bfd, sec),
+			      ".got") == 0
+		      || strcmp (bfd_get_section_name (input_bfd, sec),
+				 ".cgot") == 0);
 
 	  addend -= sec->output_section->vma + sec->output_offset + 0x8000;
 	  break;
@@ -7937,7 +7939,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      }
 	    addend -= SYM_VAL (sda);
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (input_bfd, sec->output_section);
 	    if (! ((CONST_STRNEQ (name, ".sdata")
 		    && (name[6] == 0 || name[6] == '.'))
 		   || (CONST_STRNEQ (name, ".sbss")
@@ -7969,7 +7971,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      }
 	    addend -= SYM_VAL (sda);
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (input_bfd, sec->output_section);
 	    if (! (CONST_STRNEQ (name, ".sdata2")
 		   || CONST_STRNEQ (name, ".sbss2")))
 	      {
@@ -7998,7 +8000,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 		break;
 	      }
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (input_bfd, sec->output_section);
 	    if (((CONST_STRNEQ (name, ".sdata")
 		  && (name[6] == 0 || name[6] == '.'))
 		 || (CONST_STRNEQ (name, ".sbss")
diff --git a/bfd/elf64-mmix.c b/bfd/elf64-mmix.c
index fd5921a..4a33fba 100644
--- a/bfd/elf64-mmix.c
+++ b/bfd/elf64-mmix.c
@@ -1771,6 +1771,8 @@ mmix_final_link_relocate (reloc_howto_type *howto, asection *input_section,
 	first_global = 255;
       else
 	{
+	  bfd *abfd = NULL;
+
 	  first_global = bfd_get_section_vma (abfd, regsec) / 8;
 	  if (strcmp (bfd_get_section_name (symsec->owner, symsec),
 		      MMIX_REG_CONTENTS_SECTION_NAME) == 0)
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 0d6dd99..7c7beb6 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -4245,7 +4245,7 @@ create_linkage_sections (bfd *dynobj, struct bfd_link_info *info)
 								 ".eh_frame",
 								 flags);
       if (htab->glink_eh_frame == NULL
-	  || !bfd_set_section_alignment (abfd, htab->glink_eh_frame, 2))
+	  || !bfd_set_section_alignment (dynobj, htab->glink_eh_frame, 2))
 	return FALSE;
     }
 
-- 
1.7.7.6

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Alan Modra]

On Thu, May 03, 2012 at 01:19:21AM -0300, Sergio Durigan Junior wrote:
> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
> 	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
> 	bfd_section_size, bfd_get_section_flags,
> 	bfd_get_section_userdata): Rewrite macros in order to use the
> 	`bfd' argument.
> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
> 	as the first argument for `bfd_get_section_alignment'.
> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
> 	`bfd_set_section_alignment'.

The above is OK.  You guessed wrongly for all the bfd args below,
except in elf64-ppc.c.  I suggest expanding the macros instead.  I've
noted the correct bfds below but IMO these macros serve no useful
purpose.

> 	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
> 	`bfd_get_section_name'.
> 	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
> 	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
> 	(ppc_elf_relocate_section): Likewise.
> 	* elf64-mmix.c (mmix_final_link_relocate): Declaring proper `bfd'
> 	variable.
> 	* elf64-ppc.c (create_linkage_sections): Pass proper `bfd' as the
> 	first argument for `bfd_set_section_alignment'.

> --- a/bfd/elf32-m32r.c
> +++ b/bfd/elf32-m32r.c
> @@ -3007,7 +3007,7 @@ m32r_elf_relocate_section (bfd *output_bfd ATTRIBUTE_UNUSED,
>  		const char *name;
>  
>  		BFD_ASSERT (sec != NULL);
> -		name = bfd_get_section_name (abfd, sec);
> +		name = bfd_get_section_name (input_bfd, sec);

sec->owner

> --- a/bfd/elf32-microblaze.c
> +++ b/bfd/elf32-microblaze.c
> @@ -839,7 +839,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  		/* Only relocate if the symbol is defined.  */
>  		if (sec)
>  		  {
> -		    name = bfd_get_section_name (abfd, sec);
> +		    name = bfd_get_section_name (input_bfd, sec);

sec->owner

> @@ -868,7 +868,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  					       bfd_get_filename (input_bfd),
>  					       sym_name,
>  					       microblaze_elf_howto_table[(int) r_type]->name,
> -					       bfd_get_section_name (abfd, sec));
> +					       bfd_get_section_name (input_bfd, sec));

sec->owner

> @@ -884,7 +884,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  		/* Only relocate if the symbol is defined.  */
>  		if (sec)
>  		  {
> -		    name = bfd_get_section_name (abfd, sec);
> +		    name = bfd_get_section_name (input_bfd, sec);

sec->owner

> @@ -913,7 +913,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  					       bfd_get_filename (input_bfd),
>  					       sym_name,
>  					       microblaze_elf_howto_table[(int) r_type]->name,
> -					       bfd_get_section_name (abfd, sec));
> +					       bfd_get_section_name (input_bfd, sec));

sec->owner

> --- a/bfd/elf32-ppc.c
> +++ b/bfd/elf32-ppc.c
> @@ -5867,7 +5867,7 @@ ppc_elf_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
>  	{
>  	  /* Strip these too.  */
>  	}
> -      else if (CONST_STRNEQ (bfd_get_section_name (dynobj, s), ".rela"))
> +      else if (CONST_STRNEQ (bfd_get_section_name (ibfd, s), ".rela"))

htab->elf.dynobj

> @@ -7886,8 +7886,10 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  	      unresolved_reloc = TRUE;
>  	      break;
>  	    }
> -	  BFD_ASSERT (strcmp (bfd_get_section_name (abfd, sec), ".got") == 0
> -		      || strcmp (bfd_get_section_name (abfd, sec), ".cgot") == 0);
> +	  BFD_ASSERT (strcmp (bfd_get_section_name (input_bfd, sec),
> +			      ".got") == 0
> +		      || strcmp (bfd_get_section_name (input_bfd, sec),
> +				 ".cgot") == 0);

sec->owner twice

> @@ -7937,7 +7939,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  	      }
>  	    addend -= SYM_VAL (sda);
>  
> -	    name = bfd_get_section_name (abfd, sec->output_section);
> +	    name = bfd_get_section_name (input_bfd, sec->output_section);

output_bfd

> @@ -7969,7 +7971,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  	      }
>  	    addend -= SYM_VAL (sda);
>  
> -	    name = bfd_get_section_name (abfd, sec->output_section);
> +	    name = bfd_get_section_name (input_bfd, sec->output_section);

output_bfd

> @@ -7998,7 +8000,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  		break;
>  	      }
>  
> -	    name = bfd_get_section_name (abfd, sec->output_section);
> +	    name = bfd_get_section_name (input_bfd, sec->output_section);

output_bfd

> --- a/bfd/elf64-mmix.c
> +++ b/bfd/elf64-mmix.c
> @@ -1771,6 +1771,8 @@ mmix_final_link_relocate (reloc_howto_type *howto, asection *input_section,
>  	first_global = 255;
>        else
>  	{
> +	  bfd *abfd = NULL;
> +
>  	  first_global = bfd_get_section_vma (abfd, regsec) / 8;

input_section->output_section->owner

-- 
Alan Modra
Australia Development Lab, IBM

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Doug Evans]

On Wed, May 2, 2012 at 9:19 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> Hi,
>
> As a part of an effort to enable `-Wunused-variable' on GDB build
> system, I would like to propose this "fix" to the following macros
> declared in bfd-in2.h:
>
>  bfd_get_section_name
>  bfd_get_section_vma
>  bfd_get_section_lma
>  bfd_get_section_alignment
>  bfd_section_name
>  bfd_section_size
>  bfd_get_section_flags
>  bfd_get_section_userdata
>
> Those macros don't need the first argument (`bfd'), but they still
> require it.  However, you can pass anything there and GCC won't complain
> about it, because the argument is unused after all.  So, while fixing
> GDB to support -Wunused-variable, I saw that there was a declaration of
> a `bfd *', which is used as a first argument to those macros.  GCC
> started to complain about it (unused argument).  There were two paths
> that I could follow:
>
> 1) Delete the declaration of `bfd *', so this:
>
>   bfd *abfd = objfile->abfd;
>   bfd_get_section_name (abfd, sec);
>
> Would become:
>
>   bfd_get_section_name (objfile->abfd, sec);
>
> or
>
> 2) Fix the macros, so that they use the first argument in a safe way
> like `(void) bfd', thus silencing GCC warnings.
>
> After having tried the first option, Pedro asked me to actually fix the
> macros, so here is the patch to do this, along with small fixes through
> the code in order to adapt it.  I am labeling this patch as RFC because
> I'm not sure I fixed the code in the right way.  It wasn't clear to me
> which `bfd' to use in each situation, so I used my best judgement (which
> may not be good).
>
> As you will notice, I did not changed every bfd_*_section_* macro
> because some of them are used as lvalue (left side of assignment).
>
> I regtested the patch on Fedora 16 x86_64 with GDB, without
> regressions.  Ok to apply?

I never liked these macros, it's like they had two "this" arguments.

How much more work would it be to actually remove the bfd argument?

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Pedro Alves]

On 05/03/2012 08:53 AM, Alan Modra wrote:

> On Thu, May 03, 2012 at 01:19:21AM -0300, Sergio Durigan Junior wrote:
>> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
>> 	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
>> 	bfd_section_size, bfd_get_section_flags,
>> 	bfd_get_section_userdata): Rewrite macros in order to use the
>> 	`bfd' argument.
>> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
>> 	as the first argument for `bfd_get_section_alignment'.
>> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
>> 	`bfd_set_section_alignment'.
> 
> The above is OK.  You guessed wrongly for all the bfd args below,
> except in elf64-ppc.c.  I suggest expanding the macros instead.  I've
> noted the correct bfds below but IMO these macros serve no useful
> purpose.


#define bfd_get_section_name(bfd, ptr) ((ptr)->name + 0)
#define bfd_get_section_vma(bfd, ptr) ((ptr)->vma + 0)
#define bfd_get_section_lma(bfd, ptr) ((ptr)->lma + 0)
#define bfd_get_section_alignment(bfd, ptr) ((ptr)->alignment_power + 0)
#define bfd_section_name(bfd, ptr) ((ptr)->name)
#define bfd_section_size(bfd, ptr) ((ptr)->size)
#define bfd_get_section_size(ptr) ((ptr)->size)
#define bfd_section_vma(bfd, ptr) ((ptr)->vma)
#define bfd_section_lma(bfd, ptr) ((ptr)->lma)
#define bfd_section_alignment(bfd, ptr) ((ptr)->alignment_power)
#define bfd_get_section_flags(bfd, ptr) ((ptr)->flags + 0)
#define bfd_get_section_userdata(bfd, ptr) ((ptr)->userdata)

Or replace with bfd_section_name, bfd_section_vma, etc. (making them
lvalues along the way) ?

If dropping the bfd argument, IMO, it's better to use the 'bfd_section'
prefix for bfd_section macros (like bfd_section_name, etc. above), rather
than 'bfd_get_', and leave the latter for macros/functions that really
take a struct bfd as "this".  That'd mean
s/bfd_get_section_flags/bfd_section_flags/ for example.

But IMO, we shouldn't require Sérgio do this, unless he
wants to.  His patch just fixes a handful of places to correctly
use the already years-old existing interface...

-- 
Pedro Alves

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Sergio Durigan Junior]

On Thursday, May 03 2012, Alan Modra wrote:

> On Thu, May 03, 2012 at 01:19:21AM -0300, Sergio Durigan Junior wrote:
>> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
>> 	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
>> 	bfd_section_size, bfd_get_section_flags,
>> 	bfd_get_section_userdata): Rewrite macros in order to use the
>> 	`bfd' argument.
>> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
>> 	as the first argument for `bfd_get_section_alignment'.
>> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
>> 	`bfd_set_section_alignment'.
>
> The above is OK.  You guessed wrongly for all the bfd args below,
> except in elf64-ppc.c.  I suggest expanding the macros instead.  I've
> noted the correct bfds below but IMO these macros serve no useful
> purpose.

Thanks for the review.  I've chosen to update the patch as proposed
below, instead of expanding the macros as you suggested.  I'm involved
in other tasks and it would take much more time, I'm afraid.

WDYT of the patch below?

2012-05-03  Sergio Durigan Junior  <sergiodj@redhat.com>

	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
	bfd_section_size, bfd_get_section_flags,
	bfd_get_section_userdata): Rewrite macros in order to use the
	`bfd' argument.
	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
	as the first argument for `bfd_get_section_alignment'.
	* elf32-arm.c (create_ifunc_sections): Likewise, for
	`bfd_set_section_alignment'.
	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
	`bfd_get_section_name'.
	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
	(ppc_elf_relocate_section): Likewise.
	* elf64-mmix.c (mmix_final_link_relocate): Likewise, for
	`bfd_get_section_vma'.
	* elf64-ppc.c (create_linkage_sections): Likewise, for
	`bfd_set_section_alignment'.

---
 bfd/bfd-in2.h          |   17 +++++++++--------
 bfd/elf-vxworks.c      |    3 ++-
 bfd/elf32-arm.c        |    4 ++--
 bfd/elf32-m32r.c       |    2 +-
 bfd/elf32-microblaze.c |    8 ++++----
 bfd/elf32-ppc.c        |   15 +++++++++------
 bfd/elf64-mmix.c       |    4 +++-
 bfd/elf64-ppc.c        |    2 +-
 8 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 17dbbe1..86a65cc 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -280,18 +280,19 @@ alent;
 
 typedef struct bfd_section *sec_ptr;
 
-#define bfd_get_section_name(bfd, ptr) ((ptr)->name + 0)
-#define bfd_get_section_vma(bfd, ptr) ((ptr)->vma + 0)
-#define bfd_get_section_lma(bfd, ptr) ((ptr)->lma + 0)
-#define bfd_get_section_alignment(bfd, ptr) ((ptr)->alignment_power + 0)
-#define bfd_section_name(bfd, ptr) ((ptr)->name)
-#define bfd_section_size(bfd, ptr) ((ptr)->size)
+#define bfd_get_section_name(bfd, ptr) ((void) bfd, (ptr)->name)
+#define bfd_get_section_vma(bfd, ptr) ((void) bfd, (ptr)->vma)
+#define bfd_get_section_lma(bfd, ptr) ((void) bfd, (ptr)->lma)
+#define bfd_get_section_alignment(bfd, ptr) ((void) bfd, \
+					     (ptr)->alignment_power)
+#define bfd_section_name(bfd, ptr) ((void) bfd, (ptr)->name)
+#define bfd_section_size(bfd, ptr) ((void) bfd, (ptr)->size)
 #define bfd_get_section_size(ptr) ((ptr)->size)
 #define bfd_section_vma(bfd, ptr) ((ptr)->vma)
 #define bfd_section_lma(bfd, ptr) ((ptr)->lma)
 #define bfd_section_alignment(bfd, ptr) ((ptr)->alignment_power)
-#define bfd_get_section_flags(bfd, ptr) ((ptr)->flags + 0)
-#define bfd_get_section_userdata(bfd, ptr) ((ptr)->userdata)
+#define bfd_get_section_flags(bfd, ptr) ((void) bfd, (ptr)->flags)
+#define bfd_get_section_userdata(bfd, ptr) ((void) bfd, (ptr)->userdata)
 
 #define bfd_is_com_section(ptr) (((ptr)->flags & SEC_IS_COMMON) != 0)
 
diff --git a/bfd/elf-vxworks.c b/bfd/elf-vxworks.c
index 06edf8d..ebd8f2b 100644
--- a/bfd/elf-vxworks.c
+++ b/bfd/elf-vxworks.c
@@ -280,7 +280,8 @@ elf_vxworks_finish_dynamic_entry (bfd *output_bfd, Elf_Internal_Dyn *dyn)
     case DT_VX_WRS_TLS_DATA_ALIGN:
       sec = bfd_get_section_by_name (output_bfd, ".tls_data");
       dyn->d_un.d_val
-	= (bfd_size_type)1 << bfd_get_section_alignment (abfd, sec);
+	= (bfd_size_type)1 << bfd_get_section_alignment (output_bfd,
+							 sec);
       break;
       
     case DT_VX_WRS_TLS_VARS_START:
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index f5bad39..ae670f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -3258,7 +3258,7 @@ create_ifunc_sections (struct bfd_link_info *info)
       s = bfd_make_section_with_flags (dynobj, ".iplt",
 				       flags | SEC_READONLY | SEC_CODE);
       if (s == NULL
-	  || !bfd_set_section_alignment (abfd, s, bed->plt_alignment))
+	  || !bfd_set_section_alignment (dynobj, s, bed->plt_alignment))
 	return FALSE;
       htab->root.iplt = s;
     }
@@ -3268,7 +3268,7 @@ create_ifunc_sections (struct bfd_link_info *info)
       s = bfd_make_section_with_flags (dynobj, RELOC_SECTION (htab, ".iplt"),
 				       flags | SEC_READONLY);
       if (s == NULL
-	  || !bfd_set_section_alignment (abfd, s, bed->s->log_file_align))
+	  || !bfd_set_section_alignment (dynobj, s, bed->s->log_file_align))
 	return FALSE;
       htab->root.irelplt = s;
     }
diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
index 7ab7b60..4f06354 100644
--- a/bfd/elf32-m32r.c
+++ b/bfd/elf32-m32r.c
@@ -3007,7 +3007,7 @@ m32r_elf_relocate_section (bfd *output_bfd ATTRIBUTE_UNUSED,
 		const char *name;
 
 		BFD_ASSERT (sec != NULL);
-		name = bfd_get_section_name (abfd, sec);
+		name = bfd_get_section_name (sec->owner, sec);
 
 		if (   strcmp (name, ".sdata") == 0
 		    || strcmp (name, ".sbss") == 0
diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a58f7b3..5a20f0d 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -839,7 +839,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 		/* Only relocate if the symbol is defined.  */
 		if (sec)
 		  {
-		    name = bfd_get_section_name (abfd, sec);
+		    name = bfd_get_section_name (sec->owner, sec);
 
 		    if (strcmp (name, ".sdata2") == 0
 			|| strcmp (name, ".sbss2") == 0)
@@ -868,7 +868,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 					       bfd_get_filename (input_bfd),
 					       sym_name,
 					       microblaze_elf_howto_table[(int) r_type]->name,
-					       bfd_get_section_name (abfd, sec));
+					       bfd_get_section_name (sec->owner, sec));
 			/*bfd_set_error (bfd_error_bad_value); ??? why? */
 			ret = FALSE;
 			continue;
@@ -884,7 +884,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 		/* Only relocate if the symbol is defined.  */
 		if (sec)
 		  {
-		    name = bfd_get_section_name (abfd, sec);
+		    name = bfd_get_section_name (sec->owner, sec);
 
 		    if (strcmp (name, ".sdata") == 0
 			|| strcmp (name, ".sbss") == 0)
@@ -913,7 +913,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 					       bfd_get_filename (input_bfd),
 					       sym_name,
 					       microblaze_elf_howto_table[(int) r_type]->name,
-					       bfd_get_section_name (abfd, sec));
+					       bfd_get_section_name (sec->owner, sec));
 			/*bfd_set_error (bfd_error_bad_value); ??? why? */
 			ret = FALSE;
 			continue;
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index ca6df26..f5df17e 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -5867,7 +5867,8 @@ ppc_elf_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 	{
 	  /* Strip these too.  */
 	}
-      else if (CONST_STRNEQ (bfd_get_section_name (dynobj, s), ".rela"))
+      else if (CONST_STRNEQ (bfd_get_section_name (htab->elf.dynobj, s),
+			     ".rela"))
 	{
 	  if (s->size != 0)
 	    {
@@ -7886,8 +7887,10 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      unresolved_reloc = TRUE;
 	      break;
 	    }
-	  BFD_ASSERT (strcmp (bfd_get_section_name (abfd, sec), ".got") == 0
-		      || strcmp (bfd_get_section_name (abfd, sec), ".cgot") == 0);
+	  BFD_ASSERT (strcmp (bfd_get_section_name (sec->owner, sec),
+			      ".got") == 0
+		      || strcmp (bfd_get_section_name (sec->owner, sec),
+				 ".cgot") == 0);
 
 	  addend -= sec->output_section->vma + sec->output_offset + 0x8000;
 	  break;
@@ -7937,7 +7940,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      }
 	    addend -= SYM_VAL (sda);
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (output_bfd, sec->output_section);
 	    if (! ((CONST_STRNEQ (name, ".sdata")
 		    && (name[6] == 0 || name[6] == '.'))
 		   || (CONST_STRNEQ (name, ".sbss")
@@ -7969,7 +7972,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      }
 	    addend -= SYM_VAL (sda);
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (output_bfd, sec->output_section);
 	    if (! (CONST_STRNEQ (name, ".sdata2")
 		   || CONST_STRNEQ (name, ".sbss2")))
 	      {
@@ -7998,7 +8001,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 		break;
 	      }
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (output_bfd, sec->output_section);
 	    if (((CONST_STRNEQ (name, ".sdata")
 		  && (name[6] == 0 || name[6] == '.'))
 		 || (CONST_STRNEQ (name, ".sbss")
diff --git a/bfd/elf64-mmix.c b/bfd/elf64-mmix.c
index fd5921a..ae4b0fa 100644
--- a/bfd/elf64-mmix.c
+++ b/bfd/elf64-mmix.c
@@ -1771,7 +1771,9 @@ mmix_final_link_relocate (reloc_howto_type *howto, asection *input_section,
 	first_global = 255;
       else
 	{
-	  first_global = bfd_get_section_vma (abfd, regsec) / 8;
+	  first_global
+	    = bfd_get_section_vma (input_section->output_section->owner,
+				   regsec) / 8;
 	  if (strcmp (bfd_get_section_name (symsec->owner, symsec),
 		      MMIX_REG_CONTENTS_SECTION_NAME) == 0)
 	    {
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 0d6dd99..7c7beb6 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -4245,7 +4245,7 @@ create_linkage_sections (bfd *dynobj, struct bfd_link_info *info)
 								 ".eh_frame",
 								 flags);
       if (htab->glink_eh_frame == NULL
-	  || !bfd_set_section_alignment (abfd, htab->glink_eh_frame, 2))
+	  || !bfd_set_section_alignment (dynobj, htab->glink_eh_frame, 2))
 	return FALSE;
     }
 
-- 
1.7.7.6



-- 
Sergio

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Sergio Durigan Junior]

On Thursday, May 03 2012, I wrote:

> On Thursday, May 03 2012, Alan Modra wrote:
>
>> On Thu, May 03, 2012 at 01:19:21AM -0300, Sergio Durigan Junior wrote:
>>> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
>>> 	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
>>> 	bfd_section_size, bfd_get_section_flags,
>>> 	bfd_get_section_userdata): Rewrite macros in order to use the
>>> 	`bfd' argument.
>>> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
>>> 	as the first argument for `bfd_get_section_alignment'.
>>> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
>>> 	`bfd_set_section_alignment'.
>>
>> The above is OK.  You guessed wrongly for all the bfd args below,
>> except in elf64-ppc.c.  I suggest expanding the macros instead.  I've
>> noted the correct bfds below but IMO these macros serve no useful
>> purpose.
>
> Thanks for the review.  I've chosen to update the patch as proposed
> below, instead of expanding the macros as you suggested.  I'm involved
> in other tasks and it would take much more time, I'm afraid.
>
> WDYT of the patch below?

I did something that I should have done before: I built binutils with my
patch :-).  As it turns out, it's better not to fix `bfd_section_name'
and `bfd_section_size', so here's the new patch which builds correctly
on both binutils and GDB.  OK to apply?

2012-05-04  Sergio Durigan Junior  <sergiodj@redhat.com>

	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
	bfd_get_section_lma, bfd_get_section_alignment,
	bfd_get_section_flags,
	bfd_get_section_userdata): Rewrite macros in order to use the
	`bfd' argument.
	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
	as the first argument for `bfd_get_section_alignment'.
	* elf32-arm.c (create_ifunc_sections): Likewise, for
	`bfd_set_section_alignment'.
	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
	`bfd_get_section_name'.
	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
	(ppc_elf_relocate_section): Likewise.
	* elf64-mmix.c (mmix_final_link_relocate): Likewise, for
	`bfd_get_section_vma'.
	* elf64-ppc.c (create_linkage_sections): Likewise, for
	`bfd_set_section_alignment'.


diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 6b94f72..c7cee6d 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -280,18 +280,19 @@ alent;
 
 typedef struct bfd_section *sec_ptr;
 
-#define bfd_get_section_name(bfd, ptr) ((ptr)->name + 0)
-#define bfd_get_section_vma(bfd, ptr) ((ptr)->vma + 0)
-#define bfd_get_section_lma(bfd, ptr) ((ptr)->lma + 0)
-#define bfd_get_section_alignment(bfd, ptr) ((ptr)->alignment_power + 0)
+#define bfd_get_section_name(bfd, ptr) ((void) bfd, (ptr)->name)
+#define bfd_get_section_vma(bfd, ptr) ((void) bfd, (ptr)->vma)
+#define bfd_get_section_lma(bfd, ptr) ((void) bfd, (ptr)->lma)
+#define bfd_get_section_alignment(bfd, ptr) ((void) bfd, \
+					     (ptr)->alignment_power)
 #define bfd_section_name(bfd, ptr) ((ptr)->name)
 #define bfd_section_size(bfd, ptr) ((ptr)->size)
 #define bfd_get_section_size(ptr) ((ptr)->size)
 #define bfd_section_vma(bfd, ptr) ((ptr)->vma)
 #define bfd_section_lma(bfd, ptr) ((ptr)->lma)
 #define bfd_section_alignment(bfd, ptr) ((ptr)->alignment_power)
-#define bfd_get_section_flags(bfd, ptr) ((ptr)->flags + 0)
-#define bfd_get_section_userdata(bfd, ptr) ((ptr)->userdata)
+#define bfd_get_section_flags(bfd, ptr) ((void) bfd, (ptr)->flags)
+#define bfd_get_section_userdata(bfd, ptr) ((void) bfd, (ptr)->userdata)
 
 #define bfd_is_com_section(ptr) (((ptr)->flags & SEC_IS_COMMON) != 0)
 
diff --git a/bfd/elf-vxworks.c b/bfd/elf-vxworks.c
index 06edf8d..ebd8f2b 100644
--- a/bfd/elf-vxworks.c
+++ b/bfd/elf-vxworks.c
@@ -280,7 +280,8 @@ elf_vxworks_finish_dynamic_entry (bfd *output_bfd, Elf_Internal_Dyn *dyn)
     case DT_VX_WRS_TLS_DATA_ALIGN:
       sec = bfd_get_section_by_name (output_bfd, ".tls_data");
       dyn->d_un.d_val
-	= (bfd_size_type)1 << bfd_get_section_alignment (abfd, sec);
+	= (bfd_size_type)1 << bfd_get_section_alignment (output_bfd,
+							 sec);
       break;
       
     case DT_VX_WRS_TLS_VARS_START:
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index f5bad39..ae670f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -3258,7 +3258,7 @@ create_ifunc_sections (struct bfd_link_info *info)
       s = bfd_make_section_with_flags (dynobj, ".iplt",
 				       flags | SEC_READONLY | SEC_CODE);
       if (s == NULL
-	  || !bfd_set_section_alignment (abfd, s, bed->plt_alignment))
+	  || !bfd_set_section_alignment (dynobj, s, bed->plt_alignment))
 	return FALSE;
       htab->root.iplt = s;
     }
@@ -3268,7 +3268,7 @@ create_ifunc_sections (struct bfd_link_info *info)
       s = bfd_make_section_with_flags (dynobj, RELOC_SECTION (htab, ".iplt"),
 				       flags | SEC_READONLY);
       if (s == NULL
-	  || !bfd_set_section_alignment (abfd, s, bed->s->log_file_align))
+	  || !bfd_set_section_alignment (dynobj, s, bed->s->log_file_align))
 	return FALSE;
       htab->root.irelplt = s;
     }
diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
index 7ab7b60..4f06354 100644
--- a/bfd/elf32-m32r.c
+++ b/bfd/elf32-m32r.c
@@ -3007,7 +3007,7 @@ m32r_elf_relocate_section (bfd *output_bfd ATTRIBUTE_UNUSED,
 		const char *name;
 
 		BFD_ASSERT (sec != NULL);
-		name = bfd_get_section_name (abfd, sec);
+		name = bfd_get_section_name (sec->owner, sec);
 
 		if (   strcmp (name, ".sdata") == 0
 		    || strcmp (name, ".sbss") == 0
diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a58f7b3..5a20f0d 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -839,7 +839,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 		/* Only relocate if the symbol is defined.  */
 		if (sec)
 		  {
-		    name = bfd_get_section_name (abfd, sec);
+		    name = bfd_get_section_name (sec->owner, sec);
 
 		    if (strcmp (name, ".sdata2") == 0
 			|| strcmp (name, ".sbss2") == 0)
@@ -868,7 +868,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 					       bfd_get_filename (input_bfd),
 					       sym_name,
 					       microblaze_elf_howto_table[(int) r_type]->name,
-					       bfd_get_section_name (abfd, sec));
+					       bfd_get_section_name (sec->owner, sec));
 			/*bfd_set_error (bfd_error_bad_value); ??? why? */
 			ret = FALSE;
 			continue;
@@ -884,7 +884,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 		/* Only relocate if the symbol is defined.  */
 		if (sec)
 		  {
-		    name = bfd_get_section_name (abfd, sec);
+		    name = bfd_get_section_name (sec->owner, sec);
 
 		    if (strcmp (name, ".sdata") == 0
 			|| strcmp (name, ".sbss") == 0)
@@ -913,7 +913,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
 					       bfd_get_filename (input_bfd),
 					       sym_name,
 					       microblaze_elf_howto_table[(int) r_type]->name,
-					       bfd_get_section_name (abfd, sec));
+					       bfd_get_section_name (sec->owner, sec));
 			/*bfd_set_error (bfd_error_bad_value); ??? why? */
 			ret = FALSE;
 			continue;
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index ca6df26..f5df17e 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -5867,7 +5867,8 @@ ppc_elf_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 	{
 	  /* Strip these too.  */
 	}
-      else if (CONST_STRNEQ (bfd_get_section_name (dynobj, s), ".rela"))
+      else if (CONST_STRNEQ (bfd_get_section_name (htab->elf.dynobj, s),
+			     ".rela"))
 	{
 	  if (s->size != 0)
 	    {
@@ -7886,8 +7887,10 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      unresolved_reloc = TRUE;
 	      break;
 	    }
-	  BFD_ASSERT (strcmp (bfd_get_section_name (abfd, sec), ".got") == 0
-		      || strcmp (bfd_get_section_name (abfd, sec), ".cgot") == 0);
+	  BFD_ASSERT (strcmp (bfd_get_section_name (sec->owner, sec),
+			      ".got") == 0
+		      || strcmp (bfd_get_section_name (sec->owner, sec),
+				 ".cgot") == 0);
 
 	  addend -= sec->output_section->vma + sec->output_offset + 0x8000;
 	  break;
@@ -7937,7 +7940,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      }
 	    addend -= SYM_VAL (sda);
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (output_bfd, sec->output_section);
 	    if (! ((CONST_STRNEQ (name, ".sdata")
 		    && (name[6] == 0 || name[6] == '.'))
 		   || (CONST_STRNEQ (name, ".sbss")
@@ -7969,7 +7972,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      }
 	    addend -= SYM_VAL (sda);
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (output_bfd, sec->output_section);
 	    if (! (CONST_STRNEQ (name, ".sdata2")
 		   || CONST_STRNEQ (name, ".sbss2")))
 	      {
@@ -7998,7 +8001,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
 		break;
 	      }
 
-	    name = bfd_get_section_name (abfd, sec->output_section);
+	    name = bfd_get_section_name (output_bfd, sec->output_section);
 	    if (((CONST_STRNEQ (name, ".sdata")
 		  && (name[6] == 0 || name[6] == '.'))
 		 || (CONST_STRNEQ (name, ".sbss")
diff --git a/bfd/elf64-mmix.c b/bfd/elf64-mmix.c
index fd5921a..ae4b0fa 100644
--- a/bfd/elf64-mmix.c
+++ b/bfd/elf64-mmix.c
@@ -1771,7 +1771,9 @@ mmix_final_link_relocate (reloc_howto_type *howto, asection *input_section,
 	first_global = 255;
       else
 	{
-	  first_global = bfd_get_section_vma (abfd, regsec) / 8;
+	  first_global
+	    = bfd_get_section_vma (input_section->output_section->owner,
+				   regsec) / 8;
 	  if (strcmp (bfd_get_section_name (symsec->owner, symsec),
 		      MMIX_REG_CONTENTS_SECTION_NAME) == 0)
 	    {
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 0d6dd99..7c7beb6 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -4245,7 +4245,7 @@ create_linkage_sections (bfd *dynobj, struct bfd_link_info *info)
 								 ".eh_frame",
 								 flags);
       if (htab->glink_eh_frame == NULL
-	  || !bfd_set_section_alignment (abfd, htab->glink_eh_frame, 2))
+	  || !bfd_set_section_alignment (dynobj, htab->glink_eh_frame, 2))
 	return FALSE;
     }
 


-- 
Sergio

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Sergio Durigan Junior]

Ping.

On Friday, May 04 2012, I wrote:

> On Thursday, May 03 2012, I wrote:
>
>> On Thursday, May 03 2012, Alan Modra wrote:
>>
>>> On Thu, May 03, 2012 at 01:19:21AM -0300, Sergio Durigan Junior wrote:
>>>> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
>>>> 	bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
>>>> 	bfd_section_size, bfd_get_section_flags,
>>>> 	bfd_get_section_userdata): Rewrite macros in order to use the
>>>> 	`bfd' argument.
>>>> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
>>>> 	as the first argument for `bfd_get_section_alignment'.
>>>> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
>>>> 	`bfd_set_section_alignment'.
>>>
>>> The above is OK.  You guessed wrongly for all the bfd args below,
>>> except in elf64-ppc.c.  I suggest expanding the macros instead.  I've
>>> noted the correct bfds below but IMO these macros serve no useful
>>> purpose.
>>
>> Thanks for the review.  I've chosen to update the patch as proposed
>> below, instead of expanding the macros as you suggested.  I'm involved
>> in other tasks and it would take much more time, I'm afraid.
>>
>> WDYT of the patch below?
>
> I did something that I should have done before: I built binutils with my
> patch :-).  As it turns out, it's better not to fix `bfd_section_name'
> and `bfd_section_size', so here's the new patch which builds correctly
> on both binutils and GDB.  OK to apply?
>
> 2012-05-04  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
> 	bfd_get_section_lma, bfd_get_section_alignment,
> 	bfd_get_section_flags,
> 	bfd_get_section_userdata): Rewrite macros in order to use the
> 	`bfd' argument.
> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
> 	as the first argument for `bfd_get_section_alignment'.
> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
> 	`bfd_set_section_alignment'.
> 	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
> 	`bfd_get_section_name'.
> 	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
> 	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
> 	(ppc_elf_relocate_section): Likewise.
> 	* elf64-mmix.c (mmix_final_link_relocate): Likewise, for
> 	`bfd_get_section_vma'.
> 	* elf64-ppc.c (create_linkage_sections): Likewise, for
> 	`bfd_set_section_alignment'.
>
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 6b94f72..c7cee6d 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -280,18 +280,19 @@ alent;
>  
>  typedef struct bfd_section *sec_ptr;
>  
> -#define bfd_get_section_name(bfd, ptr) ((ptr)->name + 0)
> -#define bfd_get_section_vma(bfd, ptr) ((ptr)->vma + 0)
> -#define bfd_get_section_lma(bfd, ptr) ((ptr)->lma + 0)
> -#define bfd_get_section_alignment(bfd, ptr) ((ptr)->alignment_power + 0)
> +#define bfd_get_section_name(bfd, ptr) ((void) bfd, (ptr)->name)
> +#define bfd_get_section_vma(bfd, ptr) ((void) bfd, (ptr)->vma)
> +#define bfd_get_section_lma(bfd, ptr) ((void) bfd, (ptr)->lma)
> +#define bfd_get_section_alignment(bfd, ptr) ((void) bfd, \
> +					     (ptr)->alignment_power)
>  #define bfd_section_name(bfd, ptr) ((ptr)->name)
>  #define bfd_section_size(bfd, ptr) ((ptr)->size)
>  #define bfd_get_section_size(ptr) ((ptr)->size)
>  #define bfd_section_vma(bfd, ptr) ((ptr)->vma)
>  #define bfd_section_lma(bfd, ptr) ((ptr)->lma)
>  #define bfd_section_alignment(bfd, ptr) ((ptr)->alignment_power)
> -#define bfd_get_section_flags(bfd, ptr) ((ptr)->flags + 0)
> -#define bfd_get_section_userdata(bfd, ptr) ((ptr)->userdata)
> +#define bfd_get_section_flags(bfd, ptr) ((void) bfd, (ptr)->flags)
> +#define bfd_get_section_userdata(bfd, ptr) ((void) bfd, (ptr)->userdata)
>  
>  #define bfd_is_com_section(ptr) (((ptr)->flags & SEC_IS_COMMON) != 0)
>  
> diff --git a/bfd/elf-vxworks.c b/bfd/elf-vxworks.c
> index 06edf8d..ebd8f2b 100644
> --- a/bfd/elf-vxworks.c
> +++ b/bfd/elf-vxworks.c
> @@ -280,7 +280,8 @@ elf_vxworks_finish_dynamic_entry (bfd *output_bfd, Elf_Internal_Dyn *dyn)
>      case DT_VX_WRS_TLS_DATA_ALIGN:
>        sec = bfd_get_section_by_name (output_bfd, ".tls_data");
>        dyn->d_un.d_val
> -	= (bfd_size_type)1 << bfd_get_section_alignment (abfd, sec);
> +	= (bfd_size_type)1 << bfd_get_section_alignment (output_bfd,
> +							 sec);
>        break;
>        
>      case DT_VX_WRS_TLS_VARS_START:
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index f5bad39..ae670f1 100644
> --- a/bfd/elf32-arm.c
> +++ b/bfd/elf32-arm.c
> @@ -3258,7 +3258,7 @@ create_ifunc_sections (struct bfd_link_info *info)
>        s = bfd_make_section_with_flags (dynobj, ".iplt",
>  				       flags | SEC_READONLY | SEC_CODE);
>        if (s == NULL
> -	  || !bfd_set_section_alignment (abfd, s, bed->plt_alignment))
> +	  || !bfd_set_section_alignment (dynobj, s, bed->plt_alignment))
>  	return FALSE;
>        htab->root.iplt = s;
>      }
> @@ -3268,7 +3268,7 @@ create_ifunc_sections (struct bfd_link_info *info)
>        s = bfd_make_section_with_flags (dynobj, RELOC_SECTION (htab, ".iplt"),
>  				       flags | SEC_READONLY);
>        if (s == NULL
> -	  || !bfd_set_section_alignment (abfd, s, bed->s->log_file_align))
> +	  || !bfd_set_section_alignment (dynobj, s, bed->s->log_file_align))
>  	return FALSE;
>        htab->root.irelplt = s;
>      }
> diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
> index 7ab7b60..4f06354 100644
> --- a/bfd/elf32-m32r.c
> +++ b/bfd/elf32-m32r.c
> @@ -3007,7 +3007,7 @@ m32r_elf_relocate_section (bfd *output_bfd ATTRIBUTE_UNUSED,
>  		const char *name;
>  
>  		BFD_ASSERT (sec != NULL);
> -		name = bfd_get_section_name (abfd, sec);
> +		name = bfd_get_section_name (sec->owner, sec);
>  
>  		if (   strcmp (name, ".sdata") == 0
>  		    || strcmp (name, ".sbss") == 0
> diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
> index a58f7b3..5a20f0d 100644
> --- a/bfd/elf32-microblaze.c
> +++ b/bfd/elf32-microblaze.c
> @@ -839,7 +839,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  		/* Only relocate if the symbol is defined.  */
>  		if (sec)
>  		  {
> -		    name = bfd_get_section_name (abfd, sec);
> +		    name = bfd_get_section_name (sec->owner, sec);
>  
>  		    if (strcmp (name, ".sdata2") == 0
>  			|| strcmp (name, ".sbss2") == 0)
> @@ -868,7 +868,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  					       bfd_get_filename (input_bfd),
>  					       sym_name,
>  					       microblaze_elf_howto_table[(int) r_type]->name,
> -					       bfd_get_section_name (abfd, sec));
> +					       bfd_get_section_name (sec->owner, sec));
>  			/*bfd_set_error (bfd_error_bad_value); ??? why? */
>  			ret = FALSE;
>  			continue;
> @@ -884,7 +884,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  		/* Only relocate if the symbol is defined.  */
>  		if (sec)
>  		  {
> -		    name = bfd_get_section_name (abfd, sec);
> +		    name = bfd_get_section_name (sec->owner, sec);
>  
>  		    if (strcmp (name, ".sdata") == 0
>  			|| strcmp (name, ".sbss") == 0)
> @@ -913,7 +913,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>  					       bfd_get_filename (input_bfd),
>  					       sym_name,
>  					       microblaze_elf_howto_table[(int) r_type]->name,
> -					       bfd_get_section_name (abfd, sec));
> +					       bfd_get_section_name (sec->owner, sec));
>  			/*bfd_set_error (bfd_error_bad_value); ??? why? */
>  			ret = FALSE;
>  			continue;
> diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
> index ca6df26..f5df17e 100644
> --- a/bfd/elf32-ppc.c
> +++ b/bfd/elf32-ppc.c
> @@ -5867,7 +5867,8 @@ ppc_elf_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
>  	{
>  	  /* Strip these too.  */
>  	}
> -      else if (CONST_STRNEQ (bfd_get_section_name (dynobj, s), ".rela"))
> +      else if (CONST_STRNEQ (bfd_get_section_name (htab->elf.dynobj, s),
> +			     ".rela"))
>  	{
>  	  if (s->size != 0)
>  	    {
> @@ -7886,8 +7887,10 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  	      unresolved_reloc = TRUE;
>  	      break;
>  	    }
> -	  BFD_ASSERT (strcmp (bfd_get_section_name (abfd, sec), ".got") == 0
> -		      || strcmp (bfd_get_section_name (abfd, sec), ".cgot") == 0);
> +	  BFD_ASSERT (strcmp (bfd_get_section_name (sec->owner, sec),
> +			      ".got") == 0
> +		      || strcmp (bfd_get_section_name (sec->owner, sec),
> +				 ".cgot") == 0);
>  
>  	  addend -= sec->output_section->vma + sec->output_offset + 0x8000;
>  	  break;
> @@ -7937,7 +7940,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  	      }
>  	    addend -= SYM_VAL (sda);
>  
> -	    name = bfd_get_section_name (abfd, sec->output_section);
> +	    name = bfd_get_section_name (output_bfd, sec->output_section);
>  	    if (! ((CONST_STRNEQ (name, ".sdata")
>  		    && (name[6] == 0 || name[6] == '.'))
>  		   || (CONST_STRNEQ (name, ".sbss")
> @@ -7969,7 +7972,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  	      }
>  	    addend -= SYM_VAL (sda);
>  
> -	    name = bfd_get_section_name (abfd, sec->output_section);
> +	    name = bfd_get_section_name (output_bfd, sec->output_section);
>  	    if (! (CONST_STRNEQ (name, ".sdata2")
>  		   || CONST_STRNEQ (name, ".sbss2")))
>  	      {
> @@ -7998,7 +8001,7 @@ ppc_elf_relocate_section (bfd *output_bfd,
>  		break;
>  	      }
>  
> -	    name = bfd_get_section_name (abfd, sec->output_section);
> +	    name = bfd_get_section_name (output_bfd, sec->output_section);
>  	    if (((CONST_STRNEQ (name, ".sdata")
>  		  && (name[6] == 0 || name[6] == '.'))
>  		 || (CONST_STRNEQ (name, ".sbss")
> diff --git a/bfd/elf64-mmix.c b/bfd/elf64-mmix.c
> index fd5921a..ae4b0fa 100644
> --- a/bfd/elf64-mmix.c
> +++ b/bfd/elf64-mmix.c
> @@ -1771,7 +1771,9 @@ mmix_final_link_relocate (reloc_howto_type *howto, asection *input_section,
>  	first_global = 255;
>        else
>  	{
> -	  first_global = bfd_get_section_vma (abfd, regsec) / 8;
> +	  first_global
> +	    = bfd_get_section_vma (input_section->output_section->owner,
> +				   regsec) / 8;
>  	  if (strcmp (bfd_get_section_name (symsec->owner, symsec),
>  		      MMIX_REG_CONTENTS_SECTION_NAME) == 0)
>  	    {
> diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
> index 0d6dd99..7c7beb6 100644
> --- a/bfd/elf64-ppc.c
> +++ b/bfd/elf64-ppc.c
> @@ -4245,7 +4245,7 @@ create_linkage_sections (bfd *dynobj, struct bfd_link_info *info)
>  								 ".eh_frame",
>  								 flags);
>        if (htab->glink_eh_frame == NULL
> -	  || !bfd_set_section_alignment (abfd, htab->glink_eh_frame, 2))
> +	  || !bfd_set_section_alignment (dynobj, htab->glink_eh_frame, 2))
>  	return FALSE;
>      }
>  
>
>
> -- 
> Sergio

-- 
Sergio

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[nick clifton]

Hi Sergio,

> 2012-05-04  Sergio Durigan Junior<sergiodj@redhat.com>
>
> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
> 	bfd_get_section_lma, bfd_get_section_alignment,
> 	bfd_get_section_flags,
> 	bfd_get_section_userdata): Rewrite macros in order to use the
> 	`bfd' argument.
> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
> 	as the first argument for `bfd_get_section_alignment'.
> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
> 	`bfd_set_section_alignment'.
> 	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
> 	`bfd_get_section_name'.
> 	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
> 	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
> 	(ppc_elf_relocate_section): Likewise.
> 	* elf64-mmix.c (mmix_final_link_relocate): Likewise, for
> 	`bfd_get_section_vma'.
> 	* elf64-ppc.c (create_linkage_sections): Likewise, for
> 	`bfd_set_section_alignment'.

I have applied this patch.  There were however a few problems with it:

   * You modified bfd-in2.h, an auto-generated file, but not bfd-in.h, 
the file from which it is generated.

   * You missed a use of bfd_get_section_name() in bfd/elf32-ppc.c.

   * You missed a use of bfd_get_section_vma() in gas/config/tc-alpha.c.

   * You missed two uses of bfd_get_section_name() in 
ld/emultempl/m68hc1xelf.em.

These problems have been fixed.  For future reference I recommend 
building a toolchain configured as "--enable-64-bit-bfd 
--enable-targets=all" in order to catch problems like these.

Cheers
   Nick

Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros

[Sergio Durigan Junior]

On Wednesday, May 16 2012, nick clifton wrote:

> Hi Sergio,
>
>> 2012-05-04  Sergio Durigan Junior<sergiodj@redhat.com>
>>
>> 	* bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
>> 	bfd_get_section_lma, bfd_get_section_alignment,
>> 	bfd_get_section_flags,
>> 	bfd_get_section_userdata): Rewrite macros in order to use the
>> 	`bfd' argument.
>> 	* elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
>> 	as the first argument for `bfd_get_section_alignment'.
>> 	* elf32-arm.c (create_ifunc_sections): Likewise, for
>> 	`bfd_set_section_alignment'.
>> 	* elf32-m32r.c (m32r_elf_relocate_section): Likewise, for
>> 	`bfd_get_section_name'.
>> 	* elf32-microblaze.c (microblaze_elf_relocate_section): Likewise.
>> 	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Likewise.
>> 	(ppc_elf_relocate_section): Likewise.
>> 	* elf64-mmix.c (mmix_final_link_relocate): Likewise, for
>> 	`bfd_get_section_vma'.
>> 	* elf64-ppc.c (create_linkage_sections): Likewise, for
>> 	`bfd_set_section_alignment'.
>
> I have applied this patch.  There were however a few problems with it:

Hello Nick,

Thanks a lot for pointing out the errors.  I really did not test using
--enable-targets=all (somehow I thought this flag was specific to
GDB!).  Anyway, this won't happen again.

Thanks a lot,

-- 
Sergio