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

cntlm example without duktape #282

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

cntlm example without duktape #282

wants to merge 1 commit into from

Conversation

ailrst
Copy link
Contributor

@ailrst ailrst commented Dec 5, 2024

This adds a lifted build of cntlm with the PAC (proxy auto configuration) feature disabled and duktape removed, from the release here https://github.com/UQ-PAC/cntlm/releases/tag/build-2-glibc

This reduces its size by 20% and makes it much more manageable for basil to analyse.

@l-kent
Copy link
Contributor

l-kent commented Dec 5, 2024

Is there a reason to include the version with duktape in this? We already have that in the repo as-is

@ailrst
Copy link
Contributor Author

ailrst commented Dec 6, 2024

Just so that it is directly comparable to the example without duktape. The existing versions in the repo don't have any information attached about how they were compiled or with which sourcecode.

@l-kent
Copy link
Contributor

l-kent commented Dec 6, 2024

Do we want the existing versions at all? Do we need a version with duktape at all? I don't think we're ever going to be able to do anything useful with it.

@ailrst
Copy link
Contributor Author

ailrst commented Dec 6, 2024

I found duktape useful for stress testing the (intraprocedural) simplification-pass analyses as it contained a lot of complex behaviour and variety. I think its useful as a worst-case test case for analyses.

I'm not sure if we need the existing versions, but I think its good to keep them as they are known to lift successfully and there have been some changes to the lifter since. E.g. the gtirb here doesn't load because theres a new intobits pattern (fixed in simplification-pass branch).

@l-kent
Copy link
Contributor

l-kent commented Dec 6, 2024

If there's been a breaking change to the lifter, that's really something we want to address in a separate pull request so we can get it merged in faster?

@ailrst
Copy link
Contributor Author

ailrst commented Dec 6, 2024

Yeah. But it was trivial to fix in the simplification-pass branch and isn't simple to cherrypick out the change that fixes it.

@ailrst ailrst marked this pull request as draft December 6, 2024 00:33
@ailrst
Copy link
Contributor Author

ailrst commented Dec 6, 2024

I'm going to re-upload the example as there is also an update to the bap plugin this should be re-built using.

@l-kent
Copy link
Contributor

l-kent commented Dec 6, 2024

What's the point of committing this now then if the main branch can't actually use it?

@ailrst
Copy link
Contributor Author

ailrst commented Dec 6, 2024

  • To use to write the fix to main
  • To merge it into simplification pass which already works and is downstream of main
  • Because the BAP example already works

@l-kent
Copy link
Contributor

l-kent commented Dec 6, 2024

If the old version will be redundant once this is merged and the parser is updated for the GTIRB changes, it'd be simpler to remove the old version and update the parser all together here?

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.

2 participants