Skip to content

C#: Correct printing of returns via out/ref parameters in model generation. #19114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
michaelnebel merged 3 commits into from
Mar 27, 2025
Merged

C#: Correct printing of returns via out/ref parameters in model generation. #19114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcefdc8
C#: Add model generator test cases with in/out parameters.
michaelnebel Mar 25, 2025
8763d18
C#: Correct printing of out and ref notes in the model generator.
michaelnebel Mar 25, 2025
8bda7ce
C#: Update model generator expected test output.
michaelnebel Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
result = ParamReturnNodeAsOutput<parameterContentAccess/1>::paramReturnNodeAsOutput(c, pos)
}

ParameterPosition getReturnKindParamPosition(ReturnKind kind) {
kind.(OutRefReturnKind).getPosition() = result.getPosition()
}

Callable returnNodeEnclosingCallable(DataFlow::Node ret) {
result = DataFlowImplCommon::getNodeEnclosingCallable(ret).asCallable(_)
}
Expand Down
37 changes: 37 additions & 0 deletions csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,3 +1034,40 @@ public override object GetValue()
}
}
}

public class ParameterModifiers
{
// contentbased-summary=Models;ParameterModifiers;false;Copy;(System.Object,System.Object);;Argument[0];Argument[1];value;dfc-generated
// summary=Models;ParameterModifiers;false;Copy;(System.Object,System.Object);;Argument[0];Argument[1];taint;df-generated
public void Copy(object key, out object value)
{
value = key;
}

// contentbased-summary=Models;ParameterModifiers;false;CopyToRef;(System.Object,System.Object);;Argument[0];Argument[1];value;dfc-generated
// summary=Models;ParameterModifiers;false;CopyToRef;(System.Object,System.Object);;Argument[0];Argument[1];taint;df-generated
public void CopyToRef(object key, ref object value)
{
value = key;
}

// No summaries as we disregard flow from a parameter to itself.
// neutral=Models;ParameterModifiers;RefParamFlowToSelf;(System.Object,System.Boolean);summary;df-generated
public void RefParamFlowToSelf(ref object value, bool b)
{
value = b ? value : null;
}

// neutral=Models;ParameterModifiers;RefParamUse;(System.Object);summary;df-generated
public void RefParamUse(ref object value)
{
var b = value is null;
Copy link

Copilot AI Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable 'b' is assigned but never used; consider removing it or using it meaningfully to avoid confusion.

Suggested change
var b = value is null;
// The variable 'b' was removed as it was unused.

Copilot uses AI. Check for mistakes.
}

// contentbased-summary=Models;ParameterModifiers;false;InReturn;(System.Object);;Argument[0];ReturnValue;value;dfc-generated
// summary=Models;ParameterModifiers;false;InReturn;(System.Object);;Argument[0];ReturnValue;taint;df-generated
public object InReturn(in object v)
{
return v;
}
}
16 changes: 15 additions & 1 deletion shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ signature module ModelGeneratorInputSig<LocationSig Location, InputSig<Location>
*/
string printContent(Lang::ContentSet c);

/**
* Gets the parameter position of the return kind, if any.
*/
default Lang::ParameterPosition getReturnKindParamPosition(Lang::ReturnKind node) { none() }

/**
* Holds if it is irrelevant to generate models for `api` based on data flow analysis.
*
Expand Down Expand Up @@ -301,6 +306,14 @@ module MakeModelGenerator<
* Gets the kind of the return node.
*/
DataFlow::ReturnKindExt getKind() { result = kind }

/**
* Gets the parameter position of the return node, if any.
*/
DataFlow::ParameterPosition getPosition() {
result = this.getKind().(DataFlow::ParamUpdateReturnKind).getPosition() or
result = getReturnKindParamPosition(this.getKind().(DataFlow::ValueReturnKind).getKind())
}
}

bindingset[c]
Expand All @@ -309,10 +322,11 @@ module MakeModelGenerator<
private module PrintReturnNodeExt<printCallableParamSig/2 printCallableParam> {
string getOutput(ReturnNodeExt node) {
node.getKind() instanceof DataFlow::ValueReturnKind and
not exists(node.getPosition()) and
result = "ReturnValue"
or
exists(DataFlow::ParameterPosition pos |
pos = node.getKind().(DataFlow::ParamUpdateReturnKind).getPosition() and
pos = node.getPosition() and
result = printCallableParam(returnNodeEnclosingCallable(node), pos)
)
}
Expand Down
Loading