[question?] counterintuitive class method behaviour

[k4lizen]

When I write a query like this:

class Meow extends Struct {
  Field f;
  Meow() {
    f.getDeclaringType() = this
  }
  Field getField() { result = f }
}

from Meow m
select m, m.getField(), m.getField()

My mental model of this is: "Meow is essentially a (struct, struct's field) tuple. from Meow m will select all of those tuples. For each of those , select m, m.getField(), m.getField() will print their contents."

But that is actually not what happens, because you get results like this:

|                 m                 |                        col1                         |                        col2                         |
+-----------------------------------+-----------------------------------------------------+-----------------------------------------------------+
// [snip]
| port_io_ops                       | f_inb                                               | f_outb                                              |
// [snip]

Which means that the first m.getField() and the second m.getField() are operating on different Meow objects.

I've been debugging a bug in my query for hours now only to realize that this is what happens. Is this really intended? I read through a non-trivial amount of documentation and did not realize this, I feel like my whole mental model of how codeql works is shattering :S

(If it is really intended, could the documentation be updated to point this out more clearly?)

[k4lizen]

I encountered this in the following situation, this doesn't work:

class MyStuct extends Struct {
  MyFlow::PathNode source;
  MyFlow::PathNode sink;
  MyStuct() {
      MyFlow::flowPath(source, sink) and
      some_predicate(this, sink) and
  }
  MyFlow::PathNode getSink() { result = sink }
  MyFlow::PathNode getSource() { result = source }
}
from MyStuct s
select s.getSink().getNode(), s.getSource(), s.getSink(), "aaa"

, and instead you must do this:

class MyStuct extends Struct {
  MyFlow::PathNode source;
  MyFlow::PathNode sink;
  MyStuct() {
      MyFlow::flowPath(source, sink) and
      some_predicate(this, sink) and
  }
  MyFlow::PathNode getSink() { result = sink }
  MyFlow::PathNode getSource() { result = source }
}
from MyStuct s, MyFlow::PathNode source, MyFlow::PathNode sink
where
  source = s.getSource() and sink = s.getSink() 
select sink.getNode(), source, sink, "aaa"

But it's not even clear to my why in the first case the s.getSource() and s.getSink() are being taken from different objects in the select statement, but in the second case, in the where statement, the source and sink are indeed being taken from the same object.

What is the rule here?

[aschackmull]

My mental model of this is: "Meow is essentially a (struct, struct's field) tuple

This is the part that's wrong. A Meow is only a struct. And the charpred defines how the class relates to the class field - in this case it's a one-to-many relation. I'd generally recommend that you avoid class fields that are not functionally determined by the class.

Now, in order to do what you probably want, i.e. define a tuple type, then you need to construct a newtype:

newtype TMeow = TMeowPair(Struct s, Field f) { f.getDeclaringType() = s }

class Meow extends TMeow {
  Struct s;
  Field f;
  Meow() { this = TMeowPair(s, f) }
  ...
}

[k4lizen]

I see, thank you for the tip.

Could you also clarify why using

where
  source = s.getSource() and sink = s.getSink() 

even helps in this situation? Since it's a one-to-many relation I would expect this to then easily yank out a sink and source which are unrelated.

Also, what is a charpred?

[aschackmull]

If you don't add the where clause with sink = s.getSink() then you have two independent uses of getSink, which are perfectly able to retrieve different sinks.

A "charpred" is short for "characteristic predicate". It's the predicate that syntactically looks like a constructor.

[k4lizen]

If you don't add the where clause with sink = s.getSink() then you have two independent uses of getSink, which are perfectly able to retrieve different sinks.

But why is it not the case that a source which is unrelated to a sink gets taken, considering the invocations of s.getSource() and s.getSink() are unrelated?

(so a situation where you get a source which is a valid source in some path, and a sink which is a valid sink in some path, but that particular sink does not have a path to that particular source. this does not happen, but considering your explanation, i would expect it to happen.)

[aschackmull]

But why is it not the case that a source which is unrelated to a sink gets taken, considering the invocations of s.getSource() and s.getSink() are unrelated?

(so a situation where you get a source which is a valid source in some path, and a sink which is a valid sink in some path, but that particular sink does not have a path to that particular source. this does not happen, but considering your explanation, i would expect it to happen.)

I would expect that as well. So if it really doesn't, then I can't explain it without some thorough investigation.

[k4lizen]

I see.

Could a potential explanation be that it actually does happen, but that codeql database interpret-results filters those results out? Maybe this would explain why I'm seeing so many more results in the #select in #21673 .

[aschackmull]

Ah yes! Of course, that's exactly it. Interpret-results will calculate paths for each #select'ed tuple, which will effectively discard the mismatched source-sink pairs, because they don't have paths.

[k4lizen]

Alright, I managed to get it working as expected with your newtype suggestion.

I'm not closing this issue in case you want to update the docs to clarify this behaviour / add a note about your suggestion, but otherwise I have no more questions regarding this :p

Thanks.