Skip to content

Commit

Permalink
Fix bug in $LINENO implementation.
Browse files Browse the repository at this point in the history
The location was set AFTER evaluating the argv array!  So the argv array
itself had an old value of $LINENO.

This is issue #113 reported by BatmanAoD.

Also: dump more parts of the stack.  Preparing to expose all this
information in a more principled way.
  • Loading branch information
Andy Chu committed Sep 20, 2018
1 parent faaad11 commit 5451f3f
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 35 deletions.
41 changes: 18 additions & 23 deletions core/cmd_exec.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,30 +630,9 @@ def _Dispatch(self, node, fork_external):

if node.tag == command_e.SimpleCommand:
check_errexit = True
# PROBLEM: We want to log argv in 'xtrace' mode, but we may have already
# redirected here, which screws up logging. For example, 'echo hi
# >/dev/null 2>&1'. We want to evaluate argv and log it BEFORE applying
# redirects.

# Another problem:
# - tracing can be called concurrently from multiple processes, leading
# to overlap. Maybe have a mode that creates a file per process.
# xtrace-proc
# - line numbers for every command would be very nice. But then you have
# to print the filename too.

words = braces.BraceExpandWords(node.words)
argv = self.word_ev.EvalWordSequence(words)

# This is a very basic implementation for PS4='+$SOURCE_NAME:$LINENO:'

# TODO:
# - It should be a stack eventually. So if there is an exception we can
# print the full stack trace. Python has a list of frame objects, and
# each one has a location?
# - The API to get DebugInfo is overly long.
# - Maybe just do a simple thing like osh-o line-trace without any PS4?

# Find span_id for a basic implementation of $LINENO, e.g.
# PS4='+$SOURCE_NAME:$LINENO:'
# NOTE: osh2oil uses node.more_env, but we don't need that.
found = False
if node.words:
Expand All @@ -664,13 +643,29 @@ def _Dispatch(self, node, fork_external):
else:
found = True

# TODO: Move this before word evaluation!
if found:
# NOTE: This is what we want to expose as variables for PS4.
#ui.PrintFilenameAndLine(span_id, self.arena)
self._SetSourceLocation(span_id)
else:
self.mem.SetSourceLocation('<unknown>', -1)

# PROBLEM: We want to log argv in 'xtrace' mode, but we may have already
# redirected here, which screws up logging. For example, 'echo hi
# >/dev/null 2>&1'. We want to evaluate argv and log it BEFORE applying
# redirects.

# Another problem:
# - tracing can be called concurrently from multiple processes, leading
# to overlap. Maybe have a mode that creates a file per process.
# xtrace-proc
# - line numbers for every command would be very nice. But then you have
# to print the filename too.

words = braces.BraceExpandWords(node.words)
argv = self.word_ev.EvalWordSequence(words)

# This comes before evaluating env, in case there are problems evaluating
# it. We could trace the env separately? Also trace unevaluated code
# with set-o verbose?
Expand Down
16 changes: 10 additions & 6 deletions core/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ def MaybeCollect(self, ex):
if not self.do_collect: # Either we already did it, or there is no file
return

# this is a string
ex.mem
self.func_name_stack = list(ex.mem.func_name_stack)
self.var_stack = state._FormatStack(ex.mem.var_stack)
# Copy stack
self.var_stack, self.argv_stack, self.func_name_stack = ex.mem.Dump()

# TODO: Also do functions, aliases, etc.

self.do_collect = False
self.collected = True

Expand Down Expand Up @@ -92,10 +93,13 @@ def MaybeDump(self, status):
return

d = {
#'var_stack': self.var_stack,
'var_stack': self.var_stack,
'argv_stack': self.argv_stack,
'func_name_stack': self.func_name_stack,
}
path = os.path.join(self.crash_dump_dir, 'osh-crash-dump.json')
with open(path, 'w') as f:
print(repr(d), file=f)
import json
json.dump(d, f, indent=2)
#print(repr(d), file=f)
util.log('Wrote crash dump to %s', path)
21 changes: 21 additions & 0 deletions core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,12 @@ def __init__(self, argv):
def __repr__(self):
return '<_ArgFrame %s %d at %x>' % (self.argv, self.num_shifted, id(self))

def Dump(self):
return {
'argv': self.argv,
'num_shifted': self.num_shifted,
}

def GetArgNum(self, arg_num):
index = self.num_shifted + arg_num - 1
if index >= len(self.argv):
Expand All @@ -314,6 +320,16 @@ def __init__(self, readonly=False):
self.vars = {} # string -> runtime.cell
self.readonly = readonly

def Dump(self):
vars_ = {}
for name, cell in self.vars.iteritems():
# NOTE: We're using the ASDL representation. Need to dump it as JSON too.
vars_[name] = str(cell)
return {
'vars': vars_,
'readonly': self.readonly
}

def __repr__(self):
f = cStringIO.StringIO()
f.write('<_StackFrame readonly:%s' % self.readonly)
Expand Down Expand Up @@ -409,6 +425,11 @@ def __repr__(self):
parts.append('>')
return '\n'.join(parts) + '\n'

def Dump(self):
var_stack = [frame.Dump() for frame in self.var_stack]
argv_stack = [frame.Dump() for frame in self.argv_stack]
return var_stack, argv_stack, list(self.func_name_stack)

def _InitDefaults(self):
# Default value; user may unset it.
# $ echo -n "$IFS" | python -c 'import sys;print repr(sys.stdin.read())'
Expand Down
14 changes: 8 additions & 6 deletions spec/introspect.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,20 @@ f # line 9
## END

#### $LINENO is the current line, not line of function call
echo $LINENO # first line
g() {
argv.py $LINENO # line 2
argv.py $LINENO # line 3
}
f() {
argv.py $LINENO # line 5
argv.py $LINENO # line 6
g
argv.py $LINENO # line 7
argv.py $LINENO # line 8
}
f
## STDOUT:
['5']
['2']
['7']
1
['6']
['3']
['8']
## END

0 comments on commit 5451f3f

Please sign in to comment.