[PATCH v4 02/22] gas: move code for object attribute parsing into attr.c

Matthieu Longo matthieu.longo@arm.com
Fri Jul 4 13:55:19 GMT 2025


On 2025-07-04 10:18, Jan Beulich wrote:
> On 03.07.2025 18:27, Matthieu Longo wrote:
>> ---
> 
> Please can you try to get used to having non-empty descriptions unless a
> patch is really small and straightforward and doesn't leave open the
> possibility for someone asking why a certain change is being done? (It's
> really the "why" here that imo would deserve a sentence or two.)
> 
>>   gas/Makefile.am      |   2 +
>>   gas/Makefile.in      |  11 +-
>>   gas/attr.c           | 232 +++++++++++++++++++++++++++++++++++++++++++
>>   gas/attr.h           |  33 ++++++
>>   gas/config/obj-elf.c | 210 +--------------------------------------
>>   gas/config/obj-elf.h |   4 +-
>>   gas/po/POTFILES.in   |   2 +
>>   7 files changed, 278 insertions(+), 216 deletions(-)
>>   create mode 100644 gas/attr.c
>>   create mode 100644 gas/attr.h
> 
> Seeing where the code moves from, and (according to an answer to an earlier
> question I raised) there being no plans to extend this attribute machinery to
> non-ELF, the files probably want naming elf-attr.[ch] or elf-obj-attr.[ch].
> And at least the .c file's contents wants to be guarded by an OBJ_ELF /
> OBJ_MAYBE_ELF conditional (to avoid adding entirely dead code to assemblers
> for non-ELF targets).
> 
> Jan

Disclaimer: I will have a look at others comments later.

Regarding this one particularly, Richard Earnshaw suggested to me to 
move those files into gas/config. It does not change the need for 
#ifdefery inside the header to disable the prototypes, and in others 
parts of the code using the API.

Here is a draft of what it look like. You can apply the patch on top of 
the latest commit of the patch series.

It seems to me that this mechanism to include/exclude files is more 
adapted to a target which adds a few specific files in order to split 
its target code logic into different files, but those files should be 
unused by others targets.

What do you think of this approach ?
Should I keep the files in ./gas or move them to ./gas/config ?

Regarding the name of those files, what about obj-elf-attr.[h|c] ? 
(wherever they are located)

Matthieu

---
  gas/Makefile.am                       |  4 ++--
  gas/Makefile.in                       | 16 +++++++++-------
  gas/{attr.c => config/obj-elf-attr.c} |  2 +-
  gas/{attr.h => config/obj-elf-attr.h} |  0
  gas/config/obj-elf.c                  |  2 +-
  gas/config/obj-elf.h                  |  2 +-
  gas/configure                         | 13 +++++++++++++
  gas/configure.ac                      | 13 +++++++++++++
  8 files changed, 40 insertions(+), 12 deletions(-)
  rename gas/{attr.c => config/obj-elf-attr.c} (99%)
  rename gas/{attr.h => config/obj-elf-attr.h} (100%)

diff --git a/gas/Makefile.am b/gas/Makefile.am
index 1b5fba55a4b..f9b2e6891ab 100644
--- a/gas/Makefile.am
+++ b/gas/Makefile.am
@@ -68,7 +68,6 @@ GAS_CFILES = \
  	app.c \
  	as.c \
  	atof-generic.c \
-	attr.c \
  	codeview.c \
  	compress-debug.c \
  	cond.c \
@@ -108,7 +107,6 @@ CFILES = $(GAS_CFILES) itbl-ops.c cgen.c
  HFILES = \
  	as.h \
  	asintl.h \
-	attr.h \
  	bignum.h \
  	bit_fix.h \
  	cgen.h \
@@ -293,6 +291,8 @@ TARGET_CPU_HFILES = \

  TARGET_EXTRA_FILES = \
  	config/bfin-lex-wrapper.c \
+	config/obj-elf-attr.c \
+	config/obj-elf-attr.h \
  	config/xtensa-relax.c \
  	config/xtensa-relax.h \
  	config/kvx-parse.h \
diff --git a/gas/Makefile.in b/gas/Makefile.in
index 9cbb5992c53..741621206da 100644
--- a/gas/Makefile.in
+++ b/gas/Makefile.in
@@ -168,10 +168,10 @@ CONFIG_CLEAN_FILES = gdb.ini .gdbinit po/Makefile.in
  CONFIG_CLEAN_VPATH_FILES =
  PROGRAMS = $(noinst_PROGRAMS)
  am__objects_1 = app.$(OBJEXT) as.$(OBJEXT) atof-generic.$(OBJEXT) \
-	attr.$(OBJEXT) codeview.$(OBJEXT) compress-debug.$(OBJEXT) \
-	cond.$(OBJEXT) depend.$(OBJEXT) dwarf2dbg.$(OBJEXT) \
-	dw2gencfi.$(OBJEXT) ecoff.$(OBJEXT) ehopt.$(OBJEXT) \
-	expr.$(OBJEXT) flonum-copy.$(OBJEXT) flonum-konst.$(OBJEXT) \
+	codeview.$(OBJEXT) compress-debug.$(OBJEXT) cond.$(OBJEXT) \
+	depend.$(OBJEXT) dwarf2dbg.$(OBJEXT) dw2gencfi.$(OBJEXT) \
+	ecoff.$(OBJEXT) ehopt.$(OBJEXT) expr.$(OBJEXT) \
+	flonum-copy.$(OBJEXT) flonum-konst.$(OBJEXT) \
  	flonum-mult.$(OBJEXT) frags.$(OBJEXT) gen-sframe.$(OBJEXT) \
  	ginsn.$(OBJEXT) hash.$(OBJEXT) input-file.$(OBJEXT) \
  	input-scrub.$(OBJEXT) listing.$(OBJEXT) literal.$(OBJEXT) \
@@ -568,7 +568,6 @@ GAS_CFILES = \
  	app.c \
  	as.c \
  	atof-generic.c \
-	attr.c \
  	codeview.c \
  	compress-debug.c \
  	cond.c \
@@ -607,7 +606,6 @@ CFILES = $(GAS_CFILES) itbl-ops.c cgen.c
  HFILES = \
  	as.h \
  	asintl.h \
-	attr.h \
  	bignum.h \
  	bit_fix.h \
  	cgen.h \
@@ -792,6 +790,8 @@ TARGET_CPU_HFILES = \

  TARGET_EXTRA_FILES = \
  	config/bfin-lex-wrapper.c \
+	config/obj-elf-attr.c \
+	config/obj-elf-attr.h \
  	config/xtensa-relax.c \
  	config/xtensa-relax.h \
  	config/kvx-parse.h \
@@ -1246,6 +1246,8 @@ config/tc-z8k.$(OBJEXT): config/$(am__dirstamp) \
  	config/$(DEPDIR)/$(am__dirstamp)
  config/bfin-lex-wrapper.$(OBJEXT): config/$(am__dirstamp) \
  	config/$(DEPDIR)/$(am__dirstamp)
+config/obj-elf-attr.$(OBJEXT): config/$(am__dirstamp) \
+	config/$(DEPDIR)/$(am__dirstamp)
  config/xtensa-relax.$(OBJEXT): config/$(am__dirstamp) \
  	config/$(DEPDIR)/$(am__dirstamp)
  config/kvx-parse.$(OBJEXT): config/$(am__dirstamp) \
@@ -1313,7 +1315,6 @@ distclean-compile:
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/app.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/as.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@./$(DEPDIR)/atof-generic.Po@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/attr.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/cgen.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/codeview.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@./$(DEPDIR)/compress-debug.Po@am__quote@
@@ -1366,6 +1367,7 @@ distclean-compile:
  @AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-aout.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-coff.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-ecoff.Po@am__quote@
+@AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-elf-attr.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-elf.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-evax.Po@am__quote@
  @AMDEP_TRUE@@am__include@ 
@am__quote@config/$(DEPDIR)/obj-fdpicelf.Po@am__quote@
diff --git a/gas/attr.c b/gas/config/obj-elf-attr.c
similarity index 99%
rename from gas/attr.c
rename to gas/config/obj-elf-attr.c
index 48d15334530..c07ca27bcf3 100644
--- a/gas/attr.c
+++ b/gas/config/obj-elf-attr.c
@@ -18,7 +18,7 @@
     Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
     02110-1301, USA.  */

-#include "attr.h"
+#include "obj-elf-attr.h"

  #ifdef HAVE_OBJ_ATTR

diff --git a/gas/attr.h b/gas/config/obj-elf-attr.h
similarity index 100%
rename from gas/attr.h
rename to gas/config/obj-elf-attr.h
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 27f1eb323d1..e0119fc8121 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -25,7 +25,7 @@
  #include "obstack.h"
  #include "dwarf2dbg.h"
  #include "ginsn.h"
-#include "attr.h"
+#include "obj-elf-attr.h"

  #ifndef ECOFF_DEBUGGING
  #define ECOFF_DEBUGGING 0
diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
index 98368ac23c0..913921bfead 100644
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -205,7 +205,7 @@ extern void obj_elf_vtable_entry (int);
  extern struct fix * obj_elf_get_vtable_inherit (void);
  extern struct fix * obj_elf_get_vtable_entry (void);

-#include "attr.h"
+#include "obj-elf-attr.h"

  /* BFD wants to write the udata field, which is a no-no for the
     predefined section symbols in bfd/section.c.  They are read-only.  */
diff --git a/gas/configure b/gas/configure
index e39c965b465..a8267993da1 100755
--- a/gas/configure
+++ b/gas/configure
@@ -12349,6 +12349,19 @@ _ACEOF
  	;;
      esac

+    # Does the target support Object Attributes ?
+    case ${cpu_type} in
+      aarch64* | arc* | arm* | csky | m68k | mips* | msp430 | powerpc* \
+      | riscv* | s390* | sparc* | tic6x)
+	for f in config/obj-elf-attr.o; do
+	  case " $extra_objects " in
+	    *" $f "*) ;;
+	    *) extra_objects="$extra_objects $f" ;;
+	  esac
+	done
+	;;
+    esac
+
      # Any other special object files needed ?
      case ${cpu_type} in

diff --git a/gas/configure.ac b/gas/configure.ac
index e1d32c3b530..719f0039099 100644
--- a/gas/configure.ac
+++ b/gas/configure.ac
@@ -454,6 +454,19 @@ changequote([,])dnl
  	;;
      esac

+    # Does the target support Object Attributes ?
+    case ${cpu_type} in
+      aarch64* | arc* | arm* | csky | m68k | mips* | msp430 | powerpc* \
+      | riscv* | s390* | sparc* | tic6x)
+	for f in config/obj-elf-attr.o; do
+	  case " $extra_objects " in
+	    *" $f "*) ;;
+	    *) extra_objects="$extra_objects $f" ;;
+	  esac
+	done
+	;;
+    esac
+
      # Any other special object files needed ?
      case ${cpu_type} in

-- 
2.50.0



More information about the Binutils mailing list