On Thu, 2021-08-19 at 19:59 +0100, Iain Sandoe wrote:
> Hi,
>
> Preface:
>
> this is the last patch for now in my series - with this applied
> Darwin
> reports the same results as Linux (at least, for modern x86_64
> platform versions).
>
> Note
> a) that the expect expression in {fixed}host_execute seems to depend
> on the assumption that the dejagnu.h output is used by the testcase
> and that the executable’s output can be seen to end with the totals
> produced there (which might in itself be erroneous, see 3).
>
> b) the main GCC testsuite processing does not do this; rather the
> expect
> expression is somewhat simple and the output from the executable
> is copied into a secondary buffer, which is then processed by
> prune expressions and then to find the requisite matches (so most
> of the issues seen below do not occur there).
>
> ==== patch discussion
>
> The current 'fixed_host_execute' implementation fails on Darwin
> platforms for a number of reasons:
>
> 1/ If the sub-process spawn fails (e.g. because of missing or mal-
> formed params); rather than reporting the fail output into the
> match stream, as indicated by the expect manual, it terminates
> the script.
>
> - We fix this by (a) checking that the executable is valid as well
> as existing (b) we put the spawn into a catch block and report
> a failure.
>
> 2/ There is no recovery path at all for a buffer-full case (and we
> do see buffer-full events with the default sizes).
>
> - Added by the patch here, however it is not as sophisticated as
> the methods used by dejagnu internally. Here we set the process
> to be "nowait" and then close the connection - with the intent
> that this will terminate the spawned process.
>
> 3/ The expect logic assumes that 'Totals:' is a valid indicator
> for the end of the spawned process output. This is not true
> even for the default dejagnu header (there are a number of
> additional reporting lines after). In addition to this, there
> are some tests that intentionally produce more output after
> the totals report (and there are tests that do not use that
> mechanism at all).
>
> The effect is the we might arrive at the "wait" for the spawned
> process to finish - but that process might not have completed
> all its output. For Darwin, at least that causes a deadlock
> between expect and the spawnee - the latter is doing a non-
> cancellable write and the former is waiting for the latter to
> terminate. For some reason this does not seem to affect Linux
> perhaps the pty implementation allows the write(s) are able to
> proceed even though there is no reader.
>
> - This is fixed by modifying the loop termination condition to be
> either EOF (which will be the 'correct' condition) or a timeout
> which would represent an error either in the runtime or in the
> parsing of the output. As added precautions, we only try to
> wait if there is a correcly-spawned process, and we are also
> specific about which process we are waiting for.
>
> 4/ Darwin appears to have a bug in either the tcl or termios
> 'cooking' code that ocassionally inserts an additional CR char
> into the stream - thus '\n' => '\r\r\n' instead of '\r\n'. The
> original program output is correct (it only contains a single
> \n) - the additional character is being inserted somewhere in
> the translations applied before the output reaches expect.
>
> The logic of this expect implementation does not tolerate single
> \r or \n characters (it will fail with a timeout or buffer-full
> if that occurs).
>
> - This is fixed by having a line-end match that is adjusted for
> Darwin.
>
> 5/ The default buffer size does seem to be too small in some cases
> noting that GCC uses 10000 as the match buffer size and the
> default is 2000.
>
> - Fixed by increasing the size to 8192.
>
> 6/ There is a somewhat arbitrary dumping of output where we match
> ^$prefix\tSOMETHING... and then process the something. This
> essentially allows the match to start at any place in the buffer
> following any collection of non-line-end chars.
>
> - Fixed by amending the match for 'general' lines to accommodate
> these cases, and reporting such lines to the log. At least this
> should allow debugging of any cases where output that should be
> recognized is being dropped.
>
> tested on i686, x86_64-darwin, x86_64,powerpc64-linux,
Which versions of DejaGnu, BTW?
> OK for master?
Did you try this with RUN_UNDER_VALGRIND set? Assuming that that still
works, yes, looks good to me.
Thanks for the patch
Dave