#2113 I/O redirector ApiContext leaves clearConditions uninitialized

[Josep Maria Blasco]

Summary: ApiContext(RexxIORedirectorContext *c, bool blocking), the
non-blocking constructor used by the redirector query functions, never sets
its clearConditions member, and the destructor tests it. With a non-zero
garbage value, a call such as context->IsInputRedirected() from a command
handler clears the pending condition of the command's activation, and it
does so without holding the interpreter lock.

Where: interpreter/api/ContextApi.hpp. ApiContext has nine
constructors; eight of them assign clearConditions (to false, except the
blocking redirector one, which sets it to true on purpose). The
non-blocking redirector constructor only sets releaseLock, activity and
context:

inline ApiContext(RexxIORedirectorContext *c, bool blocking)
{
    // we need to cleanup on exit
    releaseLock = blocking;
    activity = contextToActivity(c);
    context = contextToActivation(c);
    context->enableConditionTraps();
}

and the destructor does

if (clearConditions)
{
    context->clearCondition();
}

The constructor is used by IsInputRedirected, IsOutputRedirected,
IsErrorRedirected and AreOutputAndErrorSameTarget
(interpreter/api/CallContextStubs.cpp, each with "this can be handled
without locking"). Unlike the other non-blocking constructors, which use
activity->getApiContext(), this one takes contextToActivation(c), the
activation the command handler is running for, so the condition it may
clear is the real one.

Effect: whether it happens depends on what is on the stack. In practice
the value is usually non-zero: building with
-fsanitize=undefined and calling IsInputRedirected() from a command
handler reports

ContextApi.hpp:257: runtime error: load of value 217, which is not a valid value for type 'bool'

(other runs: 254, 29, 47). A condition pending on the activation (raised by
an earlier context API call made by the handler, for instance one that
failed with a syntax error) can then be lost silently when the handler
afterwards asks about redirection, and the
write to the activation happens while another thread may hold the lock.
There is no deterministic reproducer, since it is a read of uninitialized
memory; UBSan shows it every time.

Fix (attached diff): initialize it like the other non-blocking
constructors, which do not hold the lock and must not touch the activation:

--- interpreter/api/ContextApi.hpp  (revision 13269)
+++ interpreter/api/ContextApi.hpp  (working copy)
@@ -195,6 +195,7 @@
      {
          // we need to cleanup on exit
          releaseLock = blocking;

+         clearConditions = false;
          activity = contextToActivity(c);
          context = contextToActivation(c);
          context->enableConditionTraps();

With it the UBSan report disappears. The full test suite (test/trunk r13269,
native, 402 groups, 24283 tests), run with this fix together with two other
fixes in the command handler code, gives the same results as without them:
no regressions, only one timing-dependent test that passes on rerun.

Found with UBSan while stress-testing the ooRexx/WASM port.

[Erich]

Committed code fix with revision [r13271]