[P4Testgen] Resolve method call arguments before stepping into an ext…

…ern - preserve InOut references (#4255)

* Resolve method call arguments before stepping into the extern.

* Cleanup.

* Review comments.

* Review comments.

backends/p4tools/common/lib/symbolic_env.cpp

@@ -105,11 +105,14 @@ bool SymbolicEnv::isSymbolicValue(const IR::Node *node) {
     if (expr->is<IR::ConcolicVariable>()) {
         return true;
     }
-    // DefaultExpresssions are symbolic values.
+    // DefaultExpressions are symbolic values.
     if (expr->is<IR::DefaultExpression>()) {
         return true;
     }
-
+    // InOut references are symbolic when the resolved input argument is symbolic.
+    if (const auto *inout = expr->to<IR::InOutReference>()) {
+        return isSymbolicValue(inout->resolvedRef);
+    }
     // Symbolic values can be composed using several IR nodes.
     if (const auto *unary = expr->to<IR::Operation_Unary>()) {
         return (unary->is<IR::Neg>() || unary->is<IR::LNot>() || unary->is<IR::Cmpl>() ||

backends/p4tools/common/lib/trace_event_types.cpp

@@ -266,49 +266,31 @@ void ExtractFailure::print(std::ostream &os) const {
  *   Emit
  * ============================================================================================= */
 
-Emit::Emit(const IR::Expression *emitHeader,
-           std::vector<std::pair<IR::StateVariable, const IR::Expression *>> fields)
-    : emitHeader(emitHeader), fields(std::move(fields)) {}
+Emit::Emit(const IR::HeaderExpression *emitHeader) : emitHeader(emitHeader) {}
 
 const Emit *Emit::subst(const SymbolicEnv &env) const {
     std::vector<std::pair<IR::StateVariable, const IR::Expression *>> applyFields;
-    applyFields.reserve(fields.size());
-    for (const auto &field : fields) {
-        applyFields.emplace_back(field.first, env.subst(field.second));
-    }
-    return new Emit(emitHeader, applyFields);
+    return new Emit(env.subst(emitHeader)->checkedTo<IR::HeaderExpression>());
 }
 
 const Emit *Emit::apply(Transform &visitor) const {
-    std::vector<std::pair<IR::StateVariable, const IR::Expression *>> applyFields;
-    applyFields.reserve(fields.size());
-    for (const auto &field : fields) {
-        applyFields.emplace_back(field.first, field.second->apply(visitor));
-    }
-    return new Emit(emitHeader, applyFields);
+    return new Emit(emitHeader->apply(visitor)->checkedTo<IR::HeaderExpression>());
 }
 
 const Emit *Emit::evaluate(const Model &model, bool doComplete) const {
-    std::vector<std::pair<IR::StateVariable, const IR::Expression *>> applyFields;
-    applyFields.reserve(fields.size());
-    for (const auto &field : fields) {
-        if (Taint::hasTaint(field.second)) {
-            applyFields.emplace_back(field.first, &Taint::TAINTED_STRING_LITERAL);
-        } else {
-            applyFields.emplace_back(field.first, model.evaluate(field.second, doComplete));
-        }
-    }
-    return new Emit(emitHeader, applyFields);
+    return new Emit(
+        model.evaluateStructExpr(emitHeader, doComplete)->checkedTo<IR::HeaderExpression>());
 }
 
 void Emit::print(std::ostream &os) const {
-    os << "[Emit] " << emitHeader->toString() << " -> ";
-    for (const auto &field : fields) {
-        os << field.first->toString() << " = " << formatHexExpr(field.second, true);
-        if (field != fields.back()) {
-            os << " | ";
-        }
-    }
+    // Convert the header expression to a string and strip any new lines.
+    // TODO: Maybe there is a better way to format newlines?
+    std::stringstream assignStream;
+    emitHeader->dbprint(assignStream);
+    auto headerString = assignStream.str();
+    headerString.erase(std::remove(headerString.begin(), headerString.end(), '\n'),
+                       headerString.cend());
+    os << "[Emit]: " << headerString;
 }
 
 /* =============================================================================================

backends/p4tools/common/lib/trace_event_types.h

@@ -228,19 +228,15 @@ class ExtractFailure : public TraceEvent {
 /// A field being emitted by a deparser.
 class Emit : public TraceEvent {
  private:
-    /// The label of the emitted header. Either a PathExpression or a member.
-    const IR::Expression *emitHeader;
-
-    /// The list of fields and their values of the emitted header.
-    std::vector<std::pair<IR::StateVariable, const IR::Expression *>> fields;
+    /// The emitted header structure.
+    const IR::HeaderExpression *emitHeader;
 
  public:
     [[nodiscard]] const Emit *subst(const SymbolicEnv &env) const override;
     const Emit *apply(Transform &visitor) const override;
     [[nodiscard]] const Emit *evaluate(const Model &model, bool doComplete) const override;
 
-    Emit(const IR::Expression *emitHeader,
-         std::vector<std::pair<IR::StateVariable, const IR::Expression *>> fields);
+    explicit Emit(const IR::HeaderExpression *emitHeader);
     ~Emit() override = default;
     Emit(const Emit &) = default;
     Emit(Emit &&) = default;

backends/p4tools/common/lib/util.cpp

@@ -91,14 +91,14 @@ const IR::Constant *Utils::getRandConstantForType(const IR::Type_Bits *type) {
 
 const IR::MethodCallExpression *Utils::generateInternalMethodCall(
     cstring methodName, const std::vector<const IR::Expression *> &argVector,
-    const IR::Type *returnType) {
+    const IR::Type *returnType, const IR::ParameterList *paramList) {
     auto *args = new IR::Vector<IR::Argument>();
     for (const auto *expr : argVector) {
         args->push_back(new IR::Argument(expr));
     }
     return new IR::MethodCallExpression(
         returnType,
-        new IR::Member(new IR::Type_Method(new IR::ParameterList(), methodName),
+        new IR::Member(new IR::Type_Method(paramList, methodName),
                        new IR::PathExpression(new IR::Type_Extern("*"), new IR::Path("*")),
                        methodName),
         args);

backends/p4tools/common/lib/util.h

@@ -89,7 +89,8 @@ class Utils {
     /// is typically Type_Void.
     static const IR::MethodCallExpression *generateInternalMethodCall(
         cstring methodName, const std::vector<const IR::Expression *> &argVector,
-        const IR::Type *returnType = IR::Type_Void::get());
+        const IR::Type *returnType = IR::Type_Void::get(),
+        const IR::ParameterList *paramList = new IR::ParameterList());
 
     /// Shuffles the given iterable @param inp
     template <typename T>

backends/p4tools/modules/testgen/core/small_step/expr_stepper.cpp

@@ -121,10 +121,54 @@ void ExprStepper::evalActionCall(const IR::P4Action *action, const IR::MethodCal
     result->emplace_back(state);
 }
 
+bool ExprStepper::resolveMethodCallArguments(const IR::MethodCallExpression *call) {
+    IR::Vector<IR::Argument> resolvedArgs;
+    const auto *method = call->method->type->checkedTo<IR::Type_MethodBase>();
+    const auto &methodParams = method->parameters->parameters;
+    const auto *callArguments = call->arguments;
+    for (size_t idx = 0; idx < callArguments->size(); ++idx) {
+        const auto *arg = callArguments->at(idx);
+        const auto *param = methodParams.at(idx);
+        const auto *argExpr = arg->expression;
+        if (param->direction == IR::Direction::Out || SymbolicEnv::isSymbolicValue(argExpr)) {
+            continue;
+        }
+        // If the parameter is not an out parameter (meaning we do not care about its content) and
+        // the argument is not yet symbolic, try to resolve it.
+        return stepToSubexpr(
+            argExpr, result, state, [call, idx, param](const Continuation::Parameter *v) {
+                // TODO: It seems expensive to copy the function every time we resolve an argument.
+                // We should do this all at once. But how?
+                // This is the same problem as in stepToListSubexpr
+                // Thankfully, most method calls have less than 10 arguments.
+                auto *clonedCall = call->clone();
+                auto *arguments = clonedCall->arguments->clone();
+                auto *arg = arguments->at(idx)->clone();
+                const IR::Expression *computedExpr = v->param;
+                // A parameter with direction InOut might be read and also written to.
+                // We capture this ambiguity with an InOutReference.
+                if (param->direction == IR::Direction::InOut) {
+                    auto stateVar = ToolsVariables::convertReference(arg->expression);
+                    computedExpr = new IR::InOutReference(stateVar, computedExpr);
+                }
+                arg->expression = computedExpr;
+                (*arguments)[idx] = arg;
+                clonedCall->arguments = arguments;
+                return Continuation::Return(clonedCall);
+            });
+    }
+    return true;
+}
+
 bool ExprStepper::preorder(const IR::MethodCallExpression *call) {
     logStep(call);
     // A method call expression represents an invocation of an action, a table, an extern, or
     // setValid/setInvalid.
+
+    if (!resolveMethodCallArguments(call)) {
+        return false;
+    }
+
     // Handle method calls. These are either table invocations or extern calls.
     if (call->method->type->is<IR::Type_Method>()) {
         if (const auto *path = call->method->to<IR::PathExpression>()) {
@@ -255,6 +299,13 @@ bool ExprStepper::preorder(const IR::Mux *mux) {
 
 bool ExprStepper::preorder(const IR::PathExpression *pathExpression) {
     logStep(pathExpression);
+    // If the path expression is a Type_MatchKind, convert it to a StringLiteral.
+    if (pathExpression->type->is<IR::Type_MatchKind>()) {
+        state.replaceTopBody(Continuation::Return(
+            new IR::StringLiteral(IR::Type_MatchKind::get(), pathExpression->path->name)));
+        result->emplace_back(state);
+        return false;
+    }
     // Otherwise convert the path expression into a qualified member and return it.
     state.replaceTopBody(Continuation::Return(state.get(pathExpression)));
     result->emplace_back(state);

backends/p4tools/modules/testgen/core/small_step/expr_stepper.h

@@ -67,6 +67,11 @@ class ExprStepper : public AbstractStepper {
     /// values for hit, miss and action_run after that.
     void handleHitMissActionRun(const IR::Member *member);
 
+    /// Resolve all arguments to the method call by stepping into each argument that is not yet
+    /// symbolic or a pure reference (represented as Out direction).
+    /// @returns false when an argument needs to be resolved, true otherwise.
+    bool resolveMethodCallArguments(const IR::MethodCallExpression *call);
+
     /// Evaluates a call to an extern method. Upon return, the given result will be augmented with
     /// the successor states resulting from evaluating the call.
     ///

backends/p4tools/modules/testgen/core/small_step/extern_stepper.cpp

@@ -3,7 +3,6 @@
 #include <functional>
 #include <optional>
 #include <ostream>
-#include <string>
 #include <utility>
 #include <vector>
 
@@ -15,13 +14,11 @@
 #include "backends/p4tools/common/lib/trace_event_types.h"
 #include "backends/p4tools/common/lib/variables.h"
 #include "ir/id.h"
-#include "ir/indexed_vector.h"
 #include "ir/ir.h"
 #include "ir/irutils.h"
 #include "ir/vector.h"
 #include "lib/cstring.h"
 #include "lib/exceptions.h"
-#include "lib/log.h"
 
 #include "backends/p4tools/modules/testgen//lib/exceptions.h"
 #include "backends/p4tools/modules/testgen/core/externs.h"
@@ -293,8 +290,8 @@ void ExprStepper::evalInternalExternMethodCall(const IR::MethodCallExpression *c
          [this](const IR::MethodCallExpression * /*call*/, const IR::Expression * /*receiver*/,
                 IR::ID & /*methodName*/, const IR::Vector<IR::Argument> *args,
                 const ExecutionState &state, SmallStepEvaluator::Result &result) {
-             const auto *blockRef = args->at(0)->expression->checkedTo<IR::PathExpression>();
-             const auto *block = state.findDecl(blockRef);
+             const auto *blockRef = args->at(0)->expression->checkedTo<IR::StringLiteral>();
+             const auto *block = state.findDecl(new IR::Path(blockRef->value));
              const auto *archSpec = TestgenTarget::getArchSpec();
              auto blockName = block->getName().name;
              auto &nextState = state.clone();
@@ -328,8 +325,8 @@ void ExprStepper::evalInternalExternMethodCall(const IR::MethodCallExpression *c
          [this](const IR::MethodCallExpression * /*call*/, const IR::Expression * /*receiver*/,
                 IR::ID & /*methodName*/, const IR::Vector<IR::Argument> *args,
                 const ExecutionState &state, SmallStepEvaluator::Result &result) {
-             const auto *blockRef = args->at(0)->expression->checkedTo<IR::PathExpression>();
-             const auto *block = state.findDecl(blockRef);
+             const auto *blockRef = args->at(0)->expression->checkedTo<IR::StringLiteral>();
+             const auto *block = state.findDecl(new IR::Path(blockRef->value));
              const auto *archSpec = TestgenTarget::getArchSpec();
              auto blockName = block->getName().name;
              auto &nextState = state.clone();
@@ -722,13 +719,8 @@ void ExprStepper::evalExternMethodCall(const IR::MethodCallExpression *call,
          [](const IR::MethodCallExpression * /*call*/, const IR::Expression * /*receiver*/,
             IR::ID & /*methodName*/, const IR::Vector<IR::Argument> *args,
             const ExecutionState &state, SmallStepEvaluator::Result &result) {
-             const auto *emitOutput = args->at(0)->expression;
-             const auto *emitType = emitOutput->type->checkedTo<IR::Type_StructLike>();
-             if (!(emitOutput->is<IR::Member>() || emitOutput->is<IR::ArrayIndex>())) {
-                 TESTGEN_UNIMPLEMENTED("Emit input %1% of type %2% not supported", emitOutput,
-                                       emitType);
-             }
-             const auto *validVar = state.get(ToolsVariables::getHeaderValidity(emitOutput));
+             const auto *emitHeader = args->at(0)->expression->checkedTo<IR::HeaderExpression>();
+             const auto *validVar = emitHeader->validity;
 
              // Check whether the validity bit of the header is tainted. If it is, the entire
              // emit is tainted. There is not much we can do here, so throw an error.
@@ -738,25 +730,21 @@ void ExprStepper::evalExternMethodCall(const IR::MethodCallExpression *call,
                      "The validity bit of %1% is tainted. Tainted emit calls can not be "
                      "mitigated "
                      "because it is unclear whether the header will be emitted. Abort.",
-                     emitOutput);
+                     emitHeader);
              }
              // This call assumes that the "expandEmit" midend pass is being used. expandEmit
              // unravels emit calls on structs into emit calls on the header members.
              {
                  auto &nextState = state.clone();
-                 std::vector<std::pair<IR::StateVariable, const IR::Expression *>> fields;
-                 for (const auto *field : emitType->fields) {
-                     const auto *fieldType = field->type;
-                     if (fieldType->is<IR::Type_StructLike>()) {
-                         BUG("Unexpected emit field %1% of type %2%", field, fieldType);
-                     }
-                     const auto *fieldRef = new IR::Member(fieldType, emitOutput, field->name);
-                     const IR::Expression *fieldExpr = nextState.get(fieldRef);
-                     fieldType = fieldExpr->type;
+                 // Append to the emit buffer.
+                 auto flatFields = IR::flattenStructExpression(emitHeader);
+                 for (const auto *fieldExpr : flatFields) {
+                     const auto *fieldType = fieldExpr->type;
+                     BUG_CHECK(!fieldType->is<IR::Type_StructLike>(),
+                               "Unexpected emit field %1% of type %2%", fieldExpr, fieldType);
                      if (const auto *varbits = fieldType->to<IR::Extracted_Varbits>()) {
                          fieldType = IR::getBitType(varbits->assignedSize);
                      }
-                     fields.emplace_back(fieldRef, fieldExpr);
                      auto fieldWidth = fieldType->width_bits();
                      // If the width is zero, do not bother with emitting.
                      if (fieldWidth == 0) {
@@ -780,7 +768,7 @@ void ExprStepper::evalExternMethodCall(const IR::MethodCallExpression *call,
                      // Append to the emit buffer.
                      nextState.appendToEmitBuffer(fieldExpr);
                  }
-                 nextState.add(*new TraceEvents::Emit(emitOutput, fields));
+                 nextState.add(*new TraceEvents::Emit(emitHeader));
                  nextState.popBody();
                  // Only when the header is valid, the members are emitted and the packet
                  // delta is adjusted.
@@ -789,7 +777,7 @@ void ExprStepper::evalExternMethodCall(const IR::MethodCallExpression *call,
              {
                  auto &invalidState = state.clone();
                  std::stringstream traceString;
-                 traceString << "Invalid emit: " << emitOutput->toString();
+                 traceString << "Invalid emit: " << emitHeader->toString();
                  invalidState.add(*new TraceEvents::Generic(traceString));
                  invalidState.popBody();
                  result->emplace_back(new IR::LNot(IR::Type::Boolean::get(), validVar), state,