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

Compute Index Out-Of-Bounds Error #106

Closed
DavePearce opened this issue Apr 8, 2024 · 11 comments · Fixed by #109
Closed

Compute Index Out-Of-Bounds Error #106

DavePearce opened this issue Apr 8, 2024 · 11 comments · Fixed by #109
Labels
bug Something isn't working

Comments

@DavePearce
Copy link
Collaborator

DavePearce commented Apr 8, 2024

I'm not sure whether this is a bug, or I have made a mistake formulating the input. But, the following command fails:

> corset compute --trace test.trace --out test.out test.lisp
thread 'main' panicked at src/compiler/generator.rs:997:67:
byte index 2 is out of bounds of `0`
...

Where we have:

test.lisp

(module test)
(defcolumns (A :binary@prove) (B))
(defconstraint test () (vanishes! (* A B)))

test.trace

{"test": { "A": [0,1], "B": [0,0] } }

I should add that this passes corset check --trace test.trace test.lisp

@DavePearce
Copy link
Collaborator Author

@letypequividelespoubelles Did I formulate the trace file correctly?

@DavePearce
Copy link
Collaborator Author

DavePearce commented Apr 8, 2024

So, the offending chunk of code is this (from src/compiler/generator.rs:

 while let Some(x) = value.next() {
          out.write_all(
               cache
                   .cache_get_or_set_with(x.to_owned(), || {
                         format!("\"0x0{}\"", x.to_string()[2..].trim_start_matches('0')) // <--- HERE
           })

Specifically, x.to_string()[2..] appears to be assuming at least two characters in the original string (hex anyone)?

@letypequividelespoubelles
Copy link
Contributor

(defcolumns (A :binary@prove) B)

Is it working like this ?

@DavePearce
Copy link
Collaborator Author

Is it working like this ?

Well, it doesn't matter whether we have :binary@prove or not --- it crashes eitherway. To be honest, its just a bug in that piece of code. But, I'm confused why that hasn't been caught already. Is it because we are using corset convert now instead of corset compute ?

@DavePearce DavePearce added the bug Something isn't working label Apr 8, 2024
@delehef
Copy link
Contributor

delehef commented Apr 8, 2024

Yes, we never use compute anymore, and very seldom convert (maybe a couple times for advanced debugging).

@DavePearce
Copy link
Collaborator Author

Thanks!! So what is used now instead of compute / convert?

@delehef
Copy link
Contributor

delehef commented Apr 8, 2024

What's the use case?

@DavePearce
Copy link
Collaborator Author

To do the trace expansion? Or is that not supported from the command line anymore? I'm just playing around with it getting a feel for how it works.

@delehef
Copy link
Contributor

delehef commented Apr 8, 2024

To do the trace expansion?

If we want to do that from the CLI rather than through the FFI/lib, then the better course of actions would probably be to leverage code from lib.rs to backport it into corset compute

@DavePearce
Copy link
Collaborator Author

Ok, thanks --- I think that has cleared up what's going on here. Ultimately, it seems the FFI is the only reliable way at this time to do trace expansion. I can fix this if I need to at some point.

DavePearce pushed a commit that referenced this issue Apr 9, 2024
@DavePearce
Copy link
Collaborator Author

Yup, so removing that slice on the to_string() allows corset compute to work again --- thanks. As far as I can tell actually its doing exactly the same thing as the FFI (aside from printing out the result to a file).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants