Sorry for the slow reply. Torbjorn SVENSSON <torbjorn.svens...@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.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".
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