Skip to content

Ruby: Context sensitive instance method resolution #10358

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.

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
merged 3 commits into from
Sep 26, 2022

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Sep 8, 2022

This PR implements call-sensitive instance method resolution, using the data-flow library's built-in support.

For example, in

class A
  def method1 x
    sink x
  end

  def method2 x
    method1 x
  end
end

class B < A
  def method1 x
    non_sink x
  end
end

B.new.method2(taint)

we will no longer report flow from taint to sink (but still to non_sink).

@github-actions github-actions bot added the Ruby label Sep 8, 2022
@hvitved hvitved force-pushed the ruby/dataflow/call-ctx branch 2 times, most recently from 6044f14 to 4607adc Compare September 14, 2022 12:45
@hvitved hvitved force-pushed the ruby/dataflow/call-ctx branch from 4607adc to 7bae643 Compare September 16, 2022 08:42
@hvitved hvitved force-pushed the ruby/dataflow/call-ctx branch 3 times, most recently from 8695ba2 to 0e0923e Compare September 20, 2022 18:01
@hvitved hvitved changed the title Ruby/dataflow/call ctx Ruby: Call sensitive instance method resolution Sep 20, 2022
@hvitved hvitved force-pushed the ruby/dataflow/call-ctx branch from 0e0923e to dc0720b Compare September 22, 2022 13:18
@hvitved hvitved force-pushed the ruby/dataflow/call-ctx branch from dc0720b to 9937ae8 Compare September 22, 2022 14:25
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Sep 23, 2022
@hvitved hvitved marked this pull request as ready for review September 23, 2022 07:16
@hvitved hvitved requested a review from a team as a code owner September 23, 2022 07:16
@hvitved hvitved changed the title Ruby: Call sensitive instance method resolution Ruby: Context sensitive instance method resolution Sep 23, 2022
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@@ -14,6 +14,9 @@ class Module extends TModule {
/** Gets the super class of this module, if any. */
Module getSuperClass() { result = getSuperClass(this) }

/** Gets a sub class of this module, if any. */
Module getASubClass() { this = getSuperClass(result) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This predicate returns direct sub classes only, doesn't it? Let document that and perhaps also make it more clear from its name.

)
or
// `in C => c then c.foo`
asModulePattern(n, tp) and
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps also add C === x tests. If I'm not mistaken, both types of case expressions are implemented using that operator.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see you just moved these predicate. Perhaps we should post-pone === for a future PR. We might want to desugar case statements into if at some point too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can do that in another PR. I guess is could be added to hasAdjacentTypeCheckedReads.

@hvitved hvitved merged commit 88baf08 into github:main Sep 26, 2022
@hvitved hvitved deleted the ruby/dataflow/call-ctx branch September 26, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants