Control flow graph has possibly incorrect statements with missing source location.

[llvmbot]

Bugzilla Link	10805
Resolution	INVALID
Resolved on	Aug 31, 2011 19:01
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@tkremenek

Extended Description

The control flow graph for the following code does not seem to be correct. In particular, the first statement has no source location. The problem may be more serious than that, as the basic block below seems to be missing some statements, such as the code that loads f from this.

struct Foomger {
void operator++();
};

struct Foomgoper {
Foomger f;

bool done();
void invalid_back_edge() {
do {
// FIXME: Possible Clang bug:
// The first statement in this basic block has no source location
++f;
} while (!done());
}
};

[llvmbot]

assigned to @tkremenek

[tkremenek]

I think you are possibly misinterpreting the format of CFGs. CFGs are not always linearized (although they can be built that way). By default, the CFG does not contain a complete linearization of all subexpressions, as the control-dependency between subexpressions and parent expressions is implied.

As for the "first" statement, are you talking about the "do"? That appears as a terminator in the basic block.

Also, SourceLocations are a property of the AST, not of the CFG.

FWIW, the completely linearized CFG (that the static analyzer sees) is:

[ B4 (ENTRY) ]
Predecessors (0):
Successors (1): B2

[ B1 ]
1: this
2: [B1.1]->done
3: B1.2
4: ![B1.3]
T: do ... while [B1.4]
Predecessors (1): B2
Successors (2): B3 B0

[ B2 ]
1: this
2: [B2.1]->f
3: operator++
4: [B2.3]
5: ++ [B2.2] (OperatorCall)
Predecessors (2): B3 B4
Successors (1): B1

[ B3 ]
Predecessors (1): B1
Successors (1): B2

[ B0 (EXIT) ]
Predecessors (1): B1
Successors (0):

This is correct. Notice that the 'do... while' appears as a terminator.

The default, non-linearized CFG looks as follows:

[ B4 (ENTRY) ]
Predecessors (0):
Successors (1): B2

[ B1 ]
1: this->done()
2: ![B1.1]
T: do ... while [B1.2]
Predecessors (1): B2
Successors (2): B3 B0

[ B2 ]
1: ++ this->f (OperatorCall)
Predecessors (2): B3 B4
Successors (1): B1

[ B3 ]
Predecessors (1): B1
Successors (1): B2

[ B0 (EXIT) ]
Predecessors (1): B1
Successors (0):

This is also correct. The reason is that the load to 'f' is a subexpression, and it implicitly has a control-dependency between that and the call to operator++().

For consistency, we can require that the entire CFG be linearized all the time, but we would need to measure the performance implication of that.

[llvmbot]

As for the "first" statement, are you talking about the "do"?

No, I am talking about the "this" in "this->f".

FWIW, the completely linearized CFG (that the static analyzer sees) is:

That would be reasonable, but that is not the CFG that I am seeing. The CFG that I am seeing, as shown by CFG::dump(), is as follows:

[ B4 (ENTRY) ]
Predecessors (0):
Successors (1): B2

[ B1 ]
1: this->done()
2: ![B1.1]
T: do ... while [B1.2]
Predecessors (1): B2
Successors (2): B3 B0

[ B2 ]
1: operator++
2: [B2.1]
3: ++ this->f (OperatorCall)
Predecessors (2): B3 B4
Successors (1): B1

[ B3 ]
Predecessors (1): B1
Successors (1): B2

[ B0 (EXIT) ]
Predecessors (1): B1
Successors (0):

Notice that Block B2 is missing the first two lines from your example:

1: this
2: [B2.1]->f

If these two lines were present, then the first statement would have a source location. However, although the lack of source location is how I discovered the issue, it is perhaps misleading. My apologies for not including the actual CFG dump in the initial bug report.

-DeLesley

[tkremenek]

As for the "first" statement, are you talking about the "do"?

No, I am talking about the "this" in "this->f".

[ B2 ]
1: operator++
2: [B2.1]
3: ++ this->f (OperatorCall)
Predecessors (2): B3 B4
Successors (1): B1

Notice that Block B2 is missing the first two lines from your example:

1: this
2: [B2.1]->f

If these two lines were present, then the first statement would have a source
location. However, although the lack of source location is how I discovered
the issue, it is perhaps misleading. My apologies for not including the actual
CFG dump in the initial bug report.

The CFG is well-formed, since the "this->f" is a subexpression of the operator call. It's not included as a "block-level expression" because the CFG isn't fully linearized.

As for the missing location for 'this', I hazard to guess it isn't there because 'this' doesn't appear in your code example. Hence it doesn't have a valid SourceLocation.

Not all AST nodes are required to have valid SourceLocations. Sometimes they are invalid in case where we implicitly add an AST element for semantic convenience in the AST. In this case, since 'this' was not explicitly written, it should not have a SourceLocation.