[PATCH] bfd: add missing smclass when creating csect for xcoff64

[PATCH] bfd: add missing smclass when creating csect for xcoff64

[CHIGOT, CLEMENT]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

Hi everyone, 

I've started to refresh a bit the AIX port of binutils. 
I've a few patches that I guess can be integrated even if they might 
change in the future. I mean to say that they are stable but as most
of the tests are failing on AIX, I cannot ensure that I'm not doing any
mistakes. 
Tell me if you're okay with that or if you'd rather have me fixing all
tests and be sure that everything is working correctly before 
submitting any patch ?

This one is pretty simple, so I know that it won't change anyway. 

Sincerely,
Clément

[-- Attachment #2: 0001-bfd-add-missing-smclass-when-creating-csect-for-xcof.patch --]
[-- Type: application/octet-stream, Size: 1393 bytes --]

From 025760e988855470a6bd38fa637d40cc9441e1b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <clement.chigot@atos.net>
Date: Fri, 16 Oct 2020 13:49:23 +0200
Subject: [PATCH] bfd: add missing smclass when creating csect for xcoff64
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ChangeLog:
2020-10-16  Clément Chigot  <clement.chigot@atos.net>

	* bfd/coff64-rs6000.c (xcoff64_create_csect_from_smclas): Add
	missing smclass.
---
 bfd/coff64-rs6000.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bfd/coff64-rs6000.c b/bfd/coff64-rs6000.c
index 9aa0304ec9f..8cabf74acc4 100644
--- a/bfd/coff64-rs6000.c
+++ b/bfd/coff64-rs6000.c
@@ -2130,14 +2130,14 @@ xcoff64_create_csect_from_smclas (bfd *abfd, union internal_auxent *aux,
   /* Changes from 32 :
      .sv == 8, is only for 32 bit programs
      .ti == 12 and .tb == 13 are now reserved.  */
-  static const char *names[19] =
+  static const char * const names[] =
   {
     ".pr", ".ro", ".db", ".tc", ".ua", ".rw", ".gl", ".xo",
     NULL, ".bs", ".ds", ".uc", NULL,  NULL,  NULL,  ".tc0",
-    ".td", ".sv64", ".sv3264"
+    ".td", ".sv64", ".sv3264", NULL, ".tl", ".ul", ".te"
   };
 
-  if ((19 >= aux->x_csect.x_smclas)
+  if ((aux->x_csect.x_smclas < ARRAY_SIZE (names))
       && (NULL != names[aux->x_csect.x_smclas]))
     {
 
-- 
2.25.0

Re: [PATCH] bfd: add missing smclass when creating csect for xcoff64

[Alan Modra]

On Mon, Feb 15, 2021 at 03:03:02PM +0000, CHIGOT, CLEMENT via Binutils wrote:
> Hi everyone, 
> 
> I've started to refresh a bit the AIX port of binutils. 
> I've a few patches that I guess can be integrated even if they might 
> change in the future. I mean to say that they are stable but as most
> of the tests are failing on AIX, I cannot ensure that I'm not doing any
> mistakes. 
> Tell me if you're okay with that or if you'd rather have me fixing all
> tests and be sure that everything is working correctly before 
> submitting any patch ?

Patches should not cause regressions that's all, we don't expect you
to fix everything.

> This one is pretty simple, so I know that it won't change anyway. 

Maybe too simple.  You've added new sections without handling them at
all in aix.sc, and in the case of .tl and .ul I'm fairly sure you will
need to add quite a lot of code for thread-local support.  It is fine
to have small patches in a series, but this patch in isolation is
obviously incomplete.

So I'd like to see a little more before committing the patch.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] bfd: add missing smclass when creating csect for xcoff64

[CHIGOT, CLEMENT]

Hi Alan, 

> > This one is pretty simple, so I know that it won't change anyway. 
> 
> Maybe too simple.  You've added new sections without handling them at
> all in aix.sc, and in the case of .tl and .ul I'm fairly sure you will
> need to add quite a lot of code for thread-local support.  It is fine
> to have small patches in a series, but this patch in isolation is
> obviously incomplete.
> 
> So I'd like to see a little more before committing the patch.

Yes indeed, I've 6 patches coming up aiming to enable both TLS and large TOC
(not -bbigtoc from AIX ld, but the TOC generated when -mcmodel=large is passed
to gcc, making use of R_TOCU and R_TOCL relocations). 

I'll send them as a series. 

Clément

Re: [PATCH] bfd: add missing smclass when creating csect for xcoff64

[Alan Modra]

On Tue, Feb 16, 2021 at 09:31:08AM +0000, CHIGOT, CLEMENT wrote:
> Hi Alan, 
> 
> > > This one is pretty simple, so I know that it won't change anyway. 
> > 
> > Maybe too simple.  You've added new sections without handling them at
> > all in aix.sc, and in the case of .tl and .ul I'm fairly sure you will
> > need to add quite a lot of code for thread-local support.  It is fine
> > to have small patches in a series, but this patch in isolation is
> > obviously incomplete.
> > 
> > So I'd like to see a little more before committing the patch.
> 
> Yes indeed, I've 6 patches coming up aiming to enable both TLS and large TOC
> (not -bbigtoc from AIX ld, but the TOC generated when -mcmodel=large is passed
> to gcc, making use of R_TOCU and R_TOCL relocations). 
> 
> I'll send them as a series. 

OK, so the next question is whether you have a copyright assignment
agreement with the FSF for the binutils project?  That is needed for
non-trivial contributions to binutils.

Also, I see
+FAIL: XCOFF TOC reloc test 1
+FAIL: PowerPC Test 1, 32 bit XCOFF
in the gas testsuite and
+FAIL: Object for --just-symbols test
in the ld testsuite with your patches applied.

The last one, a segmentation fault in your new code, is easy to fix.
Didn't you run the testsuite?

The first two are related to a change I made with commit 3b8b57a949,
selecting the reloc type earlier during insn parsing rather than later
in md_apply_reloc.  Unfortunately that affected the powerpc xcoff
support with BFD_RELOC_16 not being changed to BFD_RELOC_PPC_TOC16 and
then _bfd_xcoff_reloc_type_lookup failing.  That's the origin of this
code
    case BFD_RELOC_16:
      /* Note that this relocation is only internally used by gas.  */
      return &xcoff_howto_table[0xc];
which is wrong with your change to the howtos.

However, git commit 0e2779e98dc means that handling BFD_RELOC_16
in reloc_type_lookup is no longer needed.  I'm going to apply the
following patch which will mean you don't need to worry about those
gas fails.

	* coff-rs6000.c (_bfd_xcoff_reloc_type_lookup): Remove BFD_RELOC_16.
	* coff64-rs6000.c (xcoff64_reloc_type_lookup): Likewise.

diff --git a/bfd/coff-rs6000.c b/bfd/coff-rs6000.c
index fbc1aed311..54fbf58e3a 100644
--- a/bfd/coff-rs6000.c
+++ b/bfd/coff-rs6000.c
@@ -1092,9 +1092,6 @@ _bfd_xcoff_reloc_type_lookup (bfd *abfd ATTRIBUTE_UNUSED,
       return &xcoff_howto_table[8];
     case BFD_RELOC_PPC_TOC16:
       return &xcoff_howto_table[3];
-    case BFD_RELOC_16:
-      /* Note that this relocation is only internally used by gas.  */
-      return &xcoff_howto_table[0xc];
     case BFD_RELOC_PPC_B16:
       return &xcoff_howto_table[0x1d];
     case BFD_RELOC_32:
diff --git a/bfd/coff64-rs6000.c b/bfd/coff64-rs6000.c
index 9aa0304ec9..98b0066822 100644
--- a/bfd/coff64-rs6000.c
+++ b/bfd/coff64-rs6000.c
@@ -1814,9 +1814,6 @@ xcoff64_reloc_type_lookup (bfd *abfd ATTRIBUTE_UNUSED,
       return &xcoff64_howto_table[8];
     case BFD_RELOC_PPC_TOC16:
       return &xcoff64_howto_table[3];
-    case BFD_RELOC_16:
-      /* Note that this relocation is only internally used by gas.  */
-      return &xcoff64_howto_table[0xc];
     case BFD_RELOC_PPC_B16:
       return &xcoff64_howto_table[0x1e];
     case BFD_RELOC_32:


-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] bfd: add missing smclass when creating csect for xcoff64

[CHIGOT, CLEMENT]

> OK, so the next question is whether you have a copyright assignment
> agreement with the FSF for the binutils project?  That is needed for
> non-trivial contributions to binutils.

No I don't have any and I didn't find the webpage explaining how
to proceed. Do you have it ?

> Also, I see
> +FAIL: XCOFF TOC reloc test 1
> +FAIL: PowerPC Test 1, 32 bit XCOFF
> in the gas testsuite and
> +FAIL: Object for --just-symbols test
> in the ld testsuite with your patches applied.
>
> The last one, a segmentation fault in your new code, is easy to fix.
> Didn't you run the testsuite?

It seems that I have missed them. As you might have seen, I've made
these patches two months ago and I don't remember them failing at
that time. I've rebased and checked quickly before opening this
discussion. I should have checked more, my bad...
Moreover, there is a lot of tests already failing in the testsuite
(not with gas actually). Thus, I'm "fixing" them in several other
patches. But I don't want to push them as I still don't know
the root cause.
I'll check your patch also.

Thanks,
Clément

Re: [PATCH] bfd: add missing smclass when creating csect for xcoff64

[Alan Modra]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On Tue, Feb 23, 2021 at 09:02:58AM +0000, CHIGOT, CLEMENT wrote:
> > OK, so the next question is whether you have a copyright assignment
> > agreement with the FSF for the binutils project?  That is needed for
> > non-trivial contributions to binutils.
> 
> No I don't have any and I didn't find the webpage explaining how
> to proceed. Do you have it ?

I've attached the instructions for requesting the correct assignment
forms.

-- 
Alan Modra
Australia Development Lab, IBM

[-- Attachment #2: request-assign.future --]
[-- Type: text/plain, Size: 965 bytes --]

Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]


[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]


[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]


[For the copyright registration, what country are you a citizen of?]


[What year were you born?]


[Please write your email address here.]


[Please write your postal address here.]





[Which files have you changed so far, and which new files have you written
so far?]