[v2] testsuite: Only run test on target if VMA == LMA
Checks
Commit Message
Checking that the triplet matches arm*-*-eabi (or msp430-*-*) is not
enough to know if the execution will enter an endless loop, or if it
will give a meaningful result. As the execution test only work when
VMA and LMA are equal, make sure that this condition is met.
2022-09-16 Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
gcc/testsuite/ChangeLog:
* lib/target-supports.exp (check_effective_target_vma_equals_lma): New.
* c-c++-common/torture/attr-noinit-1.c: Requre VMA == LMA to run.
* c-c++-common/torture/attr-noinit-2.c: Likewise.
* c-c++-common/torture/attr-noinit-3.c: Likewise.
* c-c++-common/torture/attr-persistent-1.c: Likewise.
* c-c++-common/torture/attr-persistent-3.c: Likewise.
Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com>
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
.../c-c++-common/torture/attr-noinit-1.c | 3 +-
.../c-c++-common/torture/attr-noinit-2.c | 3 +-
.../c-c++-common/torture/attr-noinit-3.c | 3 +-
.../c-c++-common/torture/attr-persistent-1.c | 3 +-
.../c-c++-common/torture/attr-persistent-3.c | 3 +-
gcc/testsuite/lib/target-supports.exp | 49 +++++++++++++++++++
6 files changed, 59 insertions(+), 5 deletions(-)
Comments
Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Checking that the triplet matches arm*-*-eabi (or msp430-*-*) is not
> enough to know if the execution will enter an endless loop, or if it
> will give a meaningful result. As the execution test only work when
> VMA and LMA are equal, make sure that this condition is met.
>
> 2022-09-16 Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>
> gcc/testsuite/ChangeLog:
>
> * lib/target-supports.exp (check_effective_target_vma_equals_lma): New.
> * c-c++-common/torture/attr-noinit-1.c: Requre VMA == LMA to run.
> * c-c++-common/torture/attr-noinit-2.c: Likewise.
> * c-c++-common/torture/attr-noinit-3.c: Likewise.
> * c-c++-common/torture/attr-persistent-1.c: Likewise.
> * c-c++-common/torture/attr-persistent-3.c: Likewise.
Looks like a nice thing to have.
Could you add an entry to doc/sourcebuild.texi too?
A couple of comments below...
> Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com>
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
> .../c-c++-common/torture/attr-noinit-1.c | 3 +-
> .../c-c++-common/torture/attr-noinit-2.c | 3 +-
> .../c-c++-common/torture/attr-noinit-3.c | 3 +-
> .../c-c++-common/torture/attr-persistent-1.c | 3 +-
> .../c-c++-common/torture/attr-persistent-3.c | 3 +-
> gcc/testsuite/lib/target-supports.exp | 49 +++++++++++++++++++
> 6 files changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/testsuite/c-c++-common/torture/attr-noinit-1.c b/gcc/testsuite/c-c++-common/torture/attr-noinit-1.c
> index 877e7647ac9..f84eba0b649 100644
> --- a/gcc/testsuite/c-c++-common/torture/attr-noinit-1.c
> +++ b/gcc/testsuite/c-c++-common/torture/attr-noinit-1.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run } */
> +/* { dg-do link } */
> +/* { dg-do run { target { vma_equals_lma } } } */
> /* { dg-require-effective-target noinit } */
> /* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
> /* { dg-options "-save-temps" } */
> diff --git a/gcc/testsuite/c-c++-common/torture/attr-noinit-2.c b/gcc/testsuite/c-c++-common/torture/attr-noinit-2.c
> index befa2a0bd52..4528b9e3cfa 100644
> --- a/gcc/testsuite/c-c++-common/torture/attr-noinit-2.c
> +++ b/gcc/testsuite/c-c++-common/torture/attr-noinit-2.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run } */
> +/* { dg-do link } */
> +/* { dg-do run { target { vma_equals_lma } } } */
> /* { dg-require-effective-target noinit } */
> /* { dg-options "-fdata-sections -save-temps" } */
> /* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
> diff --git a/gcc/testsuite/c-c++-common/torture/attr-noinit-3.c b/gcc/testsuite/c-c++-common/torture/attr-noinit-3.c
> index 519e88a59a6..2f1745694c9 100644
> --- a/gcc/testsuite/c-c++-common/torture/attr-noinit-3.c
> +++ b/gcc/testsuite/c-c++-common/torture/attr-noinit-3.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run } */
> +/* { dg-do link } */
> +/* { dg-do run { target { vma_equals_lma } } } */
> /* { dg-require-effective-target noinit } */
> /* { dg-options "-flto -save-temps" } */
> /* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
> diff --git a/gcc/testsuite/c-c++-common/torture/attr-persistent-1.c b/gcc/testsuite/c-c++-common/torture/attr-persistent-1.c
> index 72dc3c27192..b11a515cef8 100644
> --- a/gcc/testsuite/c-c++-common/torture/attr-persistent-1.c
> +++ b/gcc/testsuite/c-c++-common/torture/attr-persistent-1.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run } */
> +/* { dg-do link } */
> +/* { dg-do run { target { vma_equals_lma } } } */
> /* { dg-require-effective-target persistent } */
> /* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
> /* { dg-options "-save-temps" } */
> diff --git a/gcc/testsuite/c-c++-common/torture/attr-persistent-3.c b/gcc/testsuite/c-c++-common/torture/attr-persistent-3.c
> index 3e4fd28618d..068a72af5c8 100644
> --- a/gcc/testsuite/c-c++-common/torture/attr-persistent-3.c
> +++ b/gcc/testsuite/c-c++-common/torture/attr-persistent-3.c
> @@ -1,4 +1,5 @@
> -/* { dg-do run } */
> +/* { dg-do link } */
> +/* { dg-do run { target { vma_equals_lma } } } */
> /* { dg-require-effective-target persistent } */
> /* { dg-options "-flto -save-temps" } */
> /* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 703aba412a6..df8141a15d8 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -370,6 +370,55 @@ proc check_weak_override_available { } {
> return [check_weak_available]
> }
>
> +# Return 1 if VMA is equal to LMA for the .data section, 0
> +# otherwise. Cache the result.
> +
> +proc check_effective_target_vma_equals_lma { } {
> + global tool
> +
> + return [check_cached_effective_target vma_equals_lma {
> + set src vma_equals_lma[pid].c
> + set exe vma_equals_lma[pid].exe
> + verbose "check_effective_target_vma_equals_lma compiling testfile $src" 2
> + set f [open $src "w"]
> + puts $f "#ifdef __cplusplus\nextern \"C\"\n#endif\n"
> + puts $f "int foo = 42; void main() {}"
> + close $f
> + set lines [${tool}_target_compile $src $exe executable ""]
> + file delete $src
> +
> + if [string match "" $lines] then {
> + # No error messages
> +
> + set objdump_name [find_binutils_prog objdump]
> + set output [remote_exec host "$objdump_name" "--section-headers --section=.data $exe"]
> + set output [lindex $output 1]
> +
> + remote_file build delete $exe
> +
> + # Example output of objdump:
> + #vma_equals_lma9059.exe: file format elf32-littlearm
> + #
> + #Sections:
> + #Idx Name Size VMA LMA File off Algn
> + # 6 .data 00000558 20000000 08002658 00020000 2**3
> + # CONTENTS, ALLOC, LOAD, DATA
> +
> + # Capture LMA and VMA columns for .data section
> + if ![ regexp "\d*\d+\s+\.data\s+\d+\s+(\d+)\s+(\d+)" $output dummy vma lma ] {
Maybe my Tcl is getting rusty, but I'm surprised this quoting works.
I'd have expected single backslashes to be interpreted as C-style
sequences (for \n etc) and so be eaten before regexp sees them.
Quoting with {...} instead of "..." would avoid that.
> + verbose "Could not parse objdump output" 2
> + return 0
> + } else {
> + return [string equal $vma $lma]
> + }
> + } else {
> + remote_file build delete $exe
> + verbose "Could not determine if VMA is equal to LMA. Assuming not equal." 2
> + return 0
> + }
Would it be more conservative to return 1 on failure rather than 0?
That way, a faulty test would trigger XFAILs rather than UNSUPPORTEDs,
with XFAILs being more likely to get attention.
On the other hand, I suppose we don't lose much if these tests are
run on common targets only. So either way is OK, just asking. ;-)
Thanks,
Richard
> + }]
> +}
> +
> # The "noinit" attribute is only supported by some targets.
> # This proc returns 1 if it's supported, 0 if it's not.
Hi Richard,
Thanks for your review.
Comments below.
On 2022-09-23 19:34, Richard Sandiford wrote:
> Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> Checking that the triplet matches arm*-*-eabi (or msp430-*-*) is not
>> enough to know if the execution will enter an endless loop, or if it
>> will give a meaningful result. As the execution test only work when
>> VMA and LMA are equal, make sure that this condition is met.
>>
>> 2022-09-16 Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>
>> gcc/testsuite/ChangeLog:
>>
>> * lib/target-supports.exp (check_effective_target_vma_equals_lma): New.
>> * c-c++-common/torture/attr-noinit-1.c: Requre VMA == LMA to run.
>> * c-c++-common/torture/attr-noinit-2.c: Likewise.
>> * c-c++-common/torture/attr-noinit-3.c: Likewise.
>> * c-c++-common/torture/attr-persistent-1.c: Likewise.
>> * c-c++-common/torture/attr-persistent-3.c: Likewise.
>
> Looks like a nice thing to have.
>
> Could you add an entry to doc/sourcebuild.texi too?
I've added a note and will be shared in a v3.
>> +
>> + # Example output of objdump:
>> + #vma_equals_lma9059.exe: file format elf32-littlearm
>> + #
>> + #Sections:
>> + #Idx Name Size VMA LMA File off Algn
>> + # 6 .data 00000558 20000000 08002658 00020000 2**3
>> + # CONTENTS, ALLOC, LOAD, DATA
>> +
>> + # Capture LMA and VMA columns for .data section
>> + if ![ regexp "\d*\d+\s+\.data\s+\d+\s+(\d+)\s+(\d+)" $output dummy vma lma ] {
>
> Maybe my Tcl is getting rusty, but I'm surprised this quoting works.
> I'd have expected single backslashes to be interpreted as C-style
> sequences (for \n etc) and so be eaten before regexp sees them.
> Quoting with {...} instead of "..." would avoid that.
Good catch! I'm not fluent in Tcl and apparently, I was not testing this
well enough before sending it to the list. I got the expected result for
the test cases, but for the wrong reason. I've correct it for the v3.
>> + verbose "Could not parse objdump output" 2
>> + return 0
>> + } else {
>> + return [string equal $vma $lma]
>> + }
>> + } else {
>> + remote_file build delete $exe
>> + verbose "Could not determine if VMA is equal to LMA. Assuming not equal." 2
>> + return 0
>> + }
>
> Would it be more conservative to return 1 on failure rather than 0?
> That way, a faulty test would trigger XFAILs rather than UNSUPPORTEDs,
> with XFAILs being more likely to get attention.
The main issue here is that for targets where VMA != LMA, executing the
tests will fall into an endless recursion loop. Well, "endless" in the
sense that the stack might be depleted or the test will simply timeout.
The test cases are designed to assume that it's safe to call _start()
from within main() to verify that the state of some variables tagged
with certain attributes are correct after a "reset".
> On the other hand, I suppose we don't lose much if these tests are
> run on common targets only. So either way is OK, just asking. ;-)
Do you think it's worth to have these tests reach the timeout to have
them in the XFAIL list rather than in the UNSUPPORTED list?
Keep in mind that it's not just one test case, it's 5 test cases times
the number of permutations of the CFLAGS...
It's also not expected that these test cases will be changed in a way
that will make them work when VMA != LMA.
Kind regards,
Torbjörn
Sorry for the slow reply.
Torbjorn SVENSSON <torbjorn.svensson@foss.st.com> writes:
> Hi Richard,
>
> Thanks for your review.
> Comments below.
>
> On 2022-09-23 19:34, Richard Sandiford wrote:
>> Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>>> Checking that the triplet matches arm*-*-eabi (or msp430-*-*) is not
>>> enough to know if the execution will enter an endless loop, or if it
>>> will give a meaningful result. As the execution test only work when
>>> VMA and LMA are equal, make sure that this condition is met.
>>>
>>> 2022-09-16 Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * lib/target-supports.exp (check_effective_target_vma_equals_lma): New.
>>> * c-c++-common/torture/attr-noinit-1.c: Requre VMA == LMA to run.
>>> * c-c++-common/torture/attr-noinit-2.c: Likewise.
>>> * c-c++-common/torture/attr-noinit-3.c: Likewise.
>>> * c-c++-common/torture/attr-persistent-1.c: Likewise.
>>> * c-c++-common/torture/attr-persistent-3.c: Likewise.
>>
>> Looks like a nice thing to have.
>>
>> Could you add an entry to doc/sourcebuild.texi too?
>
> I've added a note and will be shared in a v3.
>
>>> +
>>> + # Example output of objdump:
>>> + #vma_equals_lma9059.exe: file format elf32-littlearm
>>> + #
>>> + #Sections:
>>> + #Idx Name Size VMA LMA File off Algn
>>> + # 6 .data 00000558 20000000 08002658 00020000 2**3
>>> + # CONTENTS, ALLOC, LOAD, DATA
>>> +
>>> + # Capture LMA and VMA columns for .data section
>>> + if ![ regexp "\d*\d+\s+\.data\s+\d+\s+(\d+)\s+(\d+)" $output dummy vma lma ] {
>>
>> Maybe my Tcl is getting rusty, but I'm surprised this quoting works.
>> I'd have expected single backslashes to be interpreted as C-style
>> sequences (for \n etc) and so be eaten before regexp sees them.
>> Quoting with {...} instead of "..." would avoid that.
>
> Good catch! I'm not fluent in Tcl and apparently, I was not testing this
> well enough before sending it to the list. I got the expected result for
> the test cases, but for the wrong reason. I've correct it for the v3.
>
>>> + verbose "Could not parse objdump output" 2
>>> + return 0
>>> + } else {
>>> + return [string equal $vma $lma]
>>> + }
>>> + } else {
>>> + remote_file build delete $exe
>>> + verbose "Could not determine if VMA is equal to LMA. Assuming not equal." 2
>>> + return 0
>>> + }
>>
>> Would it be more conservative to return 1 on failure rather than 0?
>> That way, a faulty test would trigger XFAILs rather than UNSUPPORTEDs,
>> with XFAILs being more likely to get attention.
>
> The main issue here is that for targets where VMA != LMA, executing the
> tests will fall into an endless recursion loop. Well, "endless" in the
> sense that the stack might be depleted or the test will simply timeout.
> The test cases are designed to assume that it's safe to call _start()
> from within main() to verify that the state of some variables tagged
> with certain attributes are correct after a "reset".
Ah, OK. In that case, I agree return 0 is the right choice.
>> On the other hand, I suppose we don't lose much if these tests are
>> run on common targets only. So either way is OK, just asking. ;-)
>
> Do you think it's worth to have these tests reach the timeout to have
> them in the XFAIL list rather than in the UNSUPPORTED list?
> Keep in mind that it's not just one test case, it's 5 test cases times
> the number of permutations of the CFLAGS...
> It's also not expected that these test cases will be changed in a way
> that will make them work when VMA != LMA.
Yeah, good points. Let's stick with it as it is then.
Thanks,
Richard
@@ -1,4 +1,5 @@
-/* { dg-do run } */
+/* { dg-do link } */
+/* { dg-do run { target { vma_equals_lma } } } */
/* { dg-require-effective-target noinit } */
/* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
/* { dg-options "-save-temps" } */
@@ -1,4 +1,5 @@
-/* { dg-do run } */
+/* { dg-do link } */
+/* { dg-do run { target { vma_equals_lma } } } */
/* { dg-require-effective-target noinit } */
/* { dg-options "-fdata-sections -save-temps" } */
/* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
@@ -1,4 +1,5 @@
-/* { dg-do run } */
+/* { dg-do link } */
+/* { dg-do run { target { vma_equals_lma } } } */
/* { dg-require-effective-target noinit } */
/* { dg-options "-flto -save-temps" } */
/* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
@@ -1,4 +1,5 @@
-/* { dg-do run } */
+/* { dg-do link } */
+/* { dg-do run { target { vma_equals_lma } } } */
/* { dg-require-effective-target persistent } */
/* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
/* { dg-options "-save-temps" } */
@@ -1,4 +1,5 @@
-/* { dg-do run } */
+/* { dg-do link } */
+/* { dg-do run { target { vma_equals_lma } } } */
/* { dg-require-effective-target persistent } */
/* { dg-options "-flto -save-temps" } */
/* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */
@@ -370,6 +370,55 @@ proc check_weak_override_available { } {
return [check_weak_available]
}
+# Return 1 if VMA is equal to LMA for the .data section, 0
+# otherwise. Cache the result.
+
+proc check_effective_target_vma_equals_lma { } {
+ global tool
+
+ return [check_cached_effective_target vma_equals_lma {
+ set src vma_equals_lma[pid].c
+ set exe vma_equals_lma[pid].exe
+ verbose "check_effective_target_vma_equals_lma compiling testfile $src" 2
+ set f [open $src "w"]
+ puts $f "#ifdef __cplusplus\nextern \"C\"\n#endif\n"
+ puts $f "int foo = 42; void main() {}"
+ close $f
+ set lines [${tool}_target_compile $src $exe executable ""]
+ file delete $src
+
+ if [string match "" $lines] then {
+ # No error messages
+
+ set objdump_name [find_binutils_prog objdump]
+ set output [remote_exec host "$objdump_name" "--section-headers --section=.data $exe"]
+ set output [lindex $output 1]
+
+ remote_file build delete $exe
+
+ # Example output of objdump:
+ #vma_equals_lma9059.exe: file format elf32-littlearm
+ #
+ #Sections:
+ #Idx Name Size VMA LMA File off Algn
+ # 6 .data 00000558 20000000 08002658 00020000 2**3
+ # CONTENTS, ALLOC, LOAD, DATA
+
+ # Capture LMA and VMA columns for .data section
+ if ![ regexp "\d*\d+\s+\.data\s+\d+\s+(\d+)\s+(\d+)" $output dummy vma lma ] {
+ verbose "Could not parse objdump output" 2
+ return 0
+ } else {
+ return [string equal $vma $lma]
+ }
+ } else {
+ remote_file build delete $exe
+ verbose "Could not determine if VMA is equal to LMA. Assuming not equal." 2
+ return 0
+ }
+ }]
+}
+
# The "noinit" attribute is only supported by some targets.
# This proc returns 1 if it's supported, 0 if it's not.