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.svens...@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
