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

Bounds checking for xs:integer is broken. #760

Closed
LeoWoerteler opened this issue Sep 26, 2013 · 5 comments
Closed

Bounds checking for xs:integer is broken. #760

LeoWoerteler opened this issue Sep 26, 2013 · 5 comments
Assignees

Comments

@LeoWoerteler
Copy link
Member

BaseX uses double calculations in order to check if 64-bit long calculations overflow. As IEEE754 doubles only have 53 significant binary digits but operations on long need up to 128, this is unsafe:

-9223372036854775807 - 1024 silently underflows to 9223372036854774785 even though the boundary check in org.basex.query.expr.Calc is in place.

@LeoWoerteler
Copy link
Member Author

More weirdness:

let $P63 := xs:decimal('9223372036854775808'),
    $P64 := $P63 + $P63
return $P63 - $P64 idiv 2

Should return 0, but returns 1 instead.

@LeoWoerteler
Copy link
Member Author

@LeoWoerteler
Copy link
Member Author

Related: -9223372036854775808 is a valid xs:integer constant, but it cannot be parsed because it is parsed as -(9223372036854775808) and 9223372036854775808 is not in range. This is quite confusing, because -9223372036854775807 - 1 works fine.

I don't think this is easy to fix because the unary minus has a ValueExpr as argument, which can e.g. be a SimpleMapExpr: -1 ! 2 ! 3 is parsed as -(1 ! 2 ! 3), not (-1) ! 2 ! 3. This means that we cannot just look ahead if the next token after the - is a number.

A solution would be to parse an integer literal 123 as xs:integer('123') and let the optimizer do the evaluation. Then we could look for the pattern -(xs:integer('9223372036854775808')) and evaluate it at parse time.

ChristianGruen added a commit that referenced this issue Apr 27, 2014
Exact bounds checking for `xs:integer` operations.
@ChristianGruen
Copy link
Member

The bugs of the first two comments are resolved; the last observation could be resolved by introducing BigInt instances (the cast proposal leads to wrong error codes, FORG0001 instead of FOAR0002).

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