Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
On Tue, Oct 30, 2018 at 08:58:46PM -0500, Jacob Bachmeyer wrote: > Lastly, is a soft dependency on GNU awk acceptable for DejaGnu or > would I need to rewrite my testsuite summarization tool in Tcl or > Expect before contributing it for use with the DejaGnu testsuite? I am happy for contributed tools like this to be written in GNU awk. Please try and use portal AWK if you can, but if not, then perhaps give the file a .gawk extension to make it clear that you need GNU awk? Cheers, Ben ___ Bug-dejagnu mailing list Bug-dejagnu@gnu.org https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
Ben Elliston wrote:
On Mon, Oct 29, 2018 at 11:17:33PM -0500, Jacob Bachmeyer wrote:
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?
You can tell DejaGnu to use a different site.exp using $DEJAGNU. Why
not create a different site.exp for the testsuite (testsuite/site.exp)
and then set $DEJAGNU in the Makefile rule?
Would that work?
Part of the problem is that the site.exp Automake produces actually is
important -- it provides information about where the sources are when
building in a separate directory. Automake is correct here, and the
DejaGnu testsuite (and possibly DejaGnu itself) is wrong.
The DejaGnu manual states that a testsuite is required to be in a
directory named "testsuite" that should be in the tool's source
directory. Considering DejaGnu's history with Cygnus, I believe that
Automake's $(srcdir) is the "tool's source directory" mentioned here.
The srcdir for the DejaGnu testsuite is overridden on the command line,
but gets partially overridden by the Automake-provided site.exp.
Fortunately, the only effect of this seems to be an inability to find
tool init files, which will be a problem when the MULTIPASS test expects
"set MULTIPASS ..." to be executed from its tool init file, but is not a
problem at the moment, since libdejagnu.exp is empty and runtest.exp
appears to be unneeded.
While the command line srcdir takes precedence over a value from
site.exp, there is a window between reading site.exp and reparsing the
command line, and the search for the tool init file occurs in this window.
I suggest adding a search path for the tool init file and specifically
supporting running DejaGnu at both top-level and in the testsuite
directory. (Both of those can currently work, so breaking either would
be bad.) I suggest searching for a tool init file on
${srcdir}/testsuite/lib/tool/${tool}.exp:${srcdir}/testsuite/lib/${tool}.exp:${srcdir}/lib/${tool}.exp
The last item on that path is the current behavior, on which some
testsuites may depend. The first item is an enhancement proposal to
allow tool init files to be kept separate from local testsuite library
modules. Additional processing during the search would eliminate
"testsuite/testsuite/" pairs if ${srcdir} ends with "testsuite".
Unless overridden, DejaGnu's srcdir should match Automake's srcdir.
There will need to be a different site.exp for the options tests
eventually, but that is a different problem from DejaGnu misusing the
Automake-provided site.exp.
Also, where in the documentation should I insert a section for the
configuration files DejaGnu searches? A new section in Reference?
Lastly, is a soft dependency on GNU awk acceptable for DejaGnu or would
I need to rewrite my testsuite summarization tool in Tcl or Expect
before contributing it for use with the DejaGnu testsuite?
-- Jacob
___
Bug-dejagnu mailing list
Bug-dejagnu@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
Andreas Schwab wrote:
On Okt 29 2018, Jacob Bachmeyer wrote:
Andreas Schwab wrote:
On Okt 28 2018, Jacob Bachmeyer 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.
Then "$multipass_name" wouldn't work either. In fact, tcl does not do
word splitting, so the quotes around the variables are actually useless.
You are right, and I overlooked that earlier. However, there are
currently no regression tests for this code, so I would suggest keeping
to minimal changes for now. I will remember your suggestion and will
probably write a cleanup patch for this after writing MULTIPASS
regression tests.
On a side note, I have been calling it "MULTIPASS" because that is the
name of the variable that activates this feature. What is the proper
name for this feature as it should appear in the manual?
-- Jacob
___
Bug-dejagnu mailing list
Bug-dejagnu@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
On Okt 29 2018, Jacob Bachmeyer wrote:
> Andreas Schwab wrote:
>> On Okt 28 2018, Jacob Bachmeyer 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.
Then "$multipass_name" wouldn't work either. In fact, tcl does not do
word splitting, so the quotes around the variables are actually useless.
Andreas.
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
___
Bug-dejagnu mailing list
Bug-dejagnu@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
On Okt 28 2018, Jacob Bachmeyer 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"
Andreas.
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
___
Bug-dejagnu mailing list
Bug-dejagnu@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp (follow-up)
On Sun, Oct 28, 2018 at 10:08:42PM -0500, Jacob Bachmeyer wrote: > On a related note, I note that the DejaGnu testsuite does not test > the multipass feature, nor does the feature seem to be properly > documented. Where would be the best place to insert something in > the manual describing MULTIPASS? (And where are the DocBook sources > in Git, or is Texinfo the master format now? If the latter, README > and possibly more should be updated.) The Texinfo documentation is the definitive source now. It would be great to have a more complete functional testsuite for DejaGnu. Patches welcome. Cheers, Ben signature.asc Description: PGP signature ___ Bug-dejagnu mailing list Bug-dejagnu@gnu.org https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
On Sun, Oct 28, 2018 at 07:52:55PM -0500, Jacob Bachmeyer wrote: > corrected patch: (also relative to commit > 81651abb04defb181f9c98bfcc55e077dcaea452) Ugh, don't do that. :-) Just send patche against HEAD. Thanks, Ben ___ Bug-dejagnu mailing list Bug-dejagnu@gnu.org https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp (follow-up)
Ben Elliston wrote:
On Sat, Oct 27, 2018 at 11:48:08PM -0500, Jacob Bachmeyer wrote:
A message that contains a '%' character will cause Tcl to raise an
error at the format in record_test in lib/framework.exp on line 803
as of commit 81651abb04defb181f9c98bfcc55e077dcaea452. This is a
"classic" format string vulnerability, except that Tcl catches it
and raises an error.
That bug goes all the way back to 2001, when lib/framework.exp was first
checked in to whatever became the current Git repository, so documenting
a way to detect the bugfix might be helpful -- a testsuite can work
around the bug by doubling any '%' characters, but after the bug is
fixed, that will result in "%%" in the output.
Perhaps [regexp {%s: \$message} [info body record_test]] would return
true iff running under an affected framework version? Or should
testsuites simply require the bug be fixed? Should that test still be
done in order to produce a clear error message if using an affected
DejaGnu version?
On a related note, I note that the DejaGnu testsuite does not test the
multipass feature, nor does the feature seem to be properly documented.
Where would be the best place to insert something in the manual
describing MULTIPASS? (And where are the DocBook sources in Git, or is
Texinfo the master format now? If the latter, README and possibly more
should be updated.)
How best to test MULTIPASS? (A simple regression test would have
instantly caught my previous mistake.) What are the specifications for
MULTIPASS?
-- Jacob
___
Bug-dejagnu mailing list
Bug-dejagnu@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
Ben Elliston wrote:
On Sat, Oct 27, 2018 at 11:48:08PM -0500, Jacob Bachmeyer wrote:
A message that contains a '%' character will cause Tcl to raise an
error at the format in record_test in lib/framework.exp on line 803
as of commit 81651abb04defb181f9c98bfcc55e077dcaea452. This is a
"classic" format string vulnerability, except that Tcl catches it
and raises an error.
Thanks!
Oops... I have since realized that the patch in my previous message was
subtly wrong -- it reversed the order of the pass name and message. I
did not notice because I had not actually looked at the test log -- the
program I use for summarizing the log keys off of the "Running pass"
messages instead and only counts passes/fails/etc. for a nice summary
table. /Mea culpa/; here is a corrected patch.
corrected patch: (also relative to commit
81651abb04defb181f9c98bfcc55e077dcaea452)
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"]
} else {
set message "$type: $message"
}
-- Yours sheepishly, Jacob
___
Bug-dejagnu mailing list
Bug-dejagnu@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-dejagnu
Re: [Bug-dejagnu] BUG: improper format string construction in framework.exp
On Sat, Oct 27, 2018 at 11:48:08PM -0500, Jacob Bachmeyer wrote: > A message that contains a '%' character will cause Tcl to raise an > error at the format in record_test in lib/framework.exp on line 803 > as of commit 81651abb04defb181f9c98bfcc55e077dcaea452. This is a > "classic" format string vulnerability, except that Tcl catches it > and raises an error. Thanks! Ben signature.asc Description: PGP signature ___ Bug-dejagnu mailing list Bug-dejagnu@gnu.org https://lists.gnu.org/mailman/listinfo/bug-dejagnu
