Re: BUG: improper format string construction in framework.exp

Andreas Schwab wrote:
> On Okt 28 2018, Jacob Bachmeyer <[email protected]> wrote:
>   
>> diff --git a/lib/framework.exp b/lib/framework.exp
>> index 6cb93c5..50ac757 100644
>> --- a/lib/framework.exp
>> +++ b/lib/framework.exp
>> @@ -800,7 +800,7 @@ proc record_test { type message args } {
>>  
>>      global multipass_name
>>      if { $multipass_name != "" } {
>> -       set message [format "$type: %s: $message" "$multipass_name"]
>> +       set message [format "%s: %s: %s" "$type" "$multipass_name" "$message"]
>>     
>
> What's the point of using format in the first place?
>
>           set message "$type: $multipass_name: $message"
>   

That code is ancient and was in the initial commit in whatever became 
the DejaGnu Git repository.  I presume that some version of Tcl does not 
correctly interpolate variables if the name contains underscore.  Using 
concat (another option I had considered) is not correct because concat 
inserts spaces between its arguments.

I am currently seeking to improve DejaGnu's testsuite to actually check 
this mode.  I plan to write documentation patches for MULTIPASS and 
document a hot-patch that testsuites can use to fix this bug at run-time 
when using older DejaGnu.  (As of this writing, "older DejaGnu" appears 
to be every release, ever, that had support for MULTIPASS.)

The DejaGnu testsuite appears to have some strange bugs.  In particular, 
testsuite/runtest.all/options.exp overwrites the site.exp file that the 
Automake-generated rules produce and using "--tool runtest" causes the 
final summary to fail to be printed with an expect error:  "send: spawn 
id exp0 not open" when {clone_output "\n\t\t=== $tool Summary ===\n"} is 
run during log_and_exit.  Bizarrely, it works fine if --tool is not 
given.  Perhaps it is actually an expect bug?  But I do not have similar 
problems with other testsuites, only DejaGnu itself.  Fixing this is 
necessary to test MULTIPASS, because DejaGnu cannot run both single-pass 
and multiple-pass tests in the same run and Automake needs to test 
different tools to run DejaGnu more than once.

****

Overwriting site.exp appears to actually be important, because the 
DejaGnu testsuite needs srcdir to be .../dejagnu/testsuite and Automake 
sets srcdir to .../dejagnu; thus the need for "--srcdir 
$(srcdir)/testsuite" in RUNTESTDEFAULTFLAGS in Makefile.am.  How best to 
fix this misintegration with Automake?

What is different between DejaGnu runs with the --tool option given and 
runs without it?

How do the DejaGnu instances run by options.exp know to not actually run 
the testsuite, which would lead to infinite recursion?

****

For those interested:  the hot-patch is six lines of Tcl, hereby placed 
under GPLv2+ if it is non-trivial enough to matter (thus compatible with 
every affected use of DejaGnu) (DejaGnu has already accepted the patch 
under GPLv3+, so this is an additional permission to cover all cases 
with the older releases that were GPLv2+)

if { [regexp {%s: \$message} [info body record_test]] } {
    proc record_test [info args record_test] \
	[regsub -line {format "\$type: %s: \$message" "\$multipass_name"} \
	     [info body record_test] \
	     {format "%s: %s: %s" "$type" "$multipass_name" "$message"} ]
}

It must be placed into an init file, preferably the tool init file for each affected tool.


-- Jacob