[PATCH] gas: Fix a testcase broken by new ZSTD support

[PATCH] gas: Fix a testcase broken by new ZSTD support

[Tsukasa OI]

The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
compression type for ZSTD.") added the new ELF compression type but it
accidentally broke a GAS testcase.  Since 2 is now a valid compression type
ELFCOMPRESS_ZSTD, the ".word 2" line in section10.s which expects an
**unknown** compression type needs to be changed to another unknown value.

gas/ChangeLog:

	* testsuite/gas/elf/section10.s: Use unknown ELF compression type.
---
 gas/testsuite/gas/elf/section10.s | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gas/testsuite/gas/elf/section10.s b/gas/testsuite/gas/elf/section10.s
index d52b3458fb1..6a252fb6334 100644
--- a/gas/testsuite/gas/elf/section10.s
+++ b/gas/testsuite/gas/elf/section10.s
@@ -4,7 +4,7 @@
 
 	# Make sure that a numeric value can be mixed with alpha values.
 	.section sec2, "a2048x"
-	.word 2
+	.word 9
 
 	# Make sure that specifying further arguments to .sections is still supported
 	.section sec3, "0xfedff000MS", %progbits, 32

base-commit: 731d2cc1d5106c077584bd83e96dbba4f7e11118
-- 
2.34.1

Re: [PATCH] gas: Fix a testcase broken by new ZSTD support

[Jan Beulich]

On 05.08.2022 09:12, Tsukasa OI wrote:
> The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
> compression type for ZSTD.") added the new ELF compression type but it
> accidentally broke a GAS testcase.  Since 2 is now a valid compression type
> ELFCOMPRESS_ZSTD, the ".word 2" line in section10.s which expects an
> **unknown** compression type needs to be changed to another unknown value.
> 
> gas/ChangeLog:
> 
> 	* testsuite/gas/elf/section10.s: Use unknown ELF compression type.
> ---
>  gas/testsuite/gas/elf/section10.s | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gas/testsuite/gas/elf/section10.s b/gas/testsuite/gas/elf/section10.s
> index d52b3458fb1..6a252fb6334 100644
> --- a/gas/testsuite/gas/elf/section10.s
> +++ b/gas/testsuite/gas/elf/section10.s
> @@ -4,7 +4,7 @@
>  
>  	# Make sure that a numeric value can be mixed with alpha values.
>  	.section sec2, "a2048x"
> -	.word 2
> +	.word 9

Hmm, in N years time this will become an issue again. Since it's not
really connected to the purpose of the test, would it make sense to
instead simply drop that one line from section10.d?

Jan

Re: [PATCH] gas: Fix a testcase broken by new ZSTD support

[Tsukasa OI]

On 2022/08/05 16:29, Jan Beulich wrote:
> On 05.08.2022 09:12, Tsukasa OI wrote:
>> The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
>> compression type for ZSTD.") added the new ELF compression type but it
>> accidentally broke a GAS testcase.  Since 2 is now a valid compression type
>> ELFCOMPRESS_ZSTD, the ".word 2" line in section10.s which expects an
>> **unknown** compression type needs to be changed to another unknown value.
>>
>> gas/ChangeLog:
>>
>> 	* testsuite/gas/elf/section10.s: Use unknown ELF compression type.
>> ---
>>  gas/testsuite/gas/elf/section10.s | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gas/testsuite/gas/elf/section10.s b/gas/testsuite/gas/elf/section10.s
>> index d52b3458fb1..6a252fb6334 100644
>> --- a/gas/testsuite/gas/elf/section10.s
>> +++ b/gas/testsuite/gas/elf/section10.s
>> @@ -4,7 +4,7 @@
>>  
>>  	# Make sure that a numeric value can be mixed with alpha values.
>>  	.section sec2, "a2048x"
>> -	.word 2
>> +	.word 9
> 
> Hmm, in N years time this will become an issue again. Since it's not
> really connected to the purpose of the test, would it make sense to
> instead simply drop that one line from section10.d?
> 
> Jan
> 

It seems making a testcase using SHF_COMPRESSED is not good in the long
term (because it tries to parse Elf_Internal_Chdr).  Instead, I will use
SHF_STRINGS ("32") without SHF_MERGE.

Tsukasa

[PATCH v2] gas: Fix a testcase broken by new ZSTD support

[Tsukasa OI]

The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
compression type for ZSTD.") added the new ELF compression type but it
accidentally broke a GAS testcase.  Since testing for the section type
"2048" (SHF_COMPRESSED) is not going to be portable in the long term, it
now tests SHF_STRINGS ("32") instead.  ".word 0" should be okay to
represent no null-terminated strings.

gas/ChangeLog:

	* testsuite/gas/elf/section10.s: Use SHF_STRINGS to test.  Put an
	empty string in it.
	* testsuite/gas/elf/section10.d: Reflect the changes above.
---
 gas/testsuite/gas/elf/section10.d | 3 +--
 gas/testsuite/gas/elf/section10.s | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/gas/testsuite/gas/elf/section10.d b/gas/testsuite/gas/elf/section10.d
index d51bd4e36e5..4ab3e8e56e9 100644
--- a/gas/testsuite/gas/elf/section10.d
+++ b/gas/testsuite/gas/elf/section10.d
@@ -13,8 +13,7 @@
 [ 	]*\[.*6000000\]: OS \(.*6000000\)
 [ 	]*\[.*\][ 	]+sec2
 [ 	]*PROGBITS.*
-[ 	]*\[0+00806\]: ALLOC, EXEC, COMPRESSED
-[ 	]*\[<unknown>: 0x[0-9]+\], .*
+[ 	]*\[0+00026\]: ALLOC, EXEC, STRINGS
 #...
 [ 	]*\[.*\][ 	]+sec3
 [ 	]*PROGBITS.*
diff --git a/gas/testsuite/gas/elf/section10.s b/gas/testsuite/gas/elf/section10.s
index d52b3458fb1..8b140715fb0 100644
--- a/gas/testsuite/gas/elf/section10.s
+++ b/gas/testsuite/gas/elf/section10.s
@@ -3,8 +3,8 @@
 	.word 1
 
 	# Make sure that a numeric value can be mixed with alpha values.
-	.section sec2, "a2048x"
-	.word 2
+	.section sec2, "a32x"
+	.word 0
 
 	# Make sure that specifying further arguments to .sections is still supported
 	.section sec3, "0xfedff000MS", %progbits, 32

base-commit: 731d2cc1d5106c077584bd83e96dbba4f7e11118
-- 
2.34.1

Re: [PATCH v2] gas: Fix a testcase broken by new ZSTD support

[Jan Beulich]

On 05.08.2022 09:45, Tsukasa OI wrote:
> The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
> compression type for ZSTD.") added the new ELF compression type but it
> accidentally broke a GAS testcase.  Since testing for the section type
> "2048" (SHF_COMPRESSED) is not going to be portable in the long term, it
> now tests SHF_STRINGS ("32") instead.  ".word 0" should be okay to
> represent no null-terminated strings.
> 
> gas/ChangeLog:
> 
> 	* testsuite/gas/elf/section10.s: Use SHF_STRINGS to test.  Put an
> 	empty string in it.
> 	* testsuite/gas/elf/section10.d: Reflect the changes above.

Hmm, but SHF_STRINGS requires sh_entsize to be set, so the resulting
object file is, strictly speaking, not valid. Perhaps use
SHF_LINK_ORDER then (where the spec leaves room for sh_link being zero),
or SHF_OS_NONCONFORMING (since we don't mean to actually link this
object file into anywhere), or SHF_TLS?

Jan

[PATCH v3] gas: Fix a testcase broken by new ZSTD support

[Tsukasa OI]

The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
compression type for ZSTD.") added the new ELF compression type but it
accidentally broke a GAS testcase.  Since testing for the section type
"2048" (SHF_COMPRESSED) is not going to be portable in the long term, it
now tests SHF_LINK_ORDER ("128") instead.

Using SHF_LINK_ORDER (with possibly sh_link == 0) is an idea by Jan Beulich.

gas/ChangeLog:

	* testsuite/gas/elf/section10.s: Use SHF_LINK_ORDER to test
	mixed numeric and alpha values.
	* testsuite/gas/elf/section10.d: Reflect the change above.
---
 gas/testsuite/gas/elf/section10.d | 3 +--
 gas/testsuite/gas/elf/section10.s | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/gas/testsuite/gas/elf/section10.d b/gas/testsuite/gas/elf/section10.d
index d51bd4e36e5..6aa57d42943 100644
--- a/gas/testsuite/gas/elf/section10.d
+++ b/gas/testsuite/gas/elf/section10.d
@@ -13,8 +13,7 @@
 [ 	]*\[.*6000000\]: OS \(.*6000000\)
 [ 	]*\[.*\][ 	]+sec2
 [ 	]*PROGBITS.*
-[ 	]*\[0+00806\]: ALLOC, EXEC, COMPRESSED
-[ 	]*\[<unknown>: 0x[0-9]+\], .*
+[ 	]*\[0+00086\]: ALLOC, EXEC, LINK ORDER
 #...
 [ 	]*\[.*\][ 	]+sec3
 [ 	]*PROGBITS.*
diff --git a/gas/testsuite/gas/elf/section10.s b/gas/testsuite/gas/elf/section10.s
index d52b3458fb1..e617bad609c 100644
--- a/gas/testsuite/gas/elf/section10.s
+++ b/gas/testsuite/gas/elf/section10.s
@@ -3,7 +3,7 @@
 	.word 1
 
 	# Make sure that a numeric value can be mixed with alpha values.
-	.section sec2, "a2048x"
+	.section sec2, "a128x"
 	.word 2
 
 	# Make sure that specifying further arguments to .sections is still supported

base-commit: 731d2cc1d5106c077584bd83e96dbba4f7e11118
-- 
2.34.1

Re: [PATCH v2] gas: Fix a testcase broken by new ZSTD support

[Tsukasa OI]

On 2022/08/05 17:24, Jan Beulich wrote:
> On 05.08.2022 09:45, Tsukasa OI wrote:
>> The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
>> compression type for ZSTD.") added the new ELF compression type but it
>> accidentally broke a GAS testcase.  Since testing for the section type
>> "2048" (SHF_COMPRESSED) is not going to be portable in the long term, it
>> now tests SHF_STRINGS ("32") instead.  ".word 0" should be okay to
>> represent no null-terminated strings.
>>
>> gas/ChangeLog:
>>
>> 	* testsuite/gas/elf/section10.s: Use SHF_STRINGS to test.  Put an
>> 	empty string in it.
>> 	* testsuite/gas/elf/section10.d: Reflect the changes above.
> 
> Hmm, but SHF_STRINGS requires sh_entsize to be set, so the resulting
> object file is, strictly speaking, not valid. Perhaps use
> SHF_LINK_ORDER then (where the spec leaves room for sh_link being zero),
> or SHF_OS_NONCONFORMING (since we don't mean to actually link this
> object file into anywhere), or SHF_TLS?
> 
> Jan
> 

Ah, that's a lot harder than I expected.  Okay... SHF_LINK_ORDER should
be fine, I looked into the BFD library and it seems SHF_LINK_ORDER
should not result in a harmful result.

Tsukasa

Re: [PATCH v3] gas: Fix a testcase broken by new ZSTD support

[Jan Beulich]

On 05.08.2022 10:52, Tsukasa OI wrote:
> The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
> compression type for ZSTD.") added the new ELF compression type but it
> accidentally broke a GAS testcase.  Since testing for the section type
> "2048" (SHF_COMPRESSED) is not going to be portable in the long term, it
> now tests SHF_LINK_ORDER ("128") instead.
> 
> Using SHF_LINK_ORDER (with possibly sh_link == 0) is an idea by Jan Beulich.
> 
> gas/ChangeLog:
> 
> 	* testsuite/gas/elf/section10.s: Use SHF_LINK_ORDER to test
> 	mixed numeric and alpha values.
> 	* testsuite/gas/elf/section10.d: Reflect the change above.
> ---
>  gas/testsuite/gas/elf/section10.d | 3 +--
>  gas/testsuite/gas/elf/section10.s | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)

Okay.

Thanks, Jan

Re: [PATCH] gas: Fix a testcase broken by new ZSTD support

[Cary Coutant]

> The commit 1369522f36eece1b37139a81f7f2139ba3915172 ("Recognize the new ELF
> compression type for ZSTD.") added the new ELF compression type but it
> accidentally broke a GAS testcase.  Since 2 is now a valid compression type
> ELFCOMPRESS_ZSTD, the ".word 2" line in section10.s which expects an
> **unknown** compression type needs to be changed to another unknown value.

Mea culpa! For such a trivial change, it didn't occur to me that it
might have a downstream effect, and I ran only the binutils tests.
Sorry, I should've run them all.

-cary