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

Match the base pointer of llvm_elem and llvm_field. #989

Merged
merged 1 commit into from
Jan 6, 2021

Conversation

andreistefanescu
Copy link
Contributor

@andreistefanescu andreistefanescu commented Jan 5, 2021

Currently, saw supports overrides of the form llvm_execute_func [x_p, (llvm_elem x_p 1)];, while it does not support overrides of the form llvm_execute_func [(llvm_elem x_p 1), x_p];, because matching on llvm_elem x_p 1 fails unless x_p is already matched. This changes the handling of llvm_elem to match the base pointer. This changes the handling of llvm_field in a similar way.

@atomb
Copy link
Contributor

atomb commented Jan 5, 2021

Could you provide a bit more description of the goal of this PR?

@andreistefanescu
Copy link
Contributor Author

@atomb I updated the description. Take a look at the tests, they are failing with the current version of saw.

Copy link
Contributor

@brianhuffman brianhuffman left a comment

Choose a reason for hiding this comment

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

As I understand it, this patch basically makes llvm_elem and llvm_field work as patterns. So when you want to match llvm_elem n lhs against rhs, that reduces to matching lhs against rhs - offset (where offset is computed from n and the pointer type).

I've thought about the possibility of implementing this before, and this is exactly how I would have done it. The code changes look good, so I think it's ready to merge as soon as we fix the test cases (which I'm going to have a look at now).

@brianhuffman
Copy link
Contributor

When I run saw manually on test0065_match_llvm_elem/test.saw, it works just fine. But my runtests.sh says it fails; I think it must be running an out-of-date saw executable.

@andreistefanescu
Copy link
Contributor Author

@brianhuffman the tests pass in github actions, and fail in travis because of missing external abc. The test added by this pull request passes in both. Or are you seeing a different failure?

I guess it's time to remove travis entirely.

@brianhuffman
Copy link
Contributor

My local failure was bogus, due to an out-of-date saw binary. Everything seems to be working fine. We can safely ignore the failing travis integration test.

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.

3 participants