* [PATCH 0/4] gprofng: small testsuite adjustments @ 2022-12-16 8:26 Jan Beulich 2022-12-16 8:28 ` [PATCH 1/4] gprofng/testsuite: adjust linking of synprog Jan Beulich ` (6 more replies) 0 siblings, 7 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 8:26 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] gprofng/testsuite: adjust linking of synprog 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich @ 2022-12-16 8:28 ` Jan Beulich 2022-12-16 8:29 ` [PATCH 2/4] gprofng/testsuite: correct names for signal handling tests Jan Beulich ` (5 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 8:28 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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: $@ -----" ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] gprofng/testsuite: correct names for signal handling tests 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich 2022-12-16 8:28 ` [PATCH 1/4] gprofng/testsuite: adjust linking of synprog Jan Beulich @ 2022-12-16 8:29 ` Jan Beulich 2022-12-16 8:30 ` [PATCH 3/4] gprofng/testsuite: correct line continuation in endcases.c Jan Beulich ` (4 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 8:29 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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} }; ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] gprofng/testsuite: correct line continuation in endcases.c 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich 2022-12-16 8:28 ` [PATCH 1/4] gprofng/testsuite: adjust linking of synprog Jan Beulich 2022-12-16 8:29 ` [PATCH 2/4] gprofng/testsuite: correct names for signal handling tests Jan Beulich @ 2022-12-16 8:30 ` Jan Beulich 2022-12-16 8:30 ` [PATCH 4/4] gprofng/testsuite: eliminate bogus casts Jan Beulich ` (3 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 8:30 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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; \ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4] gprofng/testsuite: eliminate bogus casts 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich ` (2 preceding siblings ...) 2022-12-16 8:30 ` [PATCH 3/4] gprofng/testsuite: correct line continuation in endcases.c Jan Beulich @ 2022-12-16 8:30 ` Jan Beulich 2022-12-16 8:36 ` [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich ` (2 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 8:30 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] gprofng: small testsuite adjustments 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich ` (3 preceding siblings ...) 2022-12-16 8:30 ` [PATCH 4/4] gprofng/testsuite: eliminate bogus casts Jan Beulich @ 2022-12-16 8:36 ` Jan Beulich 2022-12-16 9:13 ` [PATCH 5/4] gprofng/testsuite: skip Java test without JDK Jan Beulich 2022-12-16 20:16 ` [PATCH 0/4] gprofng: small testsuite adjustments Vladimir Mezentsev 6 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 8:36 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 5/4] gprofng/testsuite: skip Java test without JDK 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich ` (4 preceding siblings ...) 2022-12-16 8:36 ` [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich @ 2022-12-16 9:13 ` Jan Beulich 2022-12-16 20:16 ` [PATCH 0/4] gprofng: small testsuite adjustments Vladimir Mezentsev 6 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2022-12-16 9:13 UTC (permalink / raw) To: Vladimir Mezentsev; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] gprofng: small testsuite adjustments 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich ` (5 preceding siblings ...) 2022-12-16 9:13 ` [PATCH 5/4] gprofng/testsuite: skip Java test without JDK Jan Beulich @ 2022-12-16 20:16 ` Vladimir Mezentsev 6 siblings, 0 replies; 8+ messages in thread From: Vladimir Mezentsev @ 2022-12-16 20:16 UTC (permalink / raw) To: Jan Beulich; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-12-16 20:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-16 8:26 [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich 2022-12-16 8:28 ` [PATCH 1/4] gprofng/testsuite: adjust linking of synprog Jan Beulich 2022-12-16 8:29 ` [PATCH 2/4] gprofng/testsuite: correct names for signal handling tests Jan Beulich 2022-12-16 8:30 ` [PATCH 3/4] gprofng/testsuite: correct line continuation in endcases.c Jan Beulich 2022-12-16 8:30 ` [PATCH 4/4] gprofng/testsuite: eliminate bogus casts Jan Beulich 2022-12-16 8:36 ` [PATCH 0/4] gprofng: small testsuite adjustments Jan Beulich 2022-12-16 9:13 ` [PATCH 5/4] gprofng/testsuite: skip Java test without JDK Jan Beulich 2022-12-16 20:16 ` [PATCH 0/4] gprofng: small testsuite adjustments Vladimir Mezentsev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).