[PATCH 0/4] gprofng: small testsuite adjustments

[PATCH 0/4] gprofng: small testsuite adjustments

[Jan Beulich]

While the latter two patches are purely cosmetic, I wonder how things have
worked properly without the former two. I'm therefore not going to exclude that
the changes done really need to be conditional upon some environmental aspects,
but it's not clear to me what these would be (or why).

Beyond / independent of these small fixes I'm still concerned by the time
running these testcases takes: The few tests here take quite a bit longer than
building _and_ testing all of the rest of binutils (not gdb of course) for
those targets where gprofng is actually available. One aspect I'm wondering
about in particular: What is it that is actually tested when the binutils build
is a cross one? The produced binaries are host executables, so it's unclear to
me what meaning it has to run on them a profiler (supposedly) targeting another
architecture. Eliminating the testing in such cases would already speed up the
mass testing of many targets in a noticable way.

There are still "warning: always_inline function might not be inlinable"
instances left with the gcc version I'm using, but I can't tell whether that's
a reason for worrying.

1: adjust linking of synprog
2: correct names for signal handling tests
3: correct line continuation in endcases.c
4: eliminate bogus casts

Jan

[PATCH 1/4] gprofng/testsuite: adjust linking of synprog

[Jan Beulich]

In order for so_syn.so and so_syx.so to be able to access the main
program's "testtime" variable, that variable needs exposing in the
dynamic symbol table. Since this is a test program only, do it the brute
force way and simply expose all global symbols.

--- a/gprofng/testsuite/gprofng.display/synprog/Makefile
+++ b/gprofng/testsuite/gprofng.display/synprog/Makefile
@@ -50,7 +50,7 @@ HDRS= \
 
 $(TARGET): $(SRCS) $(HDRS) so_syx.so so_syn.so
 	@echo " ---- Build: $@ -----"
-	$(CC) $(CFLAGS) -o $@ $(SRCS) -ldl -lc -lrt
+	$(CC) $(CFLAGS) -Wl,-E -o $@ $(SRCS) -ldl -lc -lrt
 
 so_syx.so: $(srcdir)/so_syx.c
 	@echo " ---- Build: $@ -----"

[PATCH 2/4] gprofng/testsuite: correct names for signal handling tests

[Jan Beulich]

The signal handling tests spend most of their time in the signal
handlers, and hence for profile output to match anything in program
output, the respective name fields need to hold the handler function
names. This converts both respective tests from "unresolved" to actually
succeeding.

--- a/gprofng/testsuite/gprofng.display/synprog/synprog.c
+++ b/gprofng/testsuite/gprofng.display/synprog/synprog.c
@@ -184,7 +184,7 @@ struct scripttab scripttab[] = {
   {"sched",     sched,          "sched",        0,  1},
   {"so",        callso,         "callso",       0,  0},
   {"sx",        callsx,         "callsx",       0,  0},
-  {"sig",       sigtime,        "sigtime",      0,  1},
+  {"sig",       sigtime,        "sigtime_handler", 0,  1},
   {"sigprof",   sigprof,        "sigprof",      1,  0},
   {"sigprof0",  sigprof,        "sigprof",      0,  0},
   {"sigprofh",  sigprofh,       "sigprofh",     1,  0},
@@ -197,7 +197,7 @@ struct scripttab scripttab[] = {
   {"uf",        underflow,      "underflow",    0,  1},
   {"forkexec",  do_forkexec,    "forkexec",     0,  0},
   {"vforkexec", do_vforkexec,   "vforkexec",    0,  0},
-  {"uwdc",      unwindcases,    "unwindcases",  0,  0},
+  {"uwdc",      unwindcases,    "unwindcases_handler", 0,  0},
   {NULL, NULL, NULL, 0, 0}
 };
 

[PATCH 3/4] gprofng/testsuite: correct line continuation in endcases.c

[Jan Beulich]

A backslash used to indicate line continuation (in a macro definition
here) is not supposed to be followed by blanks or other white space; the
end-of-line indicator is to follow immediately.

--- a/gprofng/testsuite/gprofng.display/synprog/endcases.c
+++ b/gprofng/testsuite/gprofng.display/synprog/endcases.c
@@ -52,7 +52,7 @@ int x2M = 2000000;
 int x8M = 8000000;
 
 /* define a macro that burns CPU time */
-#define burncpu(nn) \ 
+#define burncpu(nn) \
         x = 0; \
         for (j = 0; j < (nn * x8M); j++) { \
                  x = x + 1; \

[PATCH 4/4] gprofng/testsuite: eliminate bogus casts

[Jan Beulich]

Casting pointers to unsigned int is generally problematic and hence
compilers tend to warn about such. While here they're used only in
fprintf(), it still seems better to omit such casts, even if only to
avoid setting bad precedents.

--- a/gprofng/testsuite/gprofng.display/synprog/so_syn.c
+++ b/gprofng/testsuite/gprofng.display/synprog/so_syn.c
@@ -32,7 +32,7 @@ so_cputime ()
   /* put a memory leak in here */
   (void) malloc (13);
 
-  fprintf (stderr, "so_burncpu @ 0x%08x\n", (unsigned int) so_burncpu);
+  fprintf (stderr, "so_burncpu @ %p\n", so_burncpu);
   so_burncpu ();
 
   wlog ("end of so_cputime", NULL);
--- a/gprofng/testsuite/gprofng.display/synprog/so_syx.c
+++ b/gprofng/testsuite/gprofng.display/synprog/so_syx.c
@@ -32,7 +32,7 @@ sx_cputime ()
   /* put a memory leak in here */
   (void) malloc (13);
 
-  fprintf (stderr, "sx_burncpu @ 0x%08x\n", (unsigned int) sx_burncpu);
+  fprintf (stderr, "sx_burncpu @ %p\n", sx_burncpu);
   sx_burncpu ();
   wlog ("end of sx_cputime", NULL);
   return 13;

Re: [PATCH 0/4] gprofng: small testsuite adjustments

[Jan Beulich]

On 16.12.2022 09:26, Jan Beulich via Binutils wrote:
> Beyond / independent of these small fixes I'm still concerned by the time
> running these testcases takes: The few tests here take quite a bit longer than
> building _and_ testing all of the rest of binutils (not gdb of course) for
> those targets where gprofng is actually available. One aspect I'm wondering
> about in particular: What is it that is actually tested when the binutils build
> is a cross one? The produced binaries are host executables, so it's unclear to
> me what meaning it has to run on them a profiler (supposedly) targeting another
> architecture. Eliminating the testing in such cases would already speed up the
> mass testing of many targets in a noticable way.

And just now I spotted this

set pltf [exec uname -i]

in display.exp. Shouldn't this check that the resulting $pltf actually
matches the build target before performing any of the tests?

Jan

[PATCH 5/4] gprofng/testsuite: skip Java test without JDK

[Jan Beulich]

There's no point in even trying the Java test when gprofng was built
without Java support, and when the building of the constituents of the
testcase also would fail. On such systems this converts the respective
tests from "unresolved" to "unsupported", making the overall testsuite
run no longer report failure just because of this.
---
An alternative without changing display.exp would look to be to force
GPROFNG_BROKEN_JAVAC to "yes" in the runtest invocation if $jdk_inc is
empty.

--- a/gprofng/Makefile.am
+++ b/gprofng/Makefile.am
@@ -61,6 +61,7 @@ check-DEJAGNU: site.exp development.exp
 	runtest=$(RUNTEST); \
 	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
 	  $$runtest --tool $(DEJATOOL) --srcdir $${srcroot}/testsuite \
+		JDK_INC="$(jdk_inc)" \
 		GPROFNG_BROKEN_JAVAC="$(GPROFNG_BROKEN_JAVAC)" \
 		MAKE="$(MAKE)" CC="$(CC)" CFLAGS="$(CFLAGS) $(PTHREAD_CFLAGS)" \
 		LDFLAGS="$(LDFLAGS)" LIBS="$(PTHREAD_LIBS) $(LIBS)" \
--- a/gprofng/Makefile.in
+++ b/gprofng/Makefile.in
@@ -938,6 +938,7 @@ uninstall-am:
 @TCL_TRY_TRUE@	runtest=$(RUNTEST); \
 @TCL_TRY_TRUE@	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
 @TCL_TRY_TRUE@	  $$runtest --tool $(DEJATOOL) --srcdir $${srcroot}/testsuite \
+@TCL_TRY_TRUE@		JDK_INC="$(jdk_inc)" \
 @TCL_TRY_TRUE@		GPROFNG_BROKEN_JAVAC="$(GPROFNG_BROKEN_JAVAC)" \
 @TCL_TRY_TRUE@		MAKE="$(MAKE)" CC="$(CC)" CFLAGS="$(CFLAGS) $(PTHREAD_CFLAGS)" \
 @TCL_TRY_TRUE@		LDFLAGS="$(LDFLAGS)" LIBS="$(PTHREAD_LIBS) $(LIBS)" \
--- a/gprofng/testsuite/gprofng.display/display.exp
+++ b/gprofng/testsuite/gprofng.display/display.exp
@@ -54,6 +54,7 @@ switch $pltf {
   }
 }
 
+global JDK_INC
 global GPROFNG_BROKEN_JAVAC
 
 foreach line $table {
@@ -63,7 +64,8 @@ foreach line $table {
 
   verbose [file rootname $line]
   verbose running display test $line
-  if { $GPROFNG_BROKEN_JAVAC == "yes" && $dir == "jsynprog" } {
+  if { $dir == "jsynprog"
+       && ($JDK_INC == "" || $GPROFNG_BROKEN_JAVAC == "yes") } {
     unsupported $dir
   } else {
     run_display_test $dir $cflags $gprofflags

Re: [PATCH 0/4] gprofng: small testsuite adjustments

[Vladimir Mezentsev]

On 12/16/22 00:26, Jan Beulich wrote:
> While the latter two patches are purely cosmetic, I wonder how things have
> worked properly without the former two. I'm therefore not going to exclude that
> the changes done really need to be conditional upon some environmental aspects,
> but it's not clear to me what these would be (or why).
>
> Beyond / independent of these small fixes I'm still concerned by the time
> running these testcases takes: The few tests here take quite a bit longer than
> building _and_ testing all of the rest of binutils (not gdb of course) for
> those targets where gprofng is actually available. One aspect I'm wondering
> about in particular: What is it that is actually tested when the binutils build
> is a cross one?

Nothing.

>   The produced binaries are host executables, so it's unclear to
> me what meaning it has to run on them a profiler (supposedly) targeting another
> architecture. Eliminating the testing in such cases would already speed up the
> mass testing of many targets in a noticable way.

Agree.
We need to ignore `make check` for the cross build.

I'd rather test the installed binaries.
Can we add the check-install  target in Makefile for this ?

I use a big testsuite for gprofng.
The testsuite has ~400 tests and takes ~4 hours.
The problem is that tests are only available in Oracle.

> There are still "warning: always_inline function might not be inlinable"
> instances left with the gcc version I'm using, but I can't tell whether that's
> a reason for worrying.
>
> 1: adjust linking of synprog
> 2: correct names for signal handling tests
> 3: correct line continuation in endcases.c
> 4: eliminate bogus casts

Your patches look good to me.

Thank you for the gprofng improvement.
-Vladimir