Check for out/inout bindings aliased with uses

- pass that finds captured expressions in callable (symbols defined
  outside the callable)
- modify some failing tests to not have the error
- add to p4smith's 'KNOWN_BUGS' for tests, as it generates code that
  fails this check

Signed-off-by: Chris Dodd <[email protected]>

Author: ChrisDodd

backends/p4tools/modules/smith/scripts/compilation-test.sh

@@ -12,6 +12,7 @@ KNOWN_BUGS=(
     "Cannot evaluate initializer for constant"
     "Null expr"
     "error: retval_1: declaration not found"  # V1model failure.
+    "error: \(in\)\?out argument binds value accessed by callee"
 )
 
 # Function to check if an error is triggered by a known bug.

frontends/CMakeLists.txt

@@ -15,6 +15,7 @@
 set (P4_FRONTEND_SRCS
   p4/actionsInlining.cpp
   p4/callGraph.cpp
+  p4/checkArgAlias.cpp
   p4/checkConstants.cpp
   p4/checkCoreMethods.cpp
   p4/checkNamedArgs.cpp
@@ -103,6 +104,7 @@ set (P4_FRONTEND_HDRS
   p4/actionsInlining.h
   p4/alias.h
   p4/callGraph.h
+  p4/checkArgAlias.h
   p4/checkConstants.h
   p4/checkCoreMethods.h
   p4/checkNamedArgs.h

frontends/p4/checkArgAlias.cpp

@@ -0,0 +1,135 @@
+/*
+Copyright 2025 Nvidia, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+#include "checkArgAlias.h"
+
+#include "methodInstance.h"
+
+namespace P4 {
+
+// find the enclosing 'location' expression from the current context.  That
+// is, any Member(s) or ArrayIndex(s) that are a parents of the current node.
+const IR::Expression *CheckArgAlias::FindCaptures::getRefCtxt() {
+    auto *ctxt = getChildContext();
+    for (; auto *p = ctxt->parent; ctxt = p) {
+        if (p->node->is<IR::Member>()) continue;
+        if (p->node->is<IR::ArrayIndex>() && p->child_index == 0) continue;
+        break;
+    }
+    return ctxt->node->to<IR::Expression>();
+}
+
+bool CheckArgAlias::FindCaptures::preorder(const IR::PathExpression *pe) {
+    const Context *ctxt = nullptr;
+    auto last = visiting.end();
+    --last;
+    while (auto scope = findOrigCtxt<IR::INamespace>(ctxt)) {
+        auto rv = lookup(scope, pe->path->name, ResolutionType::Any);
+        if (!rv.empty()) break;
+        if (last->second == scope->getNode()) --last;
+    }
+    auto expr = getRefCtxt();
+    for (++last; last < visiting.end(); ++last) {
+        LOG4(expr << " escapes from " << DBPrint::Brief << last->first);
+        result[last->first][pe->path->name].push_back(expr);
+    }
+    return true;
+}
+
+bool CheckArgAlias::FindCaptures::preorder(const IR::IApply *ia) {
+    LOG3("CheckArgAlias::preorder(IA): " << DBPrint::Brief << ia);
+    auto *d = ia->to<IR::IDeclaration>();
+    visiting.push_back(std::make_pair(d, d->getNode()));
+    return true;
+}
+
+bool CheckArgAlias::FindCaptures::preorder(const IR::IFunctional *fn) {
+    LOG3("CheckArgAlias::preorder(IF): " << DBPrint::Brief << fn);
+    auto *d = fn->to<IR::IDeclaration>();
+    visiting.push_back(std::make_pair(d, d->getNode()));
+    return true;
+}
+
+const IR::Expression *CheckArgAlias::Check::baseExpr(const IR::Expression *e, int *depth) {
+    if (depth) ++*depth;
+    if (auto *mem = e->to<IR::Member>()) return baseExpr(mem->expr, depth);
+    if (auto *ai = e->to<IR::ArrayIndex>()) return baseExpr(ai->left, depth);
+    return e;
+}
+
+int CheckArgAlias::Check::baseExprDepth(const IR::Expression *e) {
+    int depth = 0;
+    baseExpr(e, &depth);
+    return depth;
+}
+
+bool CheckArgAlias::Check::overlaps(const IR::Expression *e1, const IR::Expression *e2) {
+    while (auto *ai = e1->to<IR::ArrayIndex>()) e1 = ai->left;
+    while (auto *ai = e2->to<IR::ArrayIndex>()) e2 = ai->left;
+    auto *m1 = e1->to<IR::Member>();
+    auto *m2 = e2->to<IR::Member>();
+    if (m1 && m2) {
+        if (m1->expr->type == m2->expr->type) return m1->member == m2->member;
+        if (baseExprDepth(m1->expr) > baseExprDepth(m2->expr))
+            return overlaps(m1->expr, e2);
+        else
+            return overlaps(e1, m2->expr);
+    } else if (m1) {
+        return overlaps(m1->expr, e2);
+    } else if (m2) {
+        return overlaps(e1, m2->expr);
+    }
+    BUG_CHECK(e1->is<IR::PathExpression>(), "'%1%' not a PathExpression", e1);
+    BUG_CHECK(e2->is<IR::PathExpression>(), "'%1%' not a PathExpression", e2);
+    BUG_CHECK(e1->equiv(*e2), "'%1%' and '%2%' are different", e1, e2);
+    return e1->equiv(*e2);
+}
+
+bool CheckArgAlias::Check::preorder(const IR::MethodCallExpression *mce) {
+    LOG3("CheckArgAlias::Check::preorder(MCE): " << mce);
+    auto *mi = MethodInstance::resolve(mce, this, self.typeMap);
+    BUG_CHECK(mi, "MethodInstance::resolve failed");
+    if (!self.captures.any(mi->callee())) return true;
+    auto arg = mi->expr->arguments->begin();
+    auto *params = mi->actualMethodType->parameters;
+    for (auto it = params->begin(); it < params->end(); ++it, ++arg) {
+        if (arg == mi->expr->arguments->end()) {
+            // at the end of the argument list -- rest of params must be directionless
+            // and this must be an action call
+            BUG_CHECK(mi->is<P4::ActionCall>(), "not an action call?");
+            break;
+        }
+        auto *param = *it;
+        if (param->direction == IR::Direction::None) continue;
+        if (param->direction == IR::Direction::In) continue;
+        auto *exp = (*arg)->expression;
+        while (auto *sl = exp->to<IR::AbstractSlice>()) exp = sl->e0;
+        auto *pe = baseExpr(exp)->to<IR::PathExpression>();
+        BUG_CHECK(pe, "not a PathExpression");
+        for (auto *cap : self.captures(mi->callee(), pe->path->name)) {
+            if (overlaps(exp, cap)) {
+                error(ErrorType::ERR_INVALID, "%1%%2% binds value accessed by callee %3%%4%",
+                      param->direction == IR::Direction::InOut ? "inout argument" : "out argument",
+                      (*arg)->srcInfo, mi->callee(), cap->srcInfo);
+                // only trigger once per call
+                return true;
+            }
+        }
+    }
+    return true;
+}
+
+}  // namespace P4

frontends/p4/checkArgAlias.h

@@ -0,0 +1,96 @@
+/*
+Copyright 2025 Nvidia, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+#ifndef FRONTENDS_P4_CHECKARGALIAS_H_
+#define FRONTENDS_P4_CHECKARGALIAS_H_
+
+#include "frontends/common/resolveReferences/resolveReferences.h"
+#include "ir/ir.h"
+#include "ir/pass_manager.h"
+#include "typeChecking/typeChecker.h"
+#include "typeMap.h"
+
+namespace P4 {
+
+class CheckArgAlias : public PassManager {
+    TypeMap *typeMap;
+    class FindCaptures : public Inspector, public ResolutionContext {
+        std::vector<std::pair<const IR::IDeclaration *, const IR::Node *>> visiting;
+        profile_t init_apply(const IR::Node *root) {
+            auto rv = Inspector::init_apply(root);
+            // initialize visiting stack with (just) sentinel value;
+            visiting.resize(1);
+            visiting.back().first = nullptr;
+            visiting.back().second = nullptr;
+            return rv;
+        }
+
+        const IR::Expression *getRefCtxt();
+
+        bool preorder(const IR::PathExpression *);
+        bool preorder(const IR::Type_Name *) { return false; }
+
+        bool preorder(const IR::IApply *);
+        bool preorder(const IR::IFunctional *);
+
+        // FIXME -- would be nice if dynamic dispatch could handle this automatically
+        bool preorder(const IR::Node *n) {
+            if (auto *ia = n->to<IR::IApply>()) return preorder(ia);
+            if (auto *fn = n->to<IR::IFunctional>()) return preorder(fn);
+            return true;
+        }
+        void postorder(const IR::Node *n) {
+            if (!visiting.empty() && visiting.back().second == n) visiting.pop_back();
+        }
+
+        void end_apply(const IR::Node *) { BUG_CHECK(visiting.size() == 1, "failed to finish?"); }
+        void end_apply() { visiting.clear(); }
+
+        using result_t = std::vector<const IR::Expression *>;
+        std::map<const IR::IDeclaration *, std::map<cstring, result_t>> result;
+
+     public:
+        const result_t &operator()(const IR::IDeclaration *d, cstring name) const {
+            static result_t empty;
+            auto i1 = result.find(d);
+            if (i1 == result.end()) return empty;
+            auto i2 = i1->second.find(name);
+            if (i2 == i1->second.end()) return empty;
+            return i2->second;
+        }
+        bool any(const IR::IDeclaration *d) const { return result.find(d) != result.end(); }
+    } captures;
+
+    class Check : public Inspector, public ResolutionContext {
+        const CheckArgAlias &self;
+        const IR::Expression *baseExpr(const IR::Expression *, int * = nullptr);
+        int baseExprDepth(const IR::Expression *e);
+        bool overlaps(const IR::Expression *e1, const IR::Expression *e2);
+        bool preorder(const IR::MethodCallExpression *);
+
+     public:
+        explicit Check(const CheckArgAlias &s) : self(s) {}
+    };
+
+ public:
+    explicit CheckArgAlias(TypeMap *tm) : typeMap(tm) {
+        addPasses({&captures, new ReadOnlyTypeInference(typeMap), new Check(*this)});
+    }
+};
+
+}  // namespace P4
+
+#endif /* FRONTENDS_P4_CHECKARGALIAS_H_ */

frontends/p4/frontend.cpp

@@ -26,6 +26,7 @@ limitations under the License.
 #include "lib/nullstream.h"
 // Passes
 #include "actionsInlining.h"
+#include "checkArgAlias.h"
 #include "checkConstants.h"
 #include "checkCoreMethods.h"
 #include "checkNamedArgs.h"
@@ -231,6 +232,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
         }),
         new CheckCoreMethods(&typeMap),
         new StaticAssert(&typeMap),
+        new CheckArgAlias(&typeMap),
     });
     metricsPassManager.addUnusedCode(passes, true);
     passes.addPasses({

frontends/p4/methodInstance.h

@@ -84,6 +84,7 @@ class MethodInstance : public InstanceBase {
         with instantiated type parameters. */
     const IR::Type_MethodBase *actualMethodType;
     virtual bool isApply() const { return false; }
+    virtual const IR::IDeclaration *callee() const { return object; }
 
     /** @param useExpressionType If true, the typeMap can be nullptr,
      *   and then mce->type is used.  For some technical reasons
@@ -141,6 +142,7 @@ class ApplyMethod final : public MethodInstance {
     const IR::IApply *applyObject;
     bool isApply() const override { return true; }
     bool isTableApply() const { return object->is<IR::P4Table>(); }
+    const IR::IDeclaration *callee() const override { return applyObject->to<IR::IDeclaration>(); }
 
     DECLARE_TYPEINFO(ApplyMethod, MethodInstance);
 };
@@ -222,6 +224,7 @@ class ActionCall final : public MethodInstance {
     /// Generate a version of the action where the parameters in the
     /// substitution have been replaced with the arguments.
     const IR::P4Action *specialize(const DeclarationLookup *refMap) const;
+    const IR::IDeclaration *callee() const override { return action; }
 
     DECLARE_TYPEINFO(ActionCall, MethodInstance);
 };
@@ -244,6 +247,7 @@ class FunctionCall final : public MethodInstance {
 
  public:
     const IR::Function *function;
+    const IR::IDeclaration *callee() const override { return function; }
 
     DECLARE_TYPEINFO(FunctionCall, MethodInstance);
 };

ir/dbprint-p4.cpp

@@ -138,7 +138,9 @@ void IR::ActionFunction::dbprint(std::ostream &out) const {
 }
 
 void IR::P4Action::dbprint(std::ostream &out) const {
-    out << annotations << "action " << name << "(";
+    out << annotations << "action " << name;
+    if (dbgetflags(out) & Brief) return;
+    out << "(";
     const char *sep = "";
     for (auto arg : parameters->parameters) {
         out << sep << arg->direction << ' ' << arg->type << ' ' << arg->name;
@@ -214,6 +216,7 @@ void IR::V1Control::dbprint(std::ostream &out) const {
 }
 void IR::P4Control::dbprint(std::ostream &out) const {
     out << "control " << name;
+    if (dbgetflags(out) & Brief) return;
     if (type->typeParameters && !type->typeParameters->empty()) out << type->typeParameters;
     if (type->applyParams) out << '(' << type->applyParams << ')';
     if (constructorParams) out << '(' << constructorParams << ')';