[PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Samuel Thibault]

hurd's jmp_buf-ssp.sym does not define any symbol.
scripts/gen-as-const.py currently was emitting an empty line in that
case, and the gawk invocation was prepending "asconst_" to it, ending up
with:

.../build/glibc/setjmp/test-as-const-jmp_buf-ssp.c:1:2: error: expected « = », « , », « ; », « asm » or « __attribute__ » at end of input
    1 |  asconst_
      |  ^~~~~~~~

2018-12-01  Samuel Thibault  <samuel.thibault@ens-lyon.org>

	* scripts/gen-as-const.py (main): Avoid emitting empty line when
	there is no element in `consts'.

diff --git a/scripts/gen-as-const.py b/scripts/gen-as-const.py
index b7a5744bb1..cabf401ed1 100644
--- a/scripts/gen-as-const.py
+++ b/scripts/gen-as-const.py
@@ -153,7 +153,7 @@ def main():
         print(gen_test(sym_data))
     else:
         consts = compute_c_consts(sym_data, args.cc)
-        print('\n'.join('#define %s %s' % c for c in sorted(consts.items())))
+        print(''.join('#define %s %s\n' % c for c in sorted(consts.items())), end='')
 
 if __name__ == '__main__':
     main()

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Samuel Thibault]

Samuel Thibault, le dim. 02 déc. 2018 00:12:47 +0100, a ecrit:
> hurd's jmp_buf-ssp.sym does not define any symbol.
> scripts/gen-as-const.py currently was emitting an empty line in that
> case, and the gawk invocation was prepending "asconst_" to it, ending up
> with:
> 
> .../build/glibc/setjmp/test-as-const-jmp_buf-ssp.c:1:2: error: expected « = », « , », « ; », « asm » or « __attribute__ » at end of input
>     1 |  asconst_
>       |  ^~~~~~~~

That said, it's not enough, gen_test() does not emit a main() function,
and thus the test build fails with missing reference to main(). Is there
a strong reason for making the emission of code lazy with the started
boolean?

Samuel

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Joseph Myers]

On Sat, 1 Dec 2018, Samuel Thibault wrote:

> 2018-12-01  Samuel Thibault  <samuel.thibault@ens-lyon.org>
> 
> 	* scripts/gen-as-const.py (main): Avoid emitting empty line when
> 	there is no element in `consts'.

OK, please commit.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Samuel Thibault]

Joseph Myers, le lun. 03 déc. 2018 13:23:48 +0000, a ecrit:
> On Sat, 1 Dec 2018, Samuel Thibault wrote:
> 
> > 2018-12-01  Samuel Thibault  <samuel.thibault@ens-lyon.org>
> > 
> > 	* scripts/gen-as-const.py (main): Avoid emitting empty line when
> > 	there is no element in `consts'.
> 
> OK, please commit.

Done so, thanks.

Samuel

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[H.J. Lu]

On Mon, Dec 3, 2018 at 5:42 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Joseph Myers, le lun. 03 déc. 2018 13:23:48 +0000, a ecrit:
> > On Sat, 1 Dec 2018, Samuel Thibault wrote:
> >
> > > 2018-12-01  Samuel Thibault  <samuel.thibault@ens-lyon.org>
> > >
> > >     * scripts/gen-as-const.py (main): Avoid emitting empty line when
> > >     there is no element in `consts'.
> >
> > OK, please commit.
>
> Done so, thanks.

I still got

/export/ssd/build/tools-build/glibc-many/src/glibc/csu/../sysdeps/i386/start.S:122:
undefined reference to `main'
collect2: error: ld returned 1 exit status
make[4]: *** [../Rules:201:
/export/ssd/build/tools-build/glibc-many/build/glibcs/i686-gnu/glibc/setjmp/test-as-const-jmp_buf-ssp]
Error 1
make[4]: *** Waiting for unfinished jobs....
rm /export/ssd/build/tools-build/glibc-many/build/glibcs/i686-gnu/glibc/setjmp/test-as-const-jmp_buf-ssp.c
make[4]: Leaving directory
'/export/ssd/build/tools-build/glibc-many/src/glibc/setjmp'
make[3]: *** [Makefile:258: setjmp/tests] Error 2
make[3]: Leaving directory '/export/ssd/build/tools-build/glibc-many/src/glibc'
make[2]: *** [Makefile:9: check] Error 2
make[2]: Leaving directory
'/export/ssd/build/tools-build/glibc-many/build/glibcs/i686-gnu/glibc'

FAIL: glibcs-i686-gnu check

-- 
H.J.

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Samuel Thibault]

H.J. Lu, le lun. 03 déc. 2018 08:17:23 -0800, a ecrit:
> On Mon, Dec 3, 2018 at 5:42 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> >
> > Joseph Myers, le lun. 03 déc. 2018 13:23:48 +0000, a ecrit:
> > > On Sat, 1 Dec 2018, Samuel Thibault wrote:
> > >
> > > > 2018-12-01  Samuel Thibault  <samuel.thibault@ens-lyon.org>
> > > >
> > > >     * scripts/gen-as-const.py (main): Avoid emitting empty line when
> > > >     there is no element in `consts'.
> > >
> > > OK, please commit.
> >
> > Done so, thanks.
> 
> I still got
> 
> /export/ssd/build/tools-build/glibc-many/src/glibc/csu/../sysdeps/i386/start.S:122:
> undefined reference to `main'

Yes, see my second mail in the same thread:

“
That said, it's not enough, gen_test() does not emit a main() function,
and thus the test build fails with missing reference to main(). Is there
a strong reason for making the emission of code lazy with the started
boolean?
”

which needs to be discussed too.

Samuel

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Joseph Myers]

On Sat, 1 Dec 2018, Samuel Thibault wrote:

> That said, it's not enough, gen_test() does not emit a main() function,
> and thus the test build fails with missing reference to main(). Is there
> a strong reason for making the emission of code lazy with the started
> boolean?

The text can't be emitted at the start of the file, in the case of 
computing constants, because that would mean #includes from the .sym file 
end up inside a function.  It so happens that with this version (not with 
the previous one) that's not an issue for test generation, but it seems 
best to me to keep the logic for test generation and constant computation 
as similar as possible.

How does this patch seem to fix the unintended difference from the awk 
script?


Make gen-as-const.py handle '--' consistently with awk script.

It was reported in
<https://sourceware.org/ml/libc-alpha/2018-12/msg00045.html> that
gen-as-const.py fails to generate test code in the case where a .sym
file has no symbols in it, so resulting in a test failing to link for
Hurd.

The relevant difference from the old awk script is that the old script
treated '--' lines as indicating that the text to do at the start of
the test (or file used to compute constants) should be output at that
point if not already output, as well as treating lines with actual
entries for constants like that.  This patch changes gen-as-const.py
accordingly, making it the sole responsibility of the code parsing
.sym files to determine when such text should be output and ensuring
it's always output at some point even if there are no symbols and no
'--' lines, since not outputting it means the test fails to link.
Handling '--' like that also avoids any problems that would arise if
the first entry for a symbol were inside #ifdef (since the text in
question must not be output inside #ifdef).

Tested for x86_64, and with build-many-glibcs.py for i686-gnu.  Note
that there are still compilation test failures for i686-gnu
(linknamespace tests, possibly arising from recent posix_spawn-related
changes).

2018-12-03  Joseph Myers  <joseph@codesourcery.com>

	* scripts/gen-as-const.py (compute_c_consts): Take an argument
	'START' to indicate that start text should be output.
	(gen_test): Likewise.
	(main): Generate 'START' for first symbol or '--' line, or at end
	of input if not previously generated.

diff --git a/scripts/gen-as-const.py b/scripts/gen-as-const.py
index cabf401ed1..eb85ef1aa0 100644
--- a/scripts/gen-as-const.py
+++ b/scripts/gen-as-const.py
@@ -34,28 +34,28 @@ def compute_c_consts(sym_data, cc):
     """Compute the values of some C constants.
 
     The first argument is a list whose elements are either strings
-    (preprocessor directives) or pairs of strings (a name and a C
+    (preprocessor directives, or the special string 'START' to
+    indicate this function should insert its initial boilerplate text
+    in the output there) or pairs of strings (a name and a C
     expression for the corresponding value).  Preprocessor directives
     in the middle of the list may be used to select which constants
     end up being evaluated using which expressions.
 
     """
     out_lines = []
-    started = False
     for arg in sym_data:
         if isinstance(arg, str):
-            out_lines.append(arg)
+            if arg == 'START':
+                out_lines.append('void\ndummy (void)\n{')
+            else:
+                out_lines.append(arg)
             continue
         name = arg[0]
         value = arg[1]
-        if not started:
-            out_lines.append('void\ndummy (void)\n{')
-            started = True
         out_lines.append('asm ("@@@name@@@%s@@@value@@@%%0@@@end@@@" '
                          ': : \"i\" ((long int) (%s)));'
                          % (name, value))
-    if started:
-        out_lines.append('}')
+    out_lines.append('}')
     out_lines.append('')
     out_text = '\n'.join(out_lines)
     with tempfile.TemporaryDirectory() as temp_dir:
@@ -89,32 +89,32 @@ def gen_test(sym_data):
 
     """
     out_lines = []
-    started = False
     for arg in sym_data:
         if isinstance(arg, str):
-            out_lines.append(arg)
+            if arg == 'START':
+                out_lines.append('#include <stdint.h>\n'
+                                 '#include <stdio.h>\n'
+                                 '#include <bits/wordsize.h>\n'
+                                 '#if __WORDSIZE == 64\n'
+                                 'typedef uint64_t c_t;\n'
+                                 '# define U(n) UINT64_C (n)\n'
+                                 '#else\n'
+                                 'typedef uint32_t c_t;\n'
+                                 '# define U(n) UINT32_C (n)\n'
+                                 '#endif\n'
+                                 'static int\n'
+                                 'do_test (void)\n'
+                                 '{\n'
+                                 # Compilation test only, using static
+                                 # assertions.
+                                 '  return 0;\n'
+                                 '}\n'
+                                 '#include <support/test-driver.c>')
+            else:
+                out_lines.append(arg)
             continue
         name = arg[0]
         value = arg[1]
-        if not started:
-            out_lines.append('#include <stdint.h>\n'
-                             '#include <stdio.h>\n'
-                             '#include <bits/wordsize.h>\n'
-                             '#if __WORDSIZE == 64\n'
-                             'typedef uint64_t c_t;\n'
-                             '# define U(n) UINT64_C (n)\n'
-                             '#else\n'
-                             'typedef uint32_t c_t;\n'
-                             '# define U(n) UINT32_C (n)\n'
-                             '#endif\n'
-                             'static int\n'
-                             'do_test (void)\n'
-                             '{\n'
-                             # Compilation test only, using static assertions.
-                             '  return 0;\n'
-                             '}\n'
-                             '#include <support/test-driver.c>')
-            started = True
         out_lines.append('_Static_assert (U (asconst_%s) == (c_t) (%s), '
                          '"value of %s");'
                          % (name, value, name))
@@ -134,6 +134,7 @@ def main():
     args = parser.parse_args()
     sym_data = []
     with open(args.sym_file, 'r') as sym_file:
+        started = False
         for line in sym_file:
             line = line.strip()
             if line == '':
@@ -143,12 +144,17 @@ def main():
                 sym_data.append(line)
                 continue
             words = line.split(maxsplit=1)
+            if not started:
+                sym_data.append('START')
+                started = True
             # Separator.
             if words[0] == '--':
                 continue
             name = words[0]
             value = words[1] if len(words) > 1 else words[0]
             sym_data.append((name, value))
+        if not started:
+            sym_data.append('START')
     if args.test:
         print(gen_test(sym_data))
     else:

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix test-as-const-jmp_buf-ssp.c generation on gnu-i386

[Samuel Thibault]

Joseph Myers, le lun. 03 déc. 2018 18:25:38 +0000, a ecrit:
> On Sat, 1 Dec 2018, Samuel Thibault wrote:
> 
> > That said, it's not enough, gen_test() does not emit a main() function,
> > and thus the test build fails with missing reference to main(). Is there
> > a strong reason for making the emission of code lazy with the started
> > boolean?
> 
> The text can't be emitted at the start of the file, in the case of 
> computing constants, because that would mean #includes from the .sym file 
> end up inside a function.  It so happens that with this version (not with 
> the previous one) that's not an issue for test generation, but it seems 
> best to me to keep the logic for test generation and constant computation 
> as similar as possible.
> 
> How does this patch seem to fix the unintended difference from the awk 
> script?

That should be doing it indeed.

Thanks!
Samuel

> 
> Make gen-as-const.py handle '--' consistently with awk script.
> 
> It was reported in
> <https://sourceware.org/ml/libc-alpha/2018-12/msg00045.html> that
> gen-as-const.py fails to generate test code in the case where a .sym
> file has no symbols in it, so resulting in a test failing to link for
> Hurd.
> 
> The relevant difference from the old awk script is that the old script
> treated '--' lines as indicating that the text to do at the start of
> the test (or file used to compute constants) should be output at that
> point if not already output, as well as treating lines with actual
> entries for constants like that.  This patch changes gen-as-const.py
> accordingly, making it the sole responsibility of the code parsing
> .sym files to determine when such text should be output and ensuring
> it's always output at some point even if there are no symbols and no
> '--' lines, since not outputting it means the test fails to link.
> Handling '--' like that also avoids any problems that would arise if
> the first entry for a symbol were inside #ifdef (since the text in
> question must not be output inside #ifdef).
> 
> Tested for x86_64, and with build-many-glibcs.py for i686-gnu.  Note
> that there are still compilation test failures for i686-gnu
> (linknamespace tests, possibly arising from recent posix_spawn-related
> changes).
> 
> 2018-12-03  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* scripts/gen-as-const.py (compute_c_consts): Take an argument
> 	'START' to indicate that start text should be output.
> 	(gen_test): Likewise.
> 	(main): Generate 'START' for first symbol or '--' line, or at end
> 	of input if not previously generated.
> 
> diff --git a/scripts/gen-as-const.py b/scripts/gen-as-const.py
> index cabf401ed1..eb85ef1aa0 100644
> --- a/scripts/gen-as-const.py
> +++ b/scripts/gen-as-const.py
> @@ -34,28 +34,28 @@ def compute_c_consts(sym_data, cc):
>      """Compute the values of some C constants.
>  
>      The first argument is a list whose elements are either strings
> -    (preprocessor directives) or pairs of strings (a name and a C
> +    (preprocessor directives, or the special string 'START' to
> +    indicate this function should insert its initial boilerplate text
> +    in the output there) or pairs of strings (a name and a C
>      expression for the corresponding value).  Preprocessor directives
>      in the middle of the list may be used to select which constants
>      end up being evaluated using which expressions.
>  
>      """
>      out_lines = []
> -    started = False
>      for arg in sym_data:
>          if isinstance(arg, str):
> -            out_lines.append(arg)
> +            if arg == 'START':
> +                out_lines.append('void\ndummy (void)\n{')
> +            else:
> +                out_lines.append(arg)
>              continue
>          name = arg[0]
>          value = arg[1]
> -        if not started:
> -            out_lines.append('void\ndummy (void)\n{')
> -            started = True
>          out_lines.append('asm ("@@@name@@@%s@@@value@@@%%0@@@end@@@" '
>                           ': : \"i\" ((long int) (%s)));'
>                           % (name, value))
> -    if started:
> -        out_lines.append('}')
> +    out_lines.append('}')
>      out_lines.append('')
>      out_text = '\n'.join(out_lines)
>      with tempfile.TemporaryDirectory() as temp_dir:
> @@ -89,32 +89,32 @@ def gen_test(sym_data):
>  
>      """
>      out_lines = []
> -    started = False
>      for arg in sym_data:
>          if isinstance(arg, str):
> -            out_lines.append(arg)
> +            if arg == 'START':
> +                out_lines.append('#include <stdint.h>\n'
> +                                 '#include <stdio.h>\n'
> +                                 '#include <bits/wordsize.h>\n'
> +                                 '#if __WORDSIZE == 64\n'
> +                                 'typedef uint64_t c_t;\n'
> +                                 '# define U(n) UINT64_C (n)\n'
> +                                 '#else\n'
> +                                 'typedef uint32_t c_t;\n'
> +                                 '# define U(n) UINT32_C (n)\n'
> +                                 '#endif\n'
> +                                 'static int\n'
> +                                 'do_test (void)\n'
> +                                 '{\n'
> +                                 # Compilation test only, using static
> +                                 # assertions.
> +                                 '  return 0;\n'
> +                                 '}\n'
> +                                 '#include <support/test-driver.c>')
> +            else:
> +                out_lines.append(arg)
>              continue
>          name = arg[0]
>          value = arg[1]
> -        if not started:
> -            out_lines.append('#include <stdint.h>\n'
> -                             '#include <stdio.h>\n'
> -                             '#include <bits/wordsize.h>\n'
> -                             '#if __WORDSIZE == 64\n'
> -                             'typedef uint64_t c_t;\n'
> -                             '# define U(n) UINT64_C (n)\n'
> -                             '#else\n'
> -                             'typedef uint32_t c_t;\n'
> -                             '# define U(n) UINT32_C (n)\n'
> -                             '#endif\n'
> -                             'static int\n'
> -                             'do_test (void)\n'
> -                             '{\n'
> -                             # Compilation test only, using static assertions.
> -                             '  return 0;\n'
> -                             '}\n'
> -                             '#include <support/test-driver.c>')
> -            started = True
>          out_lines.append('_Static_assert (U (asconst_%s) == (c_t) (%s), '
>                           '"value of %s");'
>                           % (name, value, name))
> @@ -134,6 +134,7 @@ def main():
>      args = parser.parse_args()
>      sym_data = []
>      with open(args.sym_file, 'r') as sym_file:
> +        started = False
>          for line in sym_file:
>              line = line.strip()
>              if line == '':
> @@ -143,12 +144,17 @@ def main():
>                  sym_data.append(line)
>                  continue
>              words = line.split(maxsplit=1)
> +            if not started:
> +                sym_data.append('START')
> +                started = True
>              # Separator.
>              if words[0] == '--':
>                  continue
>              name = words[0]
>              value = words[1] if len(words) > 1 else words[0]
>              sym_data.append((name, value))
> +        if not started:
> +            sym_data.append('START')
>      if args.test:
>          print(gen_test(sym_data))
>      else:
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com
> 

-- 
Samuel
Who wants to remember that escape-x-alt-control-left shift-b puts you into
super-edit-debug-compile mode?
(Discussion in comp.os.linux.misc on the intuitiveness of commands, especially
Emacs.)