[PATCH] testsuite: Fix various scan-assembler-symbol-section issues
Iain Sandoe
iain@sandoe.co.uk
Fri Dec 4 11:26:04 GMT 2020
Hi Rainer,
thanks for looking at this, I was trying to see how to fix the failing Darwin
tests last week, and concluded that the absence of target selectors/xfail
meant skipping some tests - this is a much better solution.
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
> I recently started looking into scan-assembler-symbol-section since all
> tests using it were FAILing on Solaris/SPARC. Unfortuntely, the more I
> looked the more issues I found, both with the implementation and the
> interface. This patch addresses some of those, but there are quite a
> number of open questions.
>
> * The first issue was that on Solaris/SPARC, section names are
> double-quoted, both with as and gas:
>
> .section ".text"
>
> When using as, the section flag and type syntax is completely
> different from other ELF targets:
>
> .section "my_named_section",#alloc,#execinstr,#progbits
>
> This patch fixes this by stripping double quotes from section names.
>
> * However, this didn't work initially (only the leading quote was
> stripped), which is due to David's recent AIX patch: with the
> introduction of the new capturing group to handle both .section (ELF)
> and .csect (XCOFF), $full_section_directive would never be empty on
> ELF and Mach-O targets, so the extraction of the section name didn't
> work any longer. This had also broken the Darwin tests completely.
>
> * Wondering why this hadn't been captured by the framework tests led me
> to
>
> skipping test framework tests, CHECK_TEST_FRAMEWORK is not defined
>
> so this issue was all too easy to miss. I'll get back to the
> framework tests later: there's a whole lot of problems there even if
> run.
>
> * With working double quote stripping, all but one of the tests PASSed
> on Solaris/SPARC, the exception being:
>
> FAIL: gcc.dg/20021029-1.c scan-assembler-symbol-section symbol ar (found
> __sparc_get_pc_thunk.l7) has section ^\\\\.(const|rodata)|\\\\[RO\\\\]
> (found .text.__sparc_get_pc_thunk.l7%__sparc_get_pc_thunk.l7)
>
> This is due to the symbol name (ar) not being anchored in the test and
> unexpectedly matchting __sparc_get_pc_thunk.l7. Easily fixed, but it
> left me wondering about the interface: currently, every user of
> scan-assembler-symbol-section has to deal with anchoring and handling
> USER_LABEL_PREFIX herself. It seems to me that this were better
> handled in the framework instead. However, in the case at hand the
> actual symbol name is "ar.0" and I wonder if this can change in the
> future, i.e. we might need the generality of regexps for the symbol
> name here.
>
> * Next, I ran the tests on Darwin 11 and found two failing tests:
>
> FAIL: gcc.dg/darwin-sections.c scan-assembler-symbol-section symbol ^_a\$
> (symbol not found) has section \\\\.data
> FAIL: gcc.dg/darwin-sections.c scan-assembler-symbol-section symbol ^_b\$
> (symbol not found) has section \\\\.data
>
> is due to Iain's recent "Darwin : Begin rework of zero-fill sections."
> patch which emits
>
> .globl _a
> .zerofill __DATA,__common,_a,1,0
>
> This is already scanned for, so the two scans above can just go.
>
> The other failing test is
>
> FAIL: g++.dg/gomp/tls-5.C -std=c++14 scan-assembler-symbol-section
> symbol ^_?_ZGR2ir_\$ (symbol not found) has section
> ^\\\\.tdata|\\\\[TL\\\\]
> FAIL: g++.dg/gomp/tls-5.C -std=c++14 scan-assembler-symbol-section
> symbol ^_?ir\$ (symbol not found) has section ^\\\\.tbss|\\\\[TL\\\\]
>
> Other scans are guarded by target tls_native, and indeed the assembler
> output has
>
> ___emutls_v._ZGR2ir_:
> ___emutls_t._ZGR2ir_:
>
> ___emutls_v.ir:
I was half in mind to test for those symbols for emulated TLS (since they
indicate
the moral equivalent of placing the data in the special sections) - but
this wasn’t
possible absent the selector / xfail.
>
> Unfortunately scan-assembler-symbol-section doesn't support selects
> yet, which this test implements both for the benefit of this test and
> for symmetry.
… so now we ought to be able to make the test meaningful on emulated TLS
otherwise, it’s just consuming cpu - and we might as well have :
+// { dg-require-effective-target tls_native }
at the top…
>
> With those changes, test results are clean now on sparc-sun-solaris2.11,
> i386-pc-solaris2.11, i386-apple-darwin11.4.2, and
> powerpc-ibm-aix7.2.4.0.
>
> On AIX 7.2, there are changes like
>
> -PASS: g++.dg/gomp/tls-5.C -std=c++2a scan-assembler-symbol-section
> symbol ^_?ir$ (found ir) has section ^\\.tbss|\\[TL\\] (found
> _tls5.tls_[TL],4)
> +PASS: g++.dg/gomp/tls-5.C -std=c++2a scan-assembler-symbol-section
> symbol ^_?ir$ (found ir) has section ^\\.tbss|\\[TL\\] (found
> _tls5.tls_[TL])
>
> i.e. the ",4" after (?) the section name is now stripped. I believe
> this is benign: David?
>
> Besides, I've documented scan-assembler-symbol-section and
> scan-symbol-section in sourcebuild.texi as should have been done from
> the beginning.
>
> That's not the end of things, unfortunately:
>
> * I find my selector handling code in dg-scan-symbol-section
> particularly ugly since it has to deal with the different argument
> indices in
>
> scan-assembler-symbol-section func section [selector]
> scan-symbol-section filename func section [selector]
>
> which made me wonder about the need for scan-symbol-section in
> general. Right now, it's only used in the framework tests, and no
> other scan-* function (beside scan-file and scan-file-not, of course)
> has an explicit filename argument.
>
> Matthew, did you have any use in mind outside of the framework tests
> that justifies keeping it?
>
> Trying to get rid of it in the framework tests opened a can of worms,
> unfortunately: running
>
> gcc -S test.S -o test.s
>
> (equivalent to what the testsuite does) yields
>
> # 1 "test.S"
> # 1 "<built-in>"
> # 1 "<command-line>"
> # 31 "<command-line>"
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 32 "<command-line>" 2
> # 1 "test.S"
>
> .text
>
> I have no idea what point there is in duplicating the output on
> stdout, but this weird behaviour dates back to at least GCC 3.4, so
> I've kept scan-symbol-section for now.
>
> * Then there's a general issue with the framework tests: you get several
> FAILs when you run them, which is a no-no:
>
> FAIL: gcc.test-framework/dg-scan-symbol-section-1-exp-F.S
> scan-symbol-section symbol ^_test_symbol_.*$ (found _test_symbol_1) has
> section nomatch (found .text)
> FAIL: gcc.test-framework/dg-scan-symbol-section-1-exp-F.S
> scan-symbol-section symbol ^_test_symbol_.*$ (found _test_symbol_2) has
> section nomatch (found .data)
>
> Those are expected according to the comment, and should be xfail'ed
> accordingly (which is now possible).
>
> FAIL: gcc.test-framework/dg-scan-symbol-section-2-exp-F.S
> scan-symbol-section symbol ^_test_symbol$ (found _test_symbol) has
> section ^\\.text$ (no section detected)
>
> The comment states the test should be UNSUPPORTED, but later documents
> it as FAIL.
>
> FAIL: gcc.test-framework/dg-scan-symbol-section-3-exp-F.S
> scan-symbol-section symbol ^_test_symbol$ (symbol not found) has section
> ^\\.text$
>
> Same, with the added complication that the comment claims "(no symbol
> detected)" which doesn't match the actual output.
>
> I don't believe those outcomes should be made UNSUPPORTED, though: no
> other scan does so and when we scan for a symbol in a particular
> section and don't find it, something is just wrong.
>
> * Last but no least, I have an issue with the section argument to
> scan-assembler-symbol-section: right now the test developer needs to
> know about all possible formats of section names on a wide range of
> targets (just as he needs to take USER_LABEL_PREFIX etc. into account
> for the symbol name). This feels like a bad abstraction to me,
> although I'm uncertain what a better one would be. One vague idea is
> to use the ELF section name as a token. The framework can then
> translate it into the name appropriate for the current target. This
> way the knowledge would at least be centralized in one place and the
> task would only come up once per section name/type. Thoughts?
I think this is very hard to get right generically - I don’t see an
algorithmic
mapping between .xxxx or per-function ELF section naming semantics and,
for example, MACH-O section names. I fear that it would end up being a
look-up table and therefore probably better overt in the individual tests.
> I guess this patch could go in now as is since it addresses all current
> failures on all targets I could test, with room for later improvement?
>
> Sorry for this long and winded mail, but the issue is messy beyond belief.
indeed it is - so much so that I punted on trying to fix it (very happy you
picked it up).
thanks
Iain
>
> Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
>
>
> 2020-12-03 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
>
> gcc:
> * doc/sourcebuild.texi (Commands for use in dg-final, Scan the
> assembly output, scan-assembler-symbol-section): Document.
> (scan-symbol-section): Document.
>
> gcc/testsuite:
> * lib/scanasm.exp (scan-symbol-section): Pass args to
> dg-scan-symbol-section.
> (scan-assembler-symbol-section): Likewise.
> (dg-scan-symbol-section): Handle selector from orig_args.
> Get patterns from orig_args.
> (parse_section_of_symbols): Fix section_pattern.
> Strip double quotes from section name.
>
> * g++.dg/gomp/tls-5.C: Restrict ir, _ZGR2ir_ scans to tls_native.
> * gcc.dg/20021029-1.c: Anchor ar symbol.
> * gcc.dg/darwin-sections.c: Remove obsolete scans for _a, _b in
> .data.
>
> # HG changeset patch
> # Parent 60bd760a57db271eb2da2cb17156deca98faf14b
> testsuite: Fix various scan-assembler-symbol-section issues
>
> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
> --- a/gcc/doc/sourcebuild.texi
> +++ b/gcc/doc/sourcebuild.texi
> @@ -2925,6 +2925,14 @@ Passes if @var{regex} matches text in th
> Passes if @var{regex} does not match text in the test's demangled assembler
> output.
>
> +@item scan-assembler-symbol-section @var{functions} @var{section} [@{
> target/xfail @var{selector} @}]
> +Passes if @var{functions} are all in @var{section}. The caller needs to
> +allow for @code{USER_LABEL_PREFIX} and different section name conventions.
> +
> +@item scan-symbol-section @var{filename} @var{functions} @var{section}
> [@{ target/xfail @var{selector} @}]
> +Passes if @var{functions} are all in @var{section}in @var{filename}.
> +The same caveats as for @code{scan-assembler-symbol-section} apply.
> +
> @item scan-hidden @var{symbol} [@{ target/xfail @var{selector} @}]
> Passes if @var{symbol} is defined as a hidden symbol in the test's
> assembly output.
> diff --git a/gcc/testsuite/g++.dg/gomp/tls-5.C
> b/gcc/testsuite/g++.dg/gomp/tls-5.C
> --- a/gcc/testsuite/g++.dg/gomp/tls-5.C
> +++ b/gcc/testsuite/g++.dg/gomp/tls-5.C
> @@ -1,8 +1,8 @@
> // The reference temp should be TLS, not normal data.
> // { dg-require-effective-target c++11 }
> // { dg-final { scan-assembler-not "\\.data" { target tls_native xfail powerpc-*-aix* } } }
> -// { dg-final { scan-assembler-symbol-section {^_?ir$} {^\.tbss|\[TL\]}
> } }
> -// { dg-final { scan-assembler-symbol-section {^_?_ZGR2ir_$}
> {^\.tdata|\[TL\]} } }
> +// { dg-final { scan-assembler-symbol-section {^_?ir$} {^\.tbss|\[TL\]}
> { target tls_native } } }
> +// { dg-final { scan-assembler-symbol-section {^_?_ZGR2ir_$}
> {^\.tdata|\[TL\]} { target tls_native } } }
>
> extern int&& ir;
> #pragma omp threadprivate (ir)
> diff --git a/gcc/testsuite/gcc.dg/20021029-1.c
> b/gcc/testsuite/gcc.dg/20021029-1.c
> --- a/gcc/testsuite/gcc.dg/20021029-1.c
> +++ b/gcc/testsuite/gcc.dg/20021029-1.c
> @@ -3,7 +3,7 @@
> /* { dg-do compile { target fpic } } */
> /* { dg-options "-O2 -fpic" } */
> /* { dg-final { scan-assembler-not ".data.rel.ro.local" } } */
> -/* { dg-final { scan-assembler-symbol-section {ar}
> {^\.(const|rodata)|\[RO\]} } } */
> +/* { dg-final { scan-assembler-symbol-section {^_?ar}
> {^\.(const|rodata)|\[RO\]} } } */
> /* { dg-require-effective-target label_values } */
> /* { dg-require-effective-target indirect_jumps } */
>
> diff --git a/gcc/testsuite/gcc.dg/darwin-sections.c
> b/gcc/testsuite/gcc.dg/darwin-sections.c
> --- a/gcc/testsuite/gcc.dg/darwin-sections.c
> +++ b/gcc/testsuite/gcc.dg/darwin-sections.c
> @@ -10,9 +10,7 @@ typedef struct _empty {} e_s;
> /* These should go in .comm */
> char ub;
> e_s ea;
> -/* { dg-final { scan-assembler-symbol-section {^_a$} {\.data} } } */
> /* { dg-final { scan-assembler ".comm\[\t \]_ub,1" } } */
> -/* { dg-final { scan-assembler-symbol-section {^_b$} {\.data} } } */
> /* { dg-final { scan-assembler ".comm\[\t \]_ea,1" } } */
>
> /* These should go into __DATA,__common */
> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -173,14 +173,8 @@ proc scan-assembler-symbol-section { arg
> set testcase [testname-for-summary]
> set filename [lindex $testcase 0]
> set output_file "[file rootname [file tail $filename]].s"
> - set symbol_pattern [lindex $args 0]
> - set expected_section_pattern [lindex $args 1]
> dg-scan-symbol-section \
> - "scan-assembler-symbol-section" \
> - $testcase \
> - $output_file \
> - $symbol_pattern \
> - $expected_section_pattern
> + "scan-assembler-symbol-section" $testcase $output_file $args
> }
>
> # Check that symbols are emitted in the desired section.
> @@ -227,22 +221,44 @@ proc scan-assembler-symbol-section { arg
> proc scan-symbol-section { args } {
> set testcase [testname-for-summary]
> set output_file [lindex $args 0]
> - set symbol_pattern [lindex $args 1]
> - set expected_section_pattern [lindex $args 2]
> dg-scan-symbol-section \
> - "scan-symbol-section" \
> - $testcase \
> - $output_file \
> - $symbol_pattern \
> - $expected_section_pattern
> + "scan-symbol-section" $testcase $output_file $args
> }
>
> -# Check that symbols are emitted in the desired section.
> +# Check that symbols are emitted in the desired section. The ORIG_ARGS is
> +# the list of arguments provided by dg-final to scan-symbol-section or
> +# scan-assembler-symbol-section. The first element in ORIG_ARGS is the
> +# regular expression to look for in the file. The second element, if
> +# present, is a DejaGNU target selector.
> #
> -# Avoid calling this function directly. In tests, use scan-symbol-section,
> +# Avoid calling this function directly. In tests, use
> scan-symbol-section,
> # scan-assembler-symbol-section, or scan-lto-assembler-symbol-section instead.
>
> -proc dg-scan-symbol-section { name testcase output_file symbol_pattern
> expected_section_pattern } {
> +proc dg-scan-symbol-section { name testcase output_file orig_args } {
> + if { [llength $orig_args] < 2 } {
> + error "$name: too few arguments"
> + return
> + }
> + if { [llength $orig_args] > 4 } {
> + error "$name: too many arguments"
> + return
> + }
> + switch $name {
> + "scan-symbol-section" { set arg_incr 1 }
> + default { set arg_incr 0 }
> + }
> + if { [llength $orig_args] >= 3 } {
> + switch [dg-process-target [lindex $orig_args [expr 2 + $arg_incr]]] {
> + "S" { }
> + "N" { return }
> + "F" { setup_xfail "*-*-*" }
> + "P" { }
> + }
> + }
> +
> + set symbol_pattern [lindex $orig_args [expr 0 + $arg_incr]]
> + set expected_section_pattern [lindex $orig_args [expr 1 + $arg_incr]]
> +
> set printable_symbol_pattern [make_pattern_printable $symbol_pattern]
> set printable_expected_section_pattern [make_pattern_printable $expected_section_pattern]
>
> @@ -303,7 +319,7 @@ proc dg-scan-symbol-section { name testc
> proc parse_section_of_symbols { filename result } {
> upvar $result up_result
>
> - set section_pattern
> {^\s*(?:(\.section|\.csect)\s+(.*)|(\.const|\.data|\.text)\s*)$}
> + set section_pattern
> {^\s*(?:(?:\.section|\.csect)\s+(.*)|(\.const|\.data|\.text)\s*)$}
> set label_pattern {^(\S+):$}
>
> set fd [open $filename r]
> @@ -314,9 +330,11 @@ proc parse_section_of_symbols { filename
> } elseif { [regexp -- $section_pattern $line dummy section_directive_arguments full_section_directive] } {
> if { $full_section_directive eq "" } {
> # Example: .section .text,"ax",progbits
> + # Example: .section ".text",#alloc,#execinstr,#progbits
> # Example: .section __TEXT,__text
> set arguments [split $section_directive_arguments ","]
> set current_section [string trim [lindex $arguments 0]]
> + set current_section [string trim $current_section {"}]
> set arg_1 [string trim [lindex $arguments 1]]
> if { [regexp {^_} $arg_1] } {
> # The second argument looks like a Mach-O section name.
More information about the Gcc-patches
mailing list