[PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 8363 bytes --]

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:

  Unfortunately scan-assembler-symbol-section doesn't support selects
  yet, which this test implements both for the benefit of this test and
  for symmetry.

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

	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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: testsuite-scan-assembler-symbol-section.patch --]
[-- Type: text/x-patch, Size: 7073 bytes --]

# 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.

Re: [PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[Iain Sandoe]

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.

Re: [PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[David Edelsohn]

On Fri, Dec 4, 2020 at 5:35 AM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:

> 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?

The ",4" is the symbol alignment.  It is not necessary for the purpose
of the tests.

Thanks for looking further into this problem.  As I mentioned in my
earlier reply to the patch itself, I believe that this new feature and
infrastructure change should have been tested and fixed on
non-Linux/ELF/x86 architectures, not left as an exercise for the
maintainers of other targets.  A patch that introduces regressions in
the testsuite should be fixed or reverted and should be the
responsibility of the author -- whether the change is to the compiler
or to the testsuite.

Thanks, David

Re: [PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[Rainer Orth]

Hi Iain,

>> * Next, I ran the tests on Darwin 11 and found two failing tests:
[...]
>>  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.

indeed.  One could now add such a check if desired.

>>  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…

>> * 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'd been thinking of a lookup table myself.  We're doing something
similar already in scanasm.exp (hidden-scan-for).  The advantage of
doing it in the framework is one has to worry about the issue only once
per target, rather than (potentially) several times in different 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).

Given that the patch is a strict improvement as is and there were no
objections, I've now committed it.  I expect to work on the framework
test issues later, but that's not urgent given they aren't run by
default, so maybe GCC 12 material.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[Rainer Orth]

Hi David,

> On Fri, Dec 4, 2020 at 5:35 AM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>
>> 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?
>
> The ",4" is the symbol alignment.  It is not necessary for the purpose
> of the tests.

fine, thanks for the clarification.

> Thanks for looking further into this problem.  As I mentioned in my
> earlier reply to the patch itself, I believe that this new feature and
> infrastructure change should have been tested and fixed on
> non-Linux/ELF/x86 architectures, not left as an exercise for the
> maintainers of other targets.  A patch that introduces regressions in
> the testsuite should be fixed or reverted and should be the
> responsibility of the author -- whether the change is to the compiler
> or to the testsuite.

Yes and no: given the trouble I had myself trying an AIX bootstrap and
test and the effort it took guiding Iain through a Solaris 11.3
bootstrap on gcc211, I fear it's unreasonable to demand developers to do
all this testing themselves: if you have to address only one issue per
unfamiliar target while trying to, we'll loose contributors very
quickly.  I believe we should be able to get the setup of the various
cfarm hosts better in sync with the guidelines documented in the wiki to
avoid such trouble.

Ultimately, something like the trybot feature of the (now mostly
defunct) gdb buildbots could ease such pre-commit tests.

But I fully agree that the original patch should have seen way wider
cross-platform testing before being committed: asking target maintainers
for help usually works well enough: one just cannot expect them to spot
any patch that might require such testing themselves.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] testsuite: Fix various scan-assembler-symbol-section issues

[David Edelsohn]

On Mon, Dec 14, 2020 at 9:44 AM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>
> Hi David,
>
> > On Fri, Dec 4, 2020 at 5:35 AM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
> >
> >> 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?
> >
> > The ",4" is the symbol alignment.  It is not necessary for the purpose
> > of the tests.
>
> fine, thanks for the clarification.
>
> > Thanks for looking further into this problem.  As I mentioned in my
> > earlier reply to the patch itself, I believe that this new feature and
> > infrastructure change should have been tested and fixed on
> > non-Linux/ELF/x86 architectures, not left as an exercise for the
> > maintainers of other targets.  A patch that introduces regressions in
> > the testsuite should be fixed or reverted and should be the
> > responsibility of the author -- whether the change is to the compiler
> > or to the testsuite.
>
> Yes and no: given the trouble I had myself trying an AIX bootstrap and
> test and the effort it took guiding Iain through a Solaris 11.3
> bootstrap on gcc211, I fear it's unreasonable to demand developers to do
> all this testing themselves: if you have to address only one issue per
> unfamiliar target while trying to, we'll loose contributors very
> quickly.  I believe we should be able to get the setup of the various
> cfarm hosts better in sync with the guidelines documented in the wiki to
> avoid such trouble.

I respectfully disagree.  I find the GCC community very inconsistent
about this policy and I will continue to object.  Sometimes the
community demands thorough testing on all major targets and sometimes
the community says that it's too much trouble.  It depends on the
patch, and the developer, and the target that the patch is trying to
fix.  I believe that this inconsistent policy is driving away
developers also.

Many other software packages have a policy that any test suite
regression requires patch reversion.  GCC historically has not had
that policy.  But GCC does have a policy that testsuite failures are
the responsibility of the person who created the patch.  The GCC
community historically has decided that it is impractical and
unscalable for one person to install a patch and generate
unanticipated work for every other maintainer in the community.
Software developers do not want to work on a software package where
another developer can break their configuration, generate extra,
unanticipated work, and have no responsibility.

Most active maintainers are willing to help.  Most maintainers are
willing to help a developer debug a problem on their target.

Bootstrap on AIX works best with some additional configuration
parameters that are mentioned in the Compile Farm wiki.  Bootstrap on
AIX would have been much easier for you if you had asked for help and
advice.

>
> Ultimately, something like the trybot feature of the (now mostly
> defunct) gdb buildbots could ease such pre-commit tests.

I agree that GCC needs better CI and there are efforts in that direction.

>
> But I fully agree that the original patch should have seen way wider
> cross-platform testing before being committed: asking target maintainers
> for help usually works well enough: one just cannot expect them to spot
> any patch that might require such testing themselves.

Thanks, David