Skip to content
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

Handle pseudo return nodes in Node.of_id and improve their location #1000

Merged
merged 3 commits into from
Feb 28, 2023

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Feb 22, 2023

Closes #997.

Changes

  1. Adds Cilfacade.pseudo_return_stmt_sids, which maps pseudo return node statement IDs to the statements themselves. This allows Cilfacade.find_stmt_sid, which is used by Node.of_id to work for pseudo return nodes.
  2. Previously pseudo return nodes were given the location of the entire function definition, which starts already before the function's return type. This PR changes their location to be the end location of the function definition (with 0-width range), corresponding to the }. Since Locator uses nodes from the CFG, this already includes pseudo return nodes, but now with more reasonable locations for both lookup and display.

There is one unexpected point though as indicated by the change to the cram test: our LLoC counting can increase because the pseudo return node at its new location is on a separate line. Not sure if this weird or fine.

@sim642 sim642 added bug debugging Abstract debugging labels Feb 22, 2023
@sim642 sim642 requested a review from stilscher February 22, 2023 13:11
@michael-schwarz
Copy link
Member

Btw everyone, this is no #1000 🎉

@sim642
Copy link
Member Author

sim642 commented Feb 27, 2023

There is one unexpected point though as indicated by the change to the cram test: our LLoC counting can increase because the pseudo return node at its new location is on a separate line. Not sure if this weird or fine.

It might be possible to avoid this change by making dead code statistics ignore pseudo return nodes. Maybe that's what we should be doing because they don't correspond to anything real in the user code anyway?

@stilscher
Copy link
Member

I think that is a good idea.

@sim642 sim642 merged commit 12276b6 into master Feb 28, 2023
@sim642 sim642 deleted the issue-997 branch February 28, 2023 14:13
@sim642 sim642 added this to the v2.2.0 milestone Apr 5, 2023
sim642 added a commit to sim642/opam-repository that referenced this pull request Sep 13, 2023
CHANGES:

* Add `setjmp`/`longjmp` analysis (goblint/analyzer#887, goblint/analyzer#970, goblint/analyzer#1015, goblint/analyzer#1019).
* Refactor race analysis to lazy distribution (goblint/analyzer#1084, goblint/analyzer#1089, goblint/analyzer#1136, goblint/analyzer#1016).
* Add thread-unsafe library function call analysis (goblint/analyzer#723, goblint/analyzer#1082).
* Add mutex type analysis and mutex API analysis (goblint/analyzer#800, goblint/analyzer#839, goblint/analyzer#1073).
* Add interval set domain and string literals domain (goblint/analyzer#901, goblint/analyzer#966, goblint/analyzer#994, goblint/analyzer#1048).
* Add affine equalities analysis (goblint/analyzer#592).
* Add use-after-free analysis (goblint/analyzer#1050, goblint/analyzer#1114).
* Add dead code elimination transformation (goblint/analyzer#850, goblint/analyzer#979).
* Add taint analysis for partial contexts (goblint/analyzer#553, goblint/analyzer#952).
* Add YAML witness validation via unassume (goblint/analyzer#796, goblint/analyzer#977, goblint/analyzer#1044, goblint/analyzer#1045, goblint/analyzer#1124).
* Add incremental analysis rename detection (goblint/analyzer#774, goblint/analyzer#777).
* Fix address sets unsoundness (goblint/analyzer#822, goblint/analyzer#967, goblint/analyzer#564, goblint/analyzer#1032, goblint/analyzer#998, goblint/analyzer#1031).
* Fix thread escape analysis unsoundness (goblint/analyzer#939, goblint/analyzer#984, goblint/analyzer#1074, goblint/analyzer#1078).
* Fix many incremental analysis issues (goblint/analyzer#627, goblint/analyzer#836, goblint/analyzer#835, goblint/analyzer#841, goblint/analyzer#932, goblint/analyzer#678, goblint/analyzer#942, goblint/analyzer#949, goblint/analyzer#950, goblint/analyzer#957, goblint/analyzer#955, goblint/analyzer#954, goblint/analyzer#960, goblint/analyzer#959, goblint/analyzer#1004, goblint/analyzer#558, goblint/analyzer#1010, goblint/analyzer#1091).
* Fix server mode for abstract debugging (goblint/analyzer#983, goblint/analyzer#990, goblint/analyzer#997, goblint/analyzer#1000, goblint/analyzer#1001, goblint/analyzer#1013, goblint/analyzer#1018, goblint/analyzer#1017, goblint/analyzer#1026, goblint/analyzer#1027).
* Add documentation for configuration JSON schema and OCaml API (goblint/analyzer#999, goblint/analyzer#1054, goblint/analyzer#1055, goblint/analyzer#1053).
* Add many library function specifications (goblint/analyzer#962, goblint/analyzer#996, goblint/analyzer#1028, goblint/analyzer#1079, goblint/analyzer#1121, goblint/analyzer#1135, goblint/analyzer#1138).
* Add OCaml 5.0 support (goblint/analyzer#1003, goblint/analyzer#945, goblint/analyzer#1162).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug debugging Abstract debugging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle pseudo return nodes in server mode cfg/lookup request
3 participants