Skip to content

Commit

Permalink
[translation] Fix 32-bit truncation in (( status and for (( condition
Browse files Browse the repository at this point in the history
Bugs reported by Koiche Murase on Zulip.

- We are still using mops.BigTruncate() in BashArray indexing and slicing.
  - However that data structure may change, so I'm not touching it right
    now.
- The printf bug still needs to be fixed.  It's because we use 32-bit
  to_int() rather than 64-bit mops::FromStr().
  • Loading branch information
Andy C committed Jun 22, 2024
1 parent cfed6c3 commit c84c405
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 7 deletions.
7 changes: 5 additions & 2 deletions builtin/printf_osh.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,9 @@ def _Format(self, parts, varargs, locs, out):

try:
# note: spaces like ' -42 ' accepted and normalized
# TODO: use mops.FromStr()
# And mylib.hex_lower() etc. may have to change

d = int(s)
except ValueError:
# 'a is interpreted as the ASCII value of 'a'
Expand Down Expand Up @@ -352,8 +355,8 @@ def _Format(self, parts, varargs, locs, out):
# state.

tzcell = self.mem.GetCell('TZ')
if tzcell and tzcell.exported and tzcell.val.tag(
) == value_e.Str:
if (tzcell and tzcell.exported and
tzcell.val.tag() == value_e.Str):
tzval = cast(value.Str, tzcell.val)
posix.putenv('TZ', tzval.s)

Expand Down
9 changes: 5 additions & 4 deletions osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
from osh import braces
from osh import sh_expr_eval
from osh import word_eval
from mycpp import mops
from mycpp import mylib
from mycpp.mylib import log, probe, switch, tagswitch
from ysh import expr_eval
Expand Down Expand Up @@ -1251,8 +1252,8 @@ def _DoForExpr(self, node):
while True:
if for_cond:
# We only accept integers as conditions
cond_int = self.arith_ev.EvalToInt(for_cond)
if cond_int == 0: # false
cond_int = self.arith_ev.EvalToBigInt(for_cond)
if mops.Equal(cond_int, mops.ZERO): # false
break

try:
Expand Down Expand Up @@ -1574,8 +1575,8 @@ def _Dispatch(self, node, cmd_st):

cmd_st.check_errexit = True
cmd_st.show_code = True # this is a "leaf" for errors
i = self.arith_ev.EvalToInt(node.child)
status = 1 if i == 0 else 0
i = self.arith_ev.EvalToBigInt(node.child)
status = 1 if mops.Equal(i, mops.ZERO) else 0

elif case(command_e.ControlFlow): # LEAF command
node = cast(command.ControlFlow, UP_node)
Expand Down
2 changes: 1 addition & 1 deletion spec/bugs.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ printf '%u\n' 2147483648
2147483648
## END

#### bit shift bug (Koiche on Zulip)
#### (( status bug
case $SH in dash|ash) exit ;; esac

(( 1 << 32 ))
Expand Down
55 changes: 55 additions & 0 deletions spec/for-expr.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,58 @@ for ((i = $"3"; i < $"5"; ++i)); do echo $i; done
## OK zsh STDOUT:
## END


#### Integers near 31, 32, 62 bits

# Hm this was never a bug, but it's worth testing.
# The bug was EvalToInt() in the condition.

for base in 31 32 62; do

start=$(( (1 << $base) - 2))
end=$(( (1 << $base) + 2))

for ((i = start; i < end; ++i)); do
echo $i
done
echo ---
done

## STDOUT:
2147483646
2147483647
2147483648
2147483649
---
4294967294
4294967295
4294967296
4294967297
---
4611686018427387902
4611686018427387903
4611686018427387904
4611686018427387905
---
## END


#### Condition that's greater than 32 bits

iters=0

for ((i = 1 << 32; i; ++i)); do
echo $i
iters=$(( iters + 1 ))
if test $iters -eq 5; then
break
fi
done

## STDOUT:
4294967296
4294967297
4294967298
4294967299
4294967300
## END

0 comments on commit c84c405

Please sign in to comment.