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

Better handling of unexpected operations in bounds-checking #349

Closed
jadephilipoom opened this issue Apr 17, 2018 · 5 comments
Closed

Better handling of unexpected operations in bounds-checking #349

jadephilipoom opened this issue Apr 17, 2018 · 5 comments
Assignees

Comments

@jadephilipoom
Copy link
Collaborator

If ZRange.ident.interp returns None for the bounds of some operation in my code, it would be nice to get some message saying which operation it was--currently I just get "not unifiable" from solve_rop and I'm struggling to make it show me where exactly the None comes from.

@JasonGross
Copy link
Collaborator

This would be a nontrivial amount of work, and I'm not convinced it'll be worth it. The way to do it is to replace the option with an error type, and push that through the rest of the code ... but nth_default is the only operation that sometimes produces Some and sometimes produces None, and, beyond that, there's a list of the operations that produce None here:

| ident.primitive _ _
| ident.Nat_succ
| ident.Nat_add
| ident.Nat_sub
| ident.Nat_mul
| ident.Nat_max
| ident.bool_rect _
| ident.nat_rect _
| ident.pred
| ident.list_rect _ _
| ident.List_nth_default _
| ident.Z_pow
| ident.Z_div
| ident.Z_eqb
| ident.Z_leb
| ident.Z_of_nat
| ident.Z_mul_split
| ident.Z_add_get_carry
| ident.Z_add_with_get_carry
| ident.Z_sub_get_borrow
| ident.Z_modulo
=> fun _ => type.option.None

It should only take 5 minutes to write a procedure that lets you know if a given expression uses any of those (either in Ltac or in Gallina).

(I'm not opposed to having better error messages, I am just hesitant to spend the 3-6 hours I expect it take to write the code that pushes the relevant error messages through partial reduction :-/ )

@jadephilipoom
Copy link
Collaborator Author

Hmm, okay. No worries if it's that much work. I thought it might be easier.

I'm curious about what this 5-minute procedure is, though. I figured out where my problem was coming from[0] by printing the nested ltacs in solve_rop, painstakingly applying them until I had the reified term in view, making a simpler example with the same problem so the term wasn't huge, and then computing it step by step until I could actually see all the operations. It took me about 3 hours. I'm describing the whole process because I'm wondering if you have some more general advice about where to start with debugging when I have opaque problems with the pipeline (e.g. "there's a None in my bounds", "cache_reify takes ages", or "the bounds aren't tight enough").

[0] I think Z.cast2 should have a case in partial.bounds.ident.extract' here. Is there a reason it currently doesn't?

@JasonGross
Copy link
Collaborator

The 5-minute procedure only works if you have the reduced reified term in hand, and it wouldn't've caught [0]

But, uh, I could make the error messages for bad bounds include the reduced syntax tree, post-bounds-analysis, if that'd be useful?

Re [0]: It's because I was operating under the (now clearly mistaken) assumption that the relevant output types were Z and list Z, and not Z * Z. Feel free to add

| ident.Z_cast2 range => fun _ => ZRange.type.option.Some range

or

| ident.Z_cast2 (r1, r2) => fun _ => (Some r1, Some r2)

(they are the same)

@JasonGross
Copy link
Collaborator

Oh, urg, getting "not unifiable" is also not supposed to happen. You're supposed to get a unification failure that contains a description of the error in the term that's not unifying. Let me fix that.

JasonGross added a commit to JasonGross/fiat-crypto that referenced this issue Apr 17, 2018
This should fix mit-plv#349 (or at least most of it).
JasonGross added a commit to JasonGross/fiat-crypto that referenced this issue Apr 17, 2018
@jadephilipoom
Copy link
Collaborator Author

Closed by #351

dderjoel added a commit to dderjoel/fiat-crypto that referenced this issue Mar 22, 2022
Unable to unify: [inr [351, 349, 350]] == [inr [103, 108, 110]]
Could not unify the values at index 0: [mit-plv#351, mit-plv#349, mit-plv#350] != [mit-plv#103, mit-plv#108, mit-plv#110]
index 0: mit-plv#351 != mit-plv#103
(slice 0 44, [mit-plv#345]) != (slice 0 44, [mit-plv#100])
index 0: mit-plv#345 != mit-plv#100
(add 64, [mit-plv#58, mit-plv#95, mit-plv#343]) != (add 64, [mit-plv#58, mit-plv#98])
(add 64, [mit-plv#95, mit-plv#343]) != (add 64, [mit-plv#98])
(add 64, [mit-plv#95, (mul 64, [mit-plv#95, mit-plv#331])]) != (add 64, [(mul 64, [#3, mit-plv#95])])
(add 64, [mit-plv#95, (mul 64, [mit-plv#95, (const 4, [])])]) != (add 64, [(mul 64, [#3, mit-plv#95])])
(add 64, [(or 64, [mit-plv#91, mit-plv#93]), (mul 64, [(or 64, [mit-plv#91, mit-plv#93]), (const 4, [])])]) != (add 64, [(mul 64, [(const 5, []), (or 64, [mit-plv#91, mit-plv#93])])])
JasonGross pushed a commit to dderjoel/fiat-crypto that referenced this issue Mar 22, 2022
Unable to unify: [inr [351, 349, 350]] == [inr [103, 108, 110]]
Could not unify the values at index 0: [mit-plv#351, mit-plv#349, mit-plv#350] != [mit-plv#103, mit-plv#108, mit-plv#110]
index 0: mit-plv#351 != mit-plv#103
(slice 0 44, [mit-plv#345]) != (slice 0 44, [mit-plv#100])
index 0: mit-plv#345 != mit-plv#100
(add 64, [mit-plv#58, mit-plv#95, mit-plv#343]) != (add 64, [mit-plv#58, mit-plv#98])
(add 64, [mit-plv#95, mit-plv#343]) != (add 64, [mit-plv#98])
(add 64, [mit-plv#95, (mul 64, [mit-plv#95, mit-plv#331])]) != (add 64, [(mul 64, [#3, mit-plv#95])])
(add 64, [mit-plv#95, (mul 64, [mit-plv#95, (const 4, [])])]) != (add 64, [(mul 64, [#3, mit-plv#95])])
(add 64, [(or 64, [mit-plv#91, mit-plv#93]), (mul 64, [(or 64, [mit-plv#91, mit-plv#93]), (const 4, [])])]) != (add 64, [(mul 64, [(const 5, []), (or 64, [mit-plv#91, mit-plv#93])])])
JasonGross pushed a commit that referenced this issue Mar 23, 2022
Unable to unify: [inr [351, 349, 350]] == [inr [103, 108, 110]]
Could not unify the values at index 0: [#351, #349, #350] != [#103, #108, #110]
index 0: #351 != #103
(slice 0 44, [#345]) != (slice 0 44, [#100])
index 0: #345 != #100
(add 64, [#58, #95, #343]) != (add 64, [#58, #98])
(add 64, [#95, #343]) != (add 64, [#98])
(add 64, [#95, (mul 64, [#95, #331])]) != (add 64, [(mul 64, [#3, #95])])
(add 64, [#95, (mul 64, [#95, (const 4, [])])]) != (add 64, [(mul 64, [#3, #95])])
(add 64, [(or 64, [#91, #93]), (mul 64, [(or 64, [#91, #93]), (const 4, [])])]) != (add 64, [(mul 64, [(const 5, []), (or 64, [#91, #93])])])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants