Skip to content

Add Callgraph#185

Merged
jubnzv merged 58 commits into
nowarp:masterfrom
Esorat:91-add-callgraph
Nov 6, 2024
Merged

Add Callgraph#185
jubnzv merged 58 commits into
nowarp:masterfrom
Esorat:91-add-callgraph

Conversation

@Esorat
Copy link
Copy Markdown
Member

@Esorat Esorat commented Oct 17, 2024

Closes #91

  • I have updated CHANGELOG.md
  • I have added tests to demonstrate the contribution is correctly implemented
  • No test failures were reported when running yarn test-all
  • I did not do unrelated and/or undiscussed refactorings

Copy link
Copy Markdown
Member

@jubnzv jubnzv left a comment

Choose a reason for hiding this comment

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

I think, that's a good start. Please add an entry to CHANGELOG, and merge the master branch to your PR in order to simplify the test suite.

Comment thread src/internals/ir/callGraph.ts
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread callgraph.dot Outdated
Comment thread src/detectors/builtin/sendInLoop.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
@jubnzv jubnzv changed the title fix: Draft Pr for check CallGraph Add Callgraph Oct 18, 2024
@jubnzv jubnzv marked this pull request as draft October 18, 2024 07:37
…pdate constructor and make astStore public

- Refactored `getFunctionName` to use a switch statement for better type safety.
- Introduced `forEachExpression` helper function to handle expression processing recursively and improve code maintainability.
- Updated constructor to remove `astStore` and pass it via the `build()` method, while making `astStore` a public property.
- Updated Ir constructor add CallGraph
- Remove from git .dot output file
Comment thread src/internals/ir/callGraph.ts
Comment thread src/internals/ir/builders/ir.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts
Copy link
Copy Markdown
Member

@jubnzv jubnzv left a comment

Choose a reason for hiding this comment

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

It is much better now. We need to:

  • update the changelog
  • introduce some tests in test/tactIR.spec.ts
  • write a separate tool to dump the callgraph in the supported formats

Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts
Comment thread src/internals/ir/callGraph.ts Outdated
Add tests.  create Dump callgraph ( in detectors can add this line (    DumpCallGraph.run(cu); )
Comment thread src/internals/ir/builders/astStore.ts Outdated
Comment thread src/detectors/detector.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/tools/dumpCallgraph.ts Outdated
@Esorat Esorat marked this pull request as ready for review October 20, 2024 07:33
Comment thread test/all/syntax.expected.callgraph.dot Outdated
Comment thread src/tools/dumpCallgraph.ts Outdated
Comment thread src/tools/dumpCallgraph.ts Outdated
Comment thread src/tools/dumpCallgraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Copy link
Copy Markdown
Member

@jubnzv jubnzv left a comment

Choose a reason for hiding this comment

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

Overall looks good. Let's fix minor issues before merging it.

Comment thread src/internals/ir/builders/ir.ts Outdated
Comment thread src/internals/ir/builders/ir.ts Outdated
Comment thread src/internals/ir/callGraph.ts
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts
Comment thread src/internals/ir/callGraph.ts
@jubnzv
Copy link
Copy Markdown
Member

jubnzv commented Nov 6, 2024

We also need an entry in CHANGELOG.md

Comment thread CHANGELOG.md Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
Comment thread src/internals/ir/callGraph.ts Outdated
jubnzv and others added 14 commits November 6, 2024 10:28
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Co-authored-by: Georgiy Komarov <jubnzv@gmail.com>
Copy link
Copy Markdown
Member

@jubnzv jubnzv left a comment

Choose a reason for hiding this comment

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

LGTM!

@jubnzv jubnzv merged commit a26986c into nowarp:master Nov 6, 2024
@nowarp nowarp deleted a comment Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Callgraph

2 participants