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

Clarify handling of out-of-bounds memory access #107

Open
PhilippvK opened this issue Nov 21, 2023 · 6 comments
Open

Clarify handling of out-of-bounds memory access #107

PhilippvK opened this issue Nov 21, 2023 · 6 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@PhilippvK
Copy link

Given a register unsigned<XLEN> X[16] what should happen when someone tries to use X[25] in the behavior? This specific case might occur depending on what we end up for #94.

  • This can probably be handled by backend-specific out-of-bounds checks at runtime
  • Backends may allow the user to skip these tests to avoid the implied performance overheads
  • If the range of the index is known during parsing these checks could be omitted
  • ETISS probably won’t even give a Segfault in this case leading to undefined behavior.
  • Documentation should clarify if handling out of bounds access is up to the backend-developer or should be taken care of explicitly when writing CoreDSL code.

What do you think? @eyck @AtomCrafty @wysiwyng

@eyck eyck self-assigned this Nov 21, 2023
@eyck eyck added the documentation Improvements or additions to documentation label Nov 21, 2023
@eyck
Copy link
Contributor

eyck commented Nov 21, 2023

This is simply an invalid CoreDSL description so the validator needs to flag it. Since sizes are defined there can be proper bounds checking implemented in the parser/validator. So no backend handling should be needed...

@PhilippvK
Copy link
Author

And how about these cases?

  • X[rd] with unsigned<5> rd
  • X[rd+2] with unsigned<4> rd

@AtomCrafty
Copy link
Contributor

AtomCrafty commented Nov 21, 2023

@eyck That is incorrect. I can emit warnings if the value of the index expression is known at validation time, but that will not be the general case.

edit: Just checked and those warnings are already implemented.

@AtomCrafty
Copy link
Contributor

AtomCrafty commented Nov 21, 2023

For range accesses into address spaces and bit vectors it currently states that result is undefined if the range falls outside the bounds of the indexed element. I believe this is a sensible rule that should be extended to not only cover the range operator, but also the normal index access operator. Also the specification currently does not mention indexing into arrays, but that should generally follow the same rules as well.

I propose the following changes to the specification:

Currently:

OOB Index Access OOB Range Access
Bit Vectors Unspecified Undefined Behavior
Arrays Unspecified Unspecified
Address Spaces Unspecified Undefined Behavior

Proposed:

OOB Index Access OOB Range Access
Bit Vectors Undefined Behavior Undefined Behavior
Arrays Undefined Behavior Undefined Behavior
Address Spaces Undefined Behavior Undefined Behavior

@eyck
Copy link
Contributor

eyck commented Nov 21, 2023

And how about these cases?

* `X[rd]` with `unsigned<5> rd`

* `X[rd+2]` with `unsigned<4> rd`

Hmm, in the first case the range is 0-31. It can be detected and reported If the length of X is smaller than 32.
The second one needs some further semantic analysis (or constraint propagation) but basically the resulting index range is 2-17. If the length of X is larger than 18 this is fine, otheriwse an error or warning should be reported. Still in the frontend (@AtomCrafty independetly what is being implemented) as static analysis.

Aside of this I agree with @AtomCrafty that we should update the spec.

@AtomCrafty
Copy link
Contributor

AtomCrafty commented Nov 21, 2023

From a language perspective, X[25] is a perfectly valid expression that will simply cause undefined behavior at runtime. We can add warnings if we detect an obvious out-of-bounds access, but it does not qualify as an error.

Using the type of the indexing expression to deduce possible value ranges like you suggest is not supported by the frontend. That would be a major new feature we could possibly add down the road, but it is not feasible right now. Also note that this could only ever be used to flag a small subset of expressions as "definitely unproblematic" and would not be generally able to decide whether an access could possibly be out of bounds at runtime. Deciding that would require a symbolic solver, which is far outside the scope of a language frontend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants