diff mbox series

x86: Remove duplicated I386_PCREL_TYPE_P/X86_64_PCREL_TYPE_P

Message ID 20230104191414.149668-1-hjl.tools@gmail.com
State Repeat Merge
Headers
Series x86: Remove duplicated I386_PCREL_TYPE_P/X86_64_PCREL_TYPE_P |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

H.J. Lu Jan. 4, 2023, 7:14 p.m. UTC 
  I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
the duplications.

	* elfxx-x86.h (I386_PCREL_TYPE_P): Remove duplication.
	(X86_64_PCREL_TYPE_P): Likewise.
---
 bfd/elfxx-x86.h | 7 -------
 1 file changed, 7 deletions(-)

Comments

Jan Beulich Jan. 5, 2023, 7:42 a.m. UTC | #1 
On 04.01.2023 20:14, H.J. Lu via Binutils wrote:
> I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
> the duplications.

I recall noticing this as well, quite some time back, but I didn't feel
like touching it because I was puzzled by ...

> --- a/bfd/elfxx-x86.h
> +++ b/bfd/elfxx-x86.h
> @@ -97,13 +97,6 @@
>  #define PLT_FDE_START_OFFSET	4 + PLT_CIE_LENGTH + 8
>  #define PLT_FDE_LEN_OFFSET	4 + PLT_CIE_LENGTH + 12
>  
> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)

... this not including PC8 and PC16 when ...

> -#define X86_64_PCREL_TYPE_P(TYPE) \
> -  ((TYPE) == R_X86_64_PC8 \
> -   || (TYPE) == R_X86_64_PC16 \
> -   || (TYPE) == R_X86_64_PC32 \
> -   || (TYPE) == R_X86_64_PC64)

... this does.

Jan
H.J. Lu Jan. 5, 2023, 4:50 p.m. UTC | #2 
On Wed, Jan 4, 2023 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.01.2023 20:14, H.J. Lu via Binutils wrote:
> > I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
> > the duplications.
>
> I recall noticing this as well, quite some time back, but I didn't feel
> like touching it because I was puzzled by ...
>
> > --- a/bfd/elfxx-x86.h
> > +++ b/bfd/elfxx-x86.h
> > @@ -97,13 +97,6 @@
> >  #define PLT_FDE_START_OFFSET 4 + PLT_CIE_LENGTH + 8
> >  #define PLT_FDE_LEN_OFFSET   4 + PLT_CIE_LENGTH + 12
> >
> > -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
>
> ... this not including PC8 and PC16 when ...

This is I386_PCREL_TYPE_P.

> > -#define X86_64_PCREL_TYPE_P(TYPE) \
> > -  ((TYPE) == R_X86_64_PC8 \
> > -   || (TYPE) == R_X86_64_PC16 \
> > -   || (TYPE) == R_X86_64_PC32 \
> > -   || (TYPE) == R_X86_64_PC64)
>
> ... this does.

This is X86_64_PCREL_TYPE_P, not I386_PCREL_TYPE_P.

> Jan

The current ones have

#define X86_64_PCREL_TYPE_P(TYPE) \
  ((TYPE) == R_X86_64_PC8 \
   || (TYPE) == R_X86_64_PC16 \
   || (TYPE) == R_X86_64_PC32 \
   || (TYPE) == R_X86_64_PC64)
#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)

and the ones I removed are

-#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
-#define X86_64_PCREL_TYPE_P(TYPE) \
-  ((TYPE) == R_X86_64_PC8 \
-   || (TYPE) == R_X86_64_PC16 \
-   || (TYPE) == R_X86_64_PC32 \
-   || (TYPE) == R_X86_64_PC64)

They are identical.
Jan Beulich Jan. 5, 2023, 4:52 p.m. UTC | #3 
On 05.01.2023 17:50, H.J. Lu wrote:
> On Wed, Jan 4, 2023 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 04.01.2023 20:14, H.J. Lu via Binutils wrote:
>>> I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
>>> the duplications.
>>
>> I recall noticing this as well, quite some time back, but I didn't feel
>> like touching it because I was puzzled by ...
>>
>>> --- a/bfd/elfxx-x86.h
>>> +++ b/bfd/elfxx-x86.h
>>> @@ -97,13 +97,6 @@
>>>  #define PLT_FDE_START_OFFSET 4 + PLT_CIE_LENGTH + 8
>>>  #define PLT_FDE_LEN_OFFSET   4 + PLT_CIE_LENGTH + 12
>>>
>>> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
>>
>> ... this not including PC8 and PC16 when ...
> 
> This is I386_PCREL_TYPE_P.
> 
>>> -#define X86_64_PCREL_TYPE_P(TYPE) \
>>> -  ((TYPE) == R_X86_64_PC8 \
>>> -   || (TYPE) == R_X86_64_PC16 \
>>> -   || (TYPE) == R_X86_64_PC32 \
>>> -   || (TYPE) == R_X86_64_PC64)
>>
>> ... this does.
> 
> This is X86_64_PCREL_TYPE_P, not I386_PCREL_TYPE_P.
> 
>> Jan
> 
> The current ones have
> 
> #define X86_64_PCREL_TYPE_P(TYPE) \
>   ((TYPE) == R_X86_64_PC8 \
>    || (TYPE) == R_X86_64_PC16 \
>    || (TYPE) == R_X86_64_PC32 \
>    || (TYPE) == R_X86_64_PC64)
> #define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> 
> and the ones I removed are
> 
> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> -#define X86_64_PCREL_TYPE_P(TYPE) \
> -  ((TYPE) == R_X86_64_PC8 \
> -   || (TYPE) == R_X86_64_PC16 \
> -   || (TYPE) == R_X86_64_PC32 \
> -   || (TYPE) == R_X86_64_PC64)
> 
> They are identical.

That wasn't the question, though. I really did ask about the 32-bit vs
64-bit difference, which looks suspect to me.

Jan
H.J. Lu Jan. 5, 2023, 4:55 p.m. UTC | #4 
On Thu, Jan 5, 2023 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.01.2023 17:50, H.J. Lu wrote:
> > On Wed, Jan 4, 2023 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 04.01.2023 20:14, H.J. Lu via Binutils wrote:
> >>> I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
> >>> the duplications.
> >>
> >> I recall noticing this as well, quite some time back, but I didn't feel
> >> like touching it because I was puzzled by ...
> >>
> >>> --- a/bfd/elfxx-x86.h
> >>> +++ b/bfd/elfxx-x86.h
> >>> @@ -97,13 +97,6 @@
> >>>  #define PLT_FDE_START_OFFSET 4 + PLT_CIE_LENGTH + 8
> >>>  #define PLT_FDE_LEN_OFFSET   4 + PLT_CIE_LENGTH + 12
> >>>
> >>> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> >>
> >> ... this not including PC8 and PC16 when ...
> >
> > This is I386_PCREL_TYPE_P.
> >
> >>> -#define X86_64_PCREL_TYPE_P(TYPE) \
> >>> -  ((TYPE) == R_X86_64_PC8 \
> >>> -   || (TYPE) == R_X86_64_PC16 \
> >>> -   || (TYPE) == R_X86_64_PC32 \
> >>> -   || (TYPE) == R_X86_64_PC64)
> >>
> >> ... this does.
> >
> > This is X86_64_PCREL_TYPE_P, not I386_PCREL_TYPE_P.
> >
> >> Jan
> >
> > The current ones have
> >
> > #define X86_64_PCREL_TYPE_P(TYPE) \
> >   ((TYPE) == R_X86_64_PC8 \
> >    || (TYPE) == R_X86_64_PC16 \
> >    || (TYPE) == R_X86_64_PC32 \
> >    || (TYPE) == R_X86_64_PC64)
> > #define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> >
> > and the ones I removed are
> >
> > -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> > -#define X86_64_PCREL_TYPE_P(TYPE) \
> > -  ((TYPE) == R_X86_64_PC8 \
> > -   || (TYPE) == R_X86_64_PC16 \
> > -   || (TYPE) == R_X86_64_PC32 \
> > -   || (TYPE) == R_X86_64_PC64)
> >
> > They are identical.
>
> That wasn't the question, though. I really did ask about the 32-bit vs
> 64-bit difference, which looks suspect to me.
>

R_386_PC8 and R_386_PC16 were never handled by linker.
Jan Beulich Jan. 5, 2023, 5:01 p.m. UTC | #5 
On 05.01.2023 17:55, H.J. Lu wrote:
> On Thu, Jan 5, 2023 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 05.01.2023 17:50, H.J. Lu wrote:
>>> On Wed, Jan 4, 2023 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 04.01.2023 20:14, H.J. Lu via Binutils wrote:
>>>>> I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
>>>>> the duplications.
>>>>
>>>> I recall noticing this as well, quite some time back, but I didn't feel
>>>> like touching it because I was puzzled by ...
>>>>
>>>>> --- a/bfd/elfxx-x86.h
>>>>> +++ b/bfd/elfxx-x86.h
>>>>> @@ -97,13 +97,6 @@
>>>>>  #define PLT_FDE_START_OFFSET 4 + PLT_CIE_LENGTH + 8
>>>>>  #define PLT_FDE_LEN_OFFSET   4 + PLT_CIE_LENGTH + 12
>>>>>
>>>>> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
>>>>
>>>> ... this not including PC8 and PC16 when ...
>>>
>>> This is I386_PCREL_TYPE_P.
>>>
>>>>> -#define X86_64_PCREL_TYPE_P(TYPE) \
>>>>> -  ((TYPE) == R_X86_64_PC8 \
>>>>> -   || (TYPE) == R_X86_64_PC16 \
>>>>> -   || (TYPE) == R_X86_64_PC32 \
>>>>> -   || (TYPE) == R_X86_64_PC64)
>>>>
>>>> ... this does.
>>>
>>> This is X86_64_PCREL_TYPE_P, not I386_PCREL_TYPE_P.
>>>
>>>> Jan
>>>
>>> The current ones have
>>>
>>> #define X86_64_PCREL_TYPE_P(TYPE) \
>>>   ((TYPE) == R_X86_64_PC8 \
>>>    || (TYPE) == R_X86_64_PC16 \
>>>    || (TYPE) == R_X86_64_PC32 \
>>>    || (TYPE) == R_X86_64_PC64)
>>> #define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
>>>
>>> and the ones I removed are
>>>
>>> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
>>> -#define X86_64_PCREL_TYPE_P(TYPE) \
>>> -  ((TYPE) == R_X86_64_PC8 \
>>> -   || (TYPE) == R_X86_64_PC16 \
>>> -   || (TYPE) == R_X86_64_PC32 \
>>> -   || (TYPE) == R_X86_64_PC64)
>>>
>>> They are identical.
>>
>> That wasn't the question, though. I really did ask about the 32-bit vs
>> 64-bit difference, which looks suspect to me.
>>
> 
> R_386_PC8 and R_386_PC16 were never handled by linker.

May I then ask why that is (or, worded differently, why the two respective
types are handled for x86-64)? Is this just one of the many inconsistencies
that we have?

Jan
H.J. Lu Jan. 5, 2023, 5:03 p.m. UTC | #6 
On Thu, Jan 5, 2023 at 9:01 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.01.2023 17:55, H.J. Lu wrote:
> > On Thu, Jan 5, 2023 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 05.01.2023 17:50, H.J. Lu wrote:
> >>> On Wed, Jan 4, 2023 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 04.01.2023 20:14, H.J. Lu via Binutils wrote:
> >>>>> I386_PCREL_TYPE_P and X86_64_PCREL_TYPE_P are defined twice.  Remove
> >>>>> the duplications.
> >>>>
> >>>> I recall noticing this as well, quite some time back, but I didn't feel
> >>>> like touching it because I was puzzled by ...
> >>>>
> >>>>> --- a/bfd/elfxx-x86.h
> >>>>> +++ b/bfd/elfxx-x86.h
> >>>>> @@ -97,13 +97,6 @@
> >>>>>  #define PLT_FDE_START_OFFSET 4 + PLT_CIE_LENGTH + 8
> >>>>>  #define PLT_FDE_LEN_OFFSET   4 + PLT_CIE_LENGTH + 12
> >>>>>
> >>>>> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> >>>>
> >>>> ... this not including PC8 and PC16 when ...
> >>>
> >>> This is I386_PCREL_TYPE_P.
> >>>
> >>>>> -#define X86_64_PCREL_TYPE_P(TYPE) \
> >>>>> -  ((TYPE) == R_X86_64_PC8 \
> >>>>> -   || (TYPE) == R_X86_64_PC16 \
> >>>>> -   || (TYPE) == R_X86_64_PC32 \
> >>>>> -   || (TYPE) == R_X86_64_PC64)
> >>>>
> >>>> ... this does.
> >>>
> >>> This is X86_64_PCREL_TYPE_P, not I386_PCREL_TYPE_P.
> >>>
> >>>> Jan
> >>>
> >>> The current ones have
> >>>
> >>> #define X86_64_PCREL_TYPE_P(TYPE) \
> >>>   ((TYPE) == R_X86_64_PC8 \
> >>>    || (TYPE) == R_X86_64_PC16 \
> >>>    || (TYPE) == R_X86_64_PC32 \
> >>>    || (TYPE) == R_X86_64_PC64)
> >>> #define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> >>>
> >>> and the ones I removed are
> >>>
> >>> -#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
> >>> -#define X86_64_PCREL_TYPE_P(TYPE) \
> >>> -  ((TYPE) == R_X86_64_PC8 \
> >>> -   || (TYPE) == R_X86_64_PC16 \
> >>> -   || (TYPE) == R_X86_64_PC32 \
> >>> -   || (TYPE) == R_X86_64_PC64)
> >>>
> >>> They are identical.
> >>
> >> That wasn't the question, though. I really did ask about the 32-bit vs
> >> 64-bit difference, which looks suspect to me.
> >>
> >
> > R_386_PC8 and R_386_PC16 were never handled by linker.
>
> May I then ask why that is (or, worded differently, why the two respective
> types are handled for x86-64)? Is this just one of the many inconsistencies
> that we have?
>

I guess so.
diff mbox series

Patch

diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h
index e3f83b600ef..3b4644ca478 100644
--- a/bfd/elfxx-x86.h
+++ b/bfd/elfxx-x86.h
@@ -97,13 +97,6 @@ 
 #define PLT_FDE_START_OFFSET	4 + PLT_CIE_LENGTH + 8
 #define PLT_FDE_LEN_OFFSET	4 + PLT_CIE_LENGTH + 12
 
-#define I386_PCREL_TYPE_P(TYPE) ((TYPE) == R_386_PC32)
-#define X86_64_PCREL_TYPE_P(TYPE) \
-  ((TYPE) == R_X86_64_PC8 \
-   || (TYPE) == R_X86_64_PC16 \
-   || (TYPE) == R_X86_64_PC32 \
-   || (TYPE) == R_X86_64_PC64)
-
 /* This must be the same as sframe_get_hdr_size (sfh).  For x86-64, this value
    is the same as sizeof (sframe_header) because there is no SFrame auxilliary
    header.  */