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

Stack memory tracking improvements #573

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Conversation

dthaler
Copy link
Contributor

@dthaler dthaler commented Jan 22, 2024

Example before instruction:

s[504...511].type=number, s[504...511].svalue=1311768467463790320, s[504...511].uvalue=1311768467463790320]

The instruction then sets

s[504...507].type=number, s[504...507].svalue=2596069105, s[504...507].uvalue=2596069105

Prior to this PR, after instruction, the state of s[504...507] is known but the state of s[508...511] is not.
Since it was a singleton number before the instruction, it is possible to retain knowledge of it by
splitting it off from s[504...511] rather than forgetting everything about those bytes. This results
in greater precision, and will allow a number of conformance tests, like lock_add32.data in PR #558 to pass exactly instead of going to TOP.

Fixes #566

@coveralls
Copy link

coveralls commented Jan 22, 2024

Coverage Status

coverage: 90.381% (+0.04%) from 90.344%
when pulling f86b40a on dthaler:track-stack
into 4203e99 on vbpf:main.

@elazarg
Copy link
Collaborator

elazarg commented Mar 30, 2024

Does EBPF runtime guarantee little-endianness?

@dthaler
Copy link
Contributor Author

dthaler commented Mar 30, 2024

Does EBPF runtime guarantee little-endianness?

No. But the CI/CD tests, as well as the ebpf-for-windows runtime, today only runs on little-endian machines.

@elazarg
Copy link
Collaborator

elazarg commented Mar 30, 2024

So endiannes needs to be a parameter, no?

@dthaler
Copy link
Contributor Author

dthaler commented Mar 31, 2024

So endiannes needs to be a parameter, no?

Agreed. I may or may not get to this for a couple of days though, busy at a convention right now.

@dthaler
Copy link
Contributor Author

dthaler commented Apr 8, 2024

So endiannes needs to be a parameter, no?

Agreed. I may or may not get to this for a couple of days though, busy at a convention right now.

Looking into this now. stack.yaml already (before this PR) has a bunch of test cases that assume little-endian.
So given that, it seems that it shouldn't block this PR to add more that assume little-endian.

That said, i am working on changes that allow supporting both, but that would be needed even without this PR.

@elazarg elazarg merged commit 934951e into vbpf:main Apr 11, 2024
13 checks passed
@elazarg
Copy link
Collaborator

elazarg commented Apr 11, 2024

@dthaler this needs to be reopened

@dthaler
Copy link
Contributor Author

dthaler commented Apr 11, 2024

@dthaler this needs to be reopened

I'm still working on changes for big-endian systems. Just slow because I have a bunch of other things going on besides this but I should finish it in the next two weeks and will open an endianness PR then.

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.

Stack memory tracking improvements
3 participants