Page MenuHomePhabricator

Debug info: Fixed faulty debug locations for attributed statements

Authored by Ka-Ka on Sep 3 2017, 10:35 AM.



As the attributed statements are considered simple statements no
stoppoint was generated before emitting attributed do/while/for/range-
statement. This lead to faulty debug locations.

Diff Detail


Event Timeline

Ka-Ka created this revision.Sep 3 2017, 10:35 AM
Ka-Ka added inline comments.Sep 3 2017, 10:47 AM
18 ↗(On Diff #113708)

The part "scope: !14)" should be removed from the testcase.

bjope added a subscriber: bjope.Sep 4 2017, 9:46 AM
Ka-Ka updated this revision to Diff 113803.Sep 5 2017, 12:22 AM
Ka-Ka edited the summary of this revision. (Show Details)
Ka-Ka added a reviewer: dblaikie.

I updated the testcase according to what David Blaikie suggested.
I also rewrote to code to be a bit more robust and avoid emitting unnecessary stoppoints.

dblaikie edited edge metadata.Sep 5 2017, 8:44 AM

Not sure what you mean by "avoid emitting unnecessary stop points" - do you have a test case for that?

Ka-Ka added a comment.Sep 5 2017, 9:10 AM

Not sure what you mean by "avoid emitting unnecessary stop points" - do you have a test case for that?

In my previous patch you could end up doing two calls to EmitStopPoint() for the same statement. In the previous patch I added a EmitStopPoint() in the method EmitAttributedStmt(), but if you took the default clause in the switch statement in the same method you could end up executing another EmitStopPoint() in EmitStmt().

The new patch do not have the issue with executing EmitStopPoint() twice for the same statement. I think generally that the new patch is more robust solution.

dblaikie accepted this revision.Sep 5 2017, 9:40 AM

OK, sounds good - thanks :)

This revision is now accepted and ready to land.Sep 5 2017, 9:40 AM
echristo added inline comments.Sep 5 2017, 2:51 PM
1 ↗(On Diff #113803)

Since we're not optimizing or generating code you should be able to remove the -disable-llvm-passes I believe.

Ka-Ka added inline comments.Sep 5 2017, 10:58 PM
1 ↗(On Diff #113803)

I added the option -disable-llvm-passes as David Blaikie suggested it in his original comment:
As I have mainly worked in llvm backends I'm not that familiar with the clang -cc1 options. I have no opinion about this. I wait for you and David Blaikie to decide if I should remove or keep the option -disable-llvm-passes.

This revision was automatically updated to reflect the committed changes.