Commit: ld x86_64 tests: Accept x86-64-v3 as a needed ISA
Checks
Commit Message
Hi Guys,
Recently four of the x86_64 linker tests have started failing for the
Fedora rawhide binutils because the x86_64-v3 ISA is being used and
.note.gnu.property notes show:
x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v3
So I am applying the patch below to update the regexps for these
tests.
Cheers
Nick
ld/ChangeLog
2023-11-02 Nick Clifton <nickc@redhat.com>
* testsuite/ld-x86-64/property-3.r: Update regexp to allow for
targets which support x86-64-v3.
* testsuite/ld-x86-64/property-4.r: Likewise.
* testsuite/ld-x86-64/property-5.r: Likewise.
Comments
On 02.11.2023 10:57, Nick Clifton wrote:
> Hi Guys,
>
> Recently four of the x86_64 linker tests have started failing for the
> Fedora rawhide binutils because the x86_64-v3 ISA is being used and
> .note.gnu.property notes show:
>
> x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v3
Where's that coming from? At least ...
> --- a/ld/testsuite/ld-x86-64/property-3.r
> +++ b/ld/testsuite/ld-x86-64/property-3.r
> @@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
> GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
> Properties: stack size: 0x800000
> #...
> - x86 ISA needed: x86-64-baseline, x86-64-v2
> + x86 ISA needed: x86-64-baseline, x86-64-v2.*
... this change is suspicious at the first glance, as it raises the ISA
requirements for the resulting object. That shouldn't happen by default.
> --- a/ld/testsuite/ld-x86-64/property-4.r
> +++ b/ld/testsuite/ld-x86-64/property-4.r
> @@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
> GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
> Properties: stack size: 0x800000
> #...
> - x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v4
> + x86 ISA needed: x86-64-baseline, x86-64-v2,.*x86-64-v4
> #pass
> --- a/ld/testsuite/ld-x86-64/property-5.r
> +++ b/ld/testsuite/ld-x86-64/property-5.r
> @@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
> GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
> Properties: stack size: 0x900000
> #...
> - x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v4
> + x86 ISA needed: x86-64-baseline, x86-64-v2,.*x86-64-v4
> #pass
For these I wonder whether we should really demand v2 in the expectations.
It's the highest only which matters, so checking for just v4 ought to be
enough. H.J., can you please clarify what the original intentions here
were?
Jan
Nick Clifton <nickc@redhat.com> writes:
> Hi Guys,
>
> Recently four of the x86_64 linker tests have started failing for the
> Fedora rawhide binutils because the x86_64-v3 ISA is being used and
> .note.gnu.property notes show:
>
> x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v3
>
> So I am applying the patch below to update the regexps for these
> tests.
This is PR30722 which I'll close now, thanks!
>
> Cheers
> Nick
>
> ld/ChangeLog
> 2023-11-02 Nick Clifton <nickc@redhat.com>
>
> * testsuite/ld-x86-64/property-3.r: Update regexp to allow for
> targets which support x86-64-v3.
> * testsuite/ld-x86-64/property-4.r: Likewise.
> * testsuite/ld-x86-64/property-5.r: Likewise.
>
> diff --git a/ld/testsuite/ld-x86-64/property-3.r b/ld/testsuite/ld-x86-64/property-3.r
> index 1bdb47c7b6b..1e4a24c6fbe 100644
> --- a/ld/testsuite/ld-x86-64/property-3.r
> +++ b/ld/testsuite/ld-x86-64/property-3.r
> @@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
> GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
> Properties: stack size: 0x800000
> #...
> - x86 ISA needed: x86-64-baseline, x86-64-v2
> + x86 ISA needed: x86-64-baseline, x86-64-v2.*
> #pass
> diff --git a/ld/testsuite/ld-x86-64/property-4.r b/ld/testsuite/ld-x86-64/property-4.r
> index 8cc618cbfad..031a1d280ad 100644
> --- a/ld/testsuite/ld-x86-64/property-4.r
> +++ b/ld/testsuite/ld-x86-64/property-4.r
> @@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
> GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
> Properties: stack size: 0x800000
> #...
> - x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v4
> + x86 ISA needed: x86-64-baseline, x86-64-v2,.*x86-64-v4
> #pass
> diff --git a/ld/testsuite/ld-x86-64/property-5.r b/ld/testsuite/ld-x86-64/property-5.r
> index e3ba4f659b9..58b274b9772 100644
> --- a/ld/testsuite/ld-x86-64/property-5.r
> +++ b/ld/testsuite/ld-x86-64/property-5.r
> @@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
> GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
> Properties: stack size: 0x900000
> #...
> - x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v4
> + x86 ISA needed: x86-64-baseline, x86-64-v2,.*x86-64-v4
> #pass
Hi Jan,
>> x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v3
>
> Where's that coming from?
Have a look at:
https://bugzilla.redhat.com/show_bug.cgi?id=2247296
and also PR30722, as mentioned by Sam.
>> - x86 ISA needed: x86-64-baseline, x86-64-v2
>> + x86 ISA needed: x86-64-baseline, x86-64-v2.*
>
> ... this change is suspicious at the first glance, as it raises the ISA
> requirements for the resulting object. That shouldn't happen by default.
Agreed. I could not think of an easy-to-read way to format the
regexp so that it says "x86-64-v2 plus, optionally, other ISAs".
The change I made does work and does do this, but it does not read
that way.
Cheers
Nick
On 02.11.2023 13:23, Nick Clifton wrote:
>>> x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v3
>>
>> Where's that coming from?
>
> Have a look at:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2247296
>
> and also PR30722, as mentioned by Sam.
Sadly neither really makes clear (to me) where the v3 bit is coming from.
I suspect it's one of the crt*.o, yet then I'm also a little puzzled that
the .so output tests don't surface the same behavior.
>>> - x86 ISA needed: x86-64-baseline, x86-64-v2
>>> + x86 ISA needed: x86-64-baseline, x86-64-v2.*
>>
>> ... this change is suspicious at the first glance, as it raises the ISA
>> requirements for the resulting object. That shouldn't happen by default.
>
> Agreed. I could not think of an easy-to-read way to format the
> regexp so that it says "x86-64-v2 plus, optionally, other ISAs".
>
> The change I made does work and does do this, but it does not read
> that way.
Oh, no, me being suspicious isn't because of the way you've expressed
things. Instead you've broken the intended purpose of the test, I'm
afraid: What this is about - iiuc - is to check that the linker
correctly folds the respective notes from, in particular,
property-x86-1.o and property-x86-2.o. Relaxing the expectations as you
did means e.g. a bug in the linker wrongly folding v2+v4 into v2+v3+v4
would now go unnoticed.
If the v3 bit is coming from one of the crt*.o, then I would be inclined
to suspect a bug in glibc and/or gcc. Assuming one of these files indeed
uses some v3 feature, then surely a variant of the file(s) should be
available to link against, such that a baseline-only or v2-only binary
can be generated? Another aspect of course is that the testcases
themselves were, from the very beginning, not immune to notes also
coming from system-supplied object files (and if earlier on the distro
in question surfaced "v2 needed" notes, then that did compromise these
testcases already, just that the issue went silent). IOW perhaps it is
wrong to use the C compiler to do the linking here.
If the v3 bit was coming from other than the involved crt*.o, then I'd
be curious where/what that is.
In any event I'm of the opinion that the change wants reverting.
Jan
Hi Jan,
> Oh, no, me being suspicious isn't because of the way you've expressed
> things. Instead you've broken the intended purpose of the test, I'm
> afraid: What this is about - iiuc - is to check that the linker
> correctly folds the respective notes from, in particular,
> property-x86-1.o and property-x86-2.o. Relaxing the expectations as you
> did means e.g. a bug in the linker wrongly folding v2+v4 into v2+v3+v4
> would now go unnoticed.
Ah! I see now. Yes you are correct.
> In any event I'm of the opinion that the change wants reverting.
And the tests changed so that they do not involve any system object
files at all, right ?
Are you volunteering ? :-) If not then I will have a go myself.
Cheers
Nick
On 02.11.2023 14:00, Nick Clifton wrote:
>> Oh, no, me being suspicious isn't because of the way you've expressed
>> things. Instead you've broken the intended purpose of the test, I'm
>> afraid: What this is about - iiuc - is to check that the linker
>> correctly folds the respective notes from, in particular,
>> property-x86-1.o and property-x86-2.o. Relaxing the expectations as you
>> did means e.g. a bug in the linker wrongly folding v2+v4 into v2+v3+v4
>> would now go unnoticed.
>
> Ah! I see now. Yes you are correct.
>
>> In any event I'm of the opinion that the change wants reverting.
>
> And the tests changed so that they do not involve any system object
> files at all, right ?
>
> Are you volunteering ? :-) If not then I will have a go myself.
Well, preferably I'd like to "volunteer" H.J., as he introduced all this
machinery. Just that afaik he won't be around until the end of the month.
I wouldn't be able to tell when I might get around to try dealing with
this; not before end of next week for sure.
Jan
On 02.11.2023 14:06, Jan Beulich wrote:
> On 02.11.2023 14:00, Nick Clifton wrote:
>>> Oh, no, me being suspicious isn't because of the way you've expressed
>>> things. Instead you've broken the intended purpose of the test, I'm
>>> afraid: What this is about - iiuc - is to check that the linker
>>> correctly folds the respective notes from, in particular,
>>> property-x86-1.o and property-x86-2.o. Relaxing the expectations as you
>>> did means e.g. a bug in the linker wrongly folding v2+v4 into v2+v3+v4
>>> would now go unnoticed.
>>
>> Ah! I see now. Yes you are correct.
>>
>>> In any event I'm of the opinion that the change wants reverting.
>>
>> And the tests changed so that they do not involve any system object
>> files at all, right ?
>>
>> Are you volunteering ? :-) If not then I will have a go myself.
>
> Well, preferably I'd like to "volunteer" H.J., as he introduced all this
> machinery. Just that afaik he won't be around until the end of the month.
> I wouldn't be able to tell when I might get around to try dealing with
> this; not before end of next week for sure.
I thought I might as well try to get past this. Could you give the patch
below a try, with the other one reverted in addition? As said in the
description it deals with only the known problematic tests for now.
Jan
ld/x86: reduce testsuite dependency on system object files
PR ld/30722
Tests looking for certain .note-section recorded properties may not
involve object files from the underlying platform (e.g. via using the C
compiler for linking): Such object files may themselves have similar
note sections, and hence they may influence the overall outcome.
For now convert just the tests known to be affected by crt*.o coming
with "ISA v3 needed" notes. Eventually other tests ought to be
converted, too.
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/property-stk.s
@@ -0,0 +1,25 @@
+ .ifdef __64_bit__
+ .equ ALIGN, 3
+ .else
+ .equ ALIGN, 2
+ .endif
+
+ .section ".note.gnu.property", "a"
+ .p2align ALIGN
+ .long 1f - 0f /* name length. */
+ .long 3f - 1f /* data length. */
+ /* NT_GNU_PROPERTY_TYPE_0 */
+ .long 5 /* note type. */
+0:
+ .asciz "GNU" /* vendor name. */
+1:
+ .p2align ALIGN
+ /* GNU_PROPERTY_STACK_SIZE */
+ .long 1 /* pr_type. */
+ .long 5f - 4f /* pr_datasz. */
+4:
+ .dc.a 0x800000 /* Stack size. */
+5:
+ .p2align ALIGN
+3:
+ .section .note.GNU-stack
--- a/ld/testsuite/ld-x86-64/property-x86-1.S
+++ /dev/null
@@ -1,38 +0,0 @@
-#ifdef __LP64__
-# define ALIGN 3
-#else
-# define ALIGN 2
-#endif
- .section ".note.gnu.property", "a"
- .p2align ALIGN
- .long 1f - 0f /* name length. */
- .long 3f - 1f /* data length. */
- /* NT_GNU_PROPERTY_TYPE_0 */
- .long 5 /* note type. */
-0:
- .asciz "GNU" /* vendor name. */
-1:
- .p2align ALIGN
- /* GNU_PROPERTY_STACK_SIZE */
- .long 1 /* pr_type. */
- .long 5f - 4f /* pr_datasz. */
-4:
- .dc.a 0x600000 /* Stack size. */
-5:
- .p2align ALIGN
- /* GNU_PROPERTY_X86_ISA_1_USED */
- .long 0xc0010002 /* pr_type. */
- .long 5f - 4f /* pr_datasz. */
-4:
- .long 0xa
-5:
- .p2align ALIGN
- /* GNU_PROPERTY_X86_ISA_1_NEEDED */
- .long 0xc0008002 /* pr_type. */
- .long 5f - 4f /* pr_datasz. */
-4:
- .long 0x3
-5:
- .p2align ALIGN
-3:
- .section .note.GNU-stack
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/property-x86-1.s
@@ -0,0 +1,39 @@
+ .ifdef __64_bit__
+ .equ ALIGN, 3
+ .else
+ .equ ALIGN, 2
+ .endif
+
+ .section ".note.gnu.property", "a"
+ .p2align ALIGN
+ .long 1f - 0f /* name length. */
+ .long 3f - 1f /* data length. */
+ /* NT_GNU_PROPERTY_TYPE_0 */
+ .long 5 /* note type. */
+0:
+ .asciz "GNU" /* vendor name. */
+1:
+ .p2align ALIGN
+ /* GNU_PROPERTY_STACK_SIZE */
+ .long 1 /* pr_type. */
+ .long 5f - 4f /* pr_datasz. */
+4:
+ .dc.a 0x600000 /* Stack size. */
+5:
+ .p2align ALIGN
+ /* GNU_PROPERTY_X86_ISA_1_USED */
+ .long 0xc0010002 /* pr_type. */
+ .long 5f - 4f /* pr_datasz. */
+4:
+ .long 0xa
+5:
+ .p2align ALIGN
+ /* GNU_PROPERTY_X86_ISA_1_NEEDED */
+ .long 0xc0008002 /* pr_type. */
+ .long 5f - 4f /* pr_datasz. */
+4:
+ .long 0x3
+5:
+ .p2align ALIGN
+3:
+ .section .note.GNU-stack
--- a/ld/testsuite/ld-x86-64/property-x86-2.S
+++ /dev/null
@@ -1,31 +0,0 @@
-#ifdef __LP64__
-# define ALIGN 3
-#else
-# define ALIGN 2
-#endif
- .section ".note.gnu.property", "a"
- .p2align ALIGN
- .long 1f - 0f /* name length. */
- .long 3f - 1f /* data length. */
- /* NT_GNU_PROPERTY_TYPE_0 */
- .long 5 /* note type. */
-0:
- .asciz "GNU" /* vendor name. */
-1:
- .p2align ALIGN
- /* GNU_PROPERTY_X86_ISA_1_USED */
- .long 0xc0010002 /* pr_type. */
- .long 5f - 4f /* pr_datasz. */
-4:
- .long 0x3
-5:
- .p2align ALIGN
- /* GNU_PROPERTY_X86_ISA_1_NEEDED */
- .long 0xc0008002 /* pr_type. */
- .long 5f - 4f /* pr_datasz. */
-4:
- .long 0xa
-5:
- .p2align ALIGN
-3:
- .section .note.GNU-stack
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/property-x86-2.s
@@ -0,0 +1,32 @@
+ .ifdef __64_bit__
+ .equ ALIGN, 3
+ .else
+ .equ ALIGN, 2
+ .endif
+
+ .section ".note.gnu.property", "a"
+ .p2align ALIGN
+ .long 1f - 0f /* name length. */
+ .long 3f - 1f /* data length. */
+ /* NT_GNU_PROPERTY_TYPE_0 */
+ .long 5 /* note type. */
+0:
+ .asciz "GNU" /* vendor name. */
+1:
+ .p2align ALIGN
+ /* GNU_PROPERTY_X86_ISA_1_USED */
+ .long 0xc0010002 /* pr_type. */
+ .long 5f - 4f /* pr_datasz. */
+4:
+ .long 0x3
+5:
+ .p2align ALIGN
+ /* GNU_PROPERTY_X86_ISA_1_NEEDED */
+ .long 0xc0008002 /* pr_type. */
+ .long 5f - 4f /* pr_datasz. */
+4:
+ .long 0xa
+5:
+ .p2align ALIGN
+3:
+ .section .note.GNU-stack
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -1093,86 +1093,6 @@ if { [isnative] && [check_compiler_avail
"property-2.so" \
] \
[list \
- "Build property 3" \
- "" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-stack.S property-x86-1.S} \
- {{readelf {-n} property-3.r}} \
- "property-3" \
- ] \
- [list \
- "Build property 3 (.o)" \
- "-r -nostdlib" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-x86-1.S property-stack.S} \
- {{readelf {-n} property-3a.r}} \
- "property-3.o" \
- ] \
- [list \
- "Build property 3 (.so)" \
- "-shared" \
- "-fPIC -Wa,-mx86-used-note=yes" \
- {property-x86-1.S pass.c property-stack.S} \
- {{readelf {-n} property-3.r}} \
- "property-3.so" \
- ] \
- [list \
- "Build property 4" \
- "" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-stack.S property-x86-1.S property-x86-2.S} \
- {{readelf {-n} property-4.r}} \
- "property-4" \
- ] \
- [list \
- "Build property 4 (.o)" \
- "-r -nostdlib" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-x86-2.S property-x86-1.S property-stack.S} \
- {{readelf {-n} property-4a.r}} \
- "property-4.o" \
- ] \
- [list \
- "Build property 4 (.so)" \
- "-shared" \
- "-fPIC -Wa,-mx86-used-note=yes" \
- {property-x86-2.S property-x86-1.S pass.c property-stack.S} \
- {{readelf {-n} property-4.r}} \
- "property-4.so" \
- ] \
- [list \
- "Build property 4 (-Wl,-z,stack-size=0)" \
- "-Wl,-z,stack-size=0" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-stack.S property-x86-1.S property-x86-2.S} \
- {{readelf {-n} property-4.r}} \
- "property-4" \
- ] \
- [list \
- "Build property 5" \
- "-Wl,-z,stack-size=0x900000" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-stack.S property-x86-1.S property-x86-2.S} \
- {{readelf {-n} property-5.r}} \
- "property-5" \
- ] \
- [list \
- "Build property 5 (.o)" \
- "-r -nostdlib -Wl,-z,stack-size=0x900000" \
- "-Wa,-mx86-used-note=yes" \
- {pass.c property-x86-2.S property-x86-1.S property-stack.S} \
- {{readelf {-n} property-5a.r}} \
- "property-5.o" \
- ] \
- [list \
- "Build property 5 (.so)" \
- "-shared -Wl,-z,stack-size=0x900000" \
- "-fPIC -Wa,-mx86-used-note=yes" \
- {property-x86-2.S property-x86-1.S pass.c property-stack.S} \
- {{readelf {-n} property-5.r}} \
- "property-5.so" \
- ] \
- [list \
"Build property-6.so" \
"-shared" \
"-fPIC -Wa,-mx86-used-note=yes" \
@@ -2180,6 +2100,96 @@ run_ld_link_tests [list \
"plt2" \
] \
[list \
+ "Build property 3" \
+ "" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-stk.s property-x86-1.s} \
+ {{readelf -n property-3.r}} \
+ "property-3" \
+ ] \
+ [list \
+ "Build property 3 (.o)" \
+ "-r -nostdlib" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-x86-1.s property-stk.s} \
+ {{readelf -n property-3a.r}} \
+ "property-3.o" \
+ ] \
+ [list \
+ "Build property 3 (.so)" \
+ "-shared" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-x86-1.s property-stk.s} \
+ {{readelf -n property-3.r}} \
+ "property-3.so" \
+ ] \
+ [list \
+ "Build property 4" \
+ "" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-stk.s property-x86-1.s property-x86-2.s} \
+ {{readelf -n property-4.r}} \
+ "property-4" \
+ ] \
+ [list \
+ "Build property 4 (.o)" \
+ "-r -nostdlib" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-x86-2.s property-x86-1.s property-stk.s} \
+ {{readelf {-n} property-4a.r}} \
+ "property-4.o" \
+ ] \
+ [list \
+ "Build property 4 (.so)" \
+ "-shared" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-x86-2.s property-x86-1.s property-stk.s} \
+ {{readelf -n property-4.r}} \
+ "property-4.so" \
+ ] \
+ [list \
+ "Build property 4 (-z stack-size=0)" \
+ "-z stack-size=0" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-stk.s property-x86-1.s property-x86-2.s} \
+ {{readelf -n property-4.r}} \
+ "property-4" \
+ ] \
+ [list \
+ "Build property 5" \
+ "-z stack-size=0x900000" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-stk.s property-x86-1.s property-x86-2.s} \
+ {{readelf -n property-5.r}} \
+ "property-5" \
+ ] \
+ [list \
+ "Build property 5 (.o)" \
+ "-r -nostdlib -z stack-size=0x900000" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-x86-2.s property-x86-1.s property-stk.s} \
+ {{readelf {-n} property-5a.r}} \
+ "property-5.o" \
+ ] \
+ [list \
+ "Build property 5 (.so)" \
+ "-shared -z stack-size=0x900000" \
+ "" \
+ "--64 -defsym __64_bit__=1 -mx86-used-note=yes" \
+ {property-x86-2.s property-x86-1.s property-stk.s} \
+ {{readelf -n property-5.r}} \
+ "property-5.so" \
+ ] \
+ [list \
"Build pr21626.so" \
"-shared -melf_x86_64" \
"" \
Hi Jan,
> I thought I might as well try to get past this. Could you give the patch
> below a try, with the other one reverted in addition? As said in the
> description it deals with only the known problematic tests for now.
Thanks for doing this. I will give the patch a go today, but there was
one thing that struck me in the original sources which I think we should
also clean up:
> + /* GNU_PROPERTY_X86_ISA_1_USED */
> + .long 0xc0010002 /* pr_type. */
> + .long 5f - 4f /* pr_datasz. */
> +4:
> + .long 0xa
> +5:
> + .p2align ALIGN
> + /* GNU_PROPERTY_X86_ISA_1_NEEDED */
> + .long 0xc0008002 /* pr_type. */
> + .long 5f - 4f /* pr_datasz. */
> +4:
> + .long 0x3
I really dislike these magic numbers. We ought to at least include
a comment describing them, eg:
.long 0xa /* GNU_PROPERTY_X86_ISA_1_V2 | GNU_PROPERTY_X86_ISA_1_V4 */
and:
.long 0x3 /* GNU_PROPERTY_X86_ISA_1_BASELINE | GNU_PROPERTY_X86_ISA_1_V2 */
Even better would be if we could just #include the common.h header
file and use the constant definitions themselves.
Cheers
Nick
On 06.11.2023 14:29, Nick Clifton wrote:
>> I thought I might as well try to get past this. Could you give the patch
>> below a try, with the other one reverted in addition? As said in the
>> description it deals with only the known problematic tests for now.
>
> Thanks for doing this. I will give the patch a go today, but there was
> one thing that struck me in the original sources which I think we should
> also clean up:
>
>> + /* GNU_PROPERTY_X86_ISA_1_USED */
>> + .long 0xc0010002 /* pr_type. */
>> + .long 5f - 4f /* pr_datasz. */
>> +4:
>> + .long 0xa
>> +5:
>> + .p2align ALIGN
>> + /* GNU_PROPERTY_X86_ISA_1_NEEDED */
>> + .long 0xc0008002 /* pr_type. */
>> + .long 5f - 4f /* pr_datasz. */
>> +4:
>> + .long 0x3
>
> I really dislike these magic numbers. We ought to at least include
> a comment describing them, eg:
>
> .long 0xa /* GNU_PROPERTY_X86_ISA_1_V2 | GNU_PROPERTY_X86_ISA_1_V4 */
>
> and:
>
> .long 0x3 /* GNU_PROPERTY_X86_ISA_1_BASELINE | GNU_PROPERTY_X86_ISA_1_V2 */
>
> Even better would be if we could just #include the common.h header
> file and use the constant definitions themselves.
In the course of making the adjustment I already needed to switch .S to .s
files, as the ld-as-linker tests want to use gas directly, not go via the
C compiler (unless I overlooked some way to achieve the latter nevertheless).
Hence #include-ing a common header just won't work.
I certainly agree that comments would be nice to add, but in git what I
sent you will show up as a rename rather than deletion + creation of a new
file, and hence you would observe that within the files only the first few
lines actually changed. Everything else is simply kept as it was. I'll go
and add such comments, but any kind of rework of the involved source files
will increase the barrier towards also addressing the same latent issues
elsewhere. (Albeit, as indicated earlier, I think it would be best anyway
if H.J. cleaned up all of this, once he's back.)
Jan
Hi Jan,
> In the course of making the adjustment I already needed to switch .S to .s
> files, as the ld-as-linker tests want to use gas directly, not go via the
> C compiler (unless I overlooked some way to achieve the latter nevertheless).
> Hence #include-ing a common header just won't work.
Fair enough.
> I certainly agree that comments would be nice to add, but in git what I
> sent you will show up as a rename rather than deletion + creation of a new
> file, and hence you would observe that within the files only the first few
> lines actually changed. Everything else is simply kept as it was. I'll go
> and add such comments,
Thanks.
> but any kind of rework of the involved source files
> will increase the barrier towards also addressing the same latent issues
> elsewhere. (Albeit, as indicated earlier, I think it would be best anyway
> if H.J. cleaned up all of this, once he's back.)
Noted.
The patched tests (and reverted test regexps) do work in my local x86-64-v3
environment, so your change is looking good to me.
Cheers
Nick
On 06.11.2023 15:22, Nick Clifton wrote:
> The patched tests (and reverted test regexps) do work in my local x86-64-v3
> environment, so your change is looking good to me.
Thanks for checking this out. I'll put this in then together with the revert,
albeit maybe I'll give it at least a day just in case anyone else wants to
comment.
Jan
@@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
Properties: stack size: 0x800000
#...
- x86 ISA needed: x86-64-baseline, x86-64-v2
+ x86 ISA needed: x86-64-baseline, x86-64-v2.*
#pass
@@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
Properties: stack size: 0x800000
#...
- x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v4
+ x86 ISA needed: x86-64-baseline, x86-64-v2,.*x86-64-v4
#pass
@@ -4,5 +4,5 @@ Displaying notes found in: .note.gnu.property
GNU 0x[0-9a-f]+ NT_GNU_PROPERTY_TYPE_0
Properties: stack size: 0x900000
#...
- x86 ISA needed: x86-64-baseline, x86-64-v2, x86-64-v4
+ x86 ISA needed: x86-64-baseline, x86-64-v2,.*x86-64-v4
#pass