Commit Graph

283 Commits

Author SHA1 Message Date
Alessandro Arzilli
ac81269eef
proc: fix prettyprint for register components with large values (#3022)
Fix pretty printing for CPU register components (created with the
XMM0.uintN syntax) while using format strings
Also fixes printing large literal constants with format strings.

Fixes #3020
2022-06-02 10:16:32 -07:00
Alessandro Arzilli
b53fcbe43a
proc: fix RFLAGS corruption after call injection on amd64 (#3002)
debugCallV2 for amd64 has a bug where it corrupts the flags registers
every time it is called, this commit works around that problem by
restoring flags one extra time to its original value after stepping out
of debugCallV2.

Fixes #2985
2022-05-05 08:41:40 -07:00
Alessandro Arzilli
c9d800edb9
proc: support function call injection on arm64 (#2996)
* _scripts/test_linux.sh,_scripts/test_windows.ps1: always return exit code 0 when testing on tip

Same as what we do for test_mac.sh

* proc: support function call injection on arm64

Support function call injection on arm64 with go1.19
2022-05-03 10:46:24 -07:00
Alessandro Arzilli
3138157826
proc: skip autogenerated functions correctly (#2959)
Adjust heuristic used to skip autogenerated functions so that it works
correctly with the new naming scheme in go1.18.

Fixes #2948
2022-04-13 15:28:56 -07:00
Alessandro Arzilli
2b97231e30
proc,service: pretty print time.Time variables (#2865)
Fixes #999
2022-03-25 13:59:57 -07:00
Alessandro Arzilli
1418cfd385
proc: better handling of hardcoded breakpoints (#2852)
This commit improves the handling of hardcoded breakpoints in Delve.
A hardcoded breakpoint is a breakpoint instruction hardcoded in the
text of the program, for example through runtime.Breakpoint.

1. hardcoded breakpoints are now indicated by setting the breakpoint
   field on any thread stopped by a hardcoded breakpoint
2. if multiple hardcoded breakpoints are hit during a single stop all
   will be notified to the user.
3. a debugger breakpoint with an unmet condition can't hide a hardcoded
   breakpoint anymore.
2022-02-22 09:57:37 -08:00
Hyang-Ah Hana Kim
cec23c0aa1
go.mod: require go-delve/liner instead of peterh/liner (#2905)
Use of `replace` in go.mod breaks delve installation using
`go install`. (see https://github.com/golang/go/issues/40276)
Workaround this limitation by explicitly require the fork
github.com/go-delve/liner.

go-delve/liner@v1.2.2-1 already has go.mod module name fixed
to be github.com/go-delve/liner.

Fixes go-delve/delve#2904
2022-02-14 18:42:43 +01:00
hitzhangjie
9c18bb46dd
terminal/colorize: avoid print multiple package by accidently ast.Node startPos==token.NoPos (#2898)
fix #2896
2022-02-10 09:50:31 -08:00
Derek Parker
8ddb64c808
pkg/proc: handle double inlined calls (#2880)
It's possible that an inlined function call also contains an inlined
sunroutine. In this case we should also parse the children of
inlined calls to ensure we don't lose this information.
2022-01-24 08:56:37 +01:00
Hyang-Ah Hana Kim
21bdb466f1
dap: support 'Env' attribute for launch requests (#2846)
* dap: support 'Env' attribute for launch requests

Env is applied in addition to the delve process environment
variables. The env setting is done by calling os.Setenv
as early as possible when a Launch request is received.

Prior discussion is in https://github.com/go-delve/delve/pull/2582

In Visual Studio Code, setting null for an environment variable
in launch.json or tasks.json indicates users want to unset
the environment variable. Support the behavior by accepting
nil value.

* dap: Env field itself can be omitempty

* edit comment
2022-01-06 09:01:09 -08:00
Alessandro Arzilli
b8a9ae26f6
proc: fix signal handling during stepping (#2803)
Fix signal handling during thread single stepping so that signals that
are generated by executing the current instruction are immediately
propagated to the inferior, while signals other signals sent to the
thread are delayed until the full resume happens.

Fixes a bug where a breakpoint set on an instruction that causes a
SIGSEGV would make Delve hang and a bug where signals received during
single step would make it look like an instruction is executed twice.

Fixes #2801
Fixes #2792
2021-12-07 09:21:53 -08:00
Alessandro Arzilli
c207db792a
proc,locspec: support setting breakpoints by func name on generic funcs (#2745)
* proc,locspec: support setting breakpoints by func name on generic funcs

Changes proc.Function to parse function names correctly when they
contain instantiation lists and locspec to match generic functions.

* vendor: update golang.org/x/tools

The old version of golang.org/x/tools is incompatible with the new
iexport format.
2021-10-30 11:52:26 -07:00
Alessandro Arzilli
884ef4f338
proc/gdbserial: workaround for debugserver register set bug (#2770)
Debugserver has a bug where writing to a AVX-2 or AVX-512 register does
not work unless it is followed by at least a write to a AVX (not 2 or
512) register.

See also: https://bugs.llvm.org/show_bug.cgi?id=52362

Fixes #2767
2021-10-30 11:51:02 -07:00
Suzy Mueller
f8deab8522
service/dap: fix goroutine id selection for hardcoded breakpoints (#2748)
* service/dap: fix goroutine id selection for hardcoded breakpoints

Determining the stopped goroutine id on a breakpoint required
checking for breakpoints since some may be tracepoints. However,
there may be goroutines stopped on hardcoded breakpoints with
no breakpoint. We fix this by checking for runtime.breakpoint or
StopReason=proc.StopHardcodedBreakpoint.
2021-10-29 19:40:16 -07:00
Alessandro Arzilli
1893c9769b
Miscellaneous fixes for Windows native backend (#2736)
* proc/native: always stop after RequestManualStop on Windows

On Windows RequestManualStop will generate an exception on a special
DbgUiRemoteBreakin thread, sometimes this thread will die before we
finish stopping the process. We need to account for that and still stop
even if the thread is gone and no other thread hit a breakpoint.

Fixes flakiness of TestIssue419.

* proc/native: fix watchpoints with new threads on Windows

When a new thread is created we must reapply all watchpoints to it,
like we do on linux.

* tests: be lenient on goroutinestackprog tests on Windows

We can not guarantee that we find all goroutines stopped in a good
place and sometimes the stacktrace fails on Windows.
2021-10-13 08:43:54 -07:00
polinasok
7899e94ff2
_fixtures/loopprog: print less often (#2738)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-10-13 09:10:55 +02:00
polinasok
0baf3b7cf1
service/dap: fix nil dereference when byte array cannot be converted to string (#2733)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-10-08 08:57:51 +02:00
Alessandro Arzilli
348c722981
proc/gdbserver: support watchpoints (#2659)
Adds watchpoint support to gdbserver backend for rr debugger and
debugserver on macOS/amd64 and macOS/arm64.

Also changes stack watchpoints to support reverse execution.
2021-10-04 14:45:05 -07:00
aarzilli
18f2a4c46b proc,dwarf/godwarf: support parametric types with dictionaries
Change debug_info type reader and proc to convert parametric types into
their real types by reading the corresponding dictionary entry and
using the same method used for interfaces to retrieve the DIE from a
runtime._type address.

'2586e9b1'.
2021-10-02 15:44:30 +02:00
aarzilli
4e7b689e1a proc: rewrite FindFileLocation to support generics
With generics a single function can have multiple concrete
instantiations, the old version of FindFileLocation supported at most
one concrete instantiation per function and any number of inlined
calls, this supports any number of inlined calls and concrete
functions.
2021-10-02 15:44:30 +02:00
Michael Knyszek
041eedd126
pkg/proc: merge register data before writing to register (#2699)
Right now, if (*compositeMemory).WriteMemory needs to write a value to
a register that's smaller than the full size of the register (say, a
uint32 being passed as an argument), then (*AMD64Registers).SetReg can
later fail a sanity check that ensures the passed DwarfRegister is a
full size register.

Fix this by reading the old value of the register and overwriting just
the relevant parts with the new register. For the purposes of an
argument, it would probably be fine to just pad with zeroes, but merging
with the existing value is what gdb does.

Fixes #2698
2021-09-24 15:27:44 -07:00
Alessandro Arzilli
4264bf00f2
proc,terminal,service: support stack watchpoints (#2521)
* terminal,service: add way to see internal breakpoints

Now that Delve has internal breakpoints that survive for long periods
of time it will be useful to have an option to display them.

* proc,terminal,service: support stack watchpoints

Adds support for watchpoints on stack allocated variables.

When a stack variable is watched, in addition to the normal watchpoint
some support breakpoints are created:

- one breakpoint inside runtime.copystack, used to adjust the address
  of the watchpoint when the stack is resized
- one or more breakpoints used to detect when the stack variable goes
  out of scope, those are similar to the breakpoints set by StepOut.

Implements #279
2021-08-09 10:41:25 -07:00
Alessandro Arzilli
f0a32c8e1b
Go 1.17 support branch (#2451)
* proc: support new Go 1.17 panic/defer mechanism

Go 1.17 will create wrappers for deferred calls that take arguments.
Change defer reading code so that wrappers are automatically unwrapped.

Also the deferred function is called directly by runtime.gopanic, without going through runtime.callN which means that sometimes when a panic happens the stack is either:

0. deferred function call
1. deferred call wrapper
2. runtime.gopanic

or:

0. deferred function call
1. runtime.gopanic

instead of always being:

0. deferred function call
1. runtime.callN
2. runtime.gopanic

the isPanicCall check is changed accordingly.

* test: miscellaneous minor test fixes for Go 1.17

* proc: resolve inlined calls when stepping out of runtime.breakpoint

Calls to runtime.Breakpoint are inlined in Go 1.17 when inlining is
enabled, resolve inlined calls in stepInstructionOut.

* proc: add support for debugCallV2 with regabi

This change adds support for the new debug call protocol which had to
change for the new register ABI introduced in Go 1.17.

Summary of changes:
- Abstracts over the debug call version depending on the Go version
  found in the binary.
- Uses R12 instead of RAX as the debug protocol register when the binary
  is from Go 1.17 or later.
- Creates a variable directly from the DWARF entry for function
  arguments to support passing arguments however the ABI expects.
- Computes a very conservative stack frame size for the call when
  injecting a call into a Go process whose version is >=1.17.

Co-authored-by: Michael Anthony Knyszek <mknyszek@google.com>
Co-authored-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>

* TeamCity: enable tests on go-tip

* goversion: version compatibility bump

* TeamCity: fix go-tip builds on macOS/arm64

Co-authored-by: Michael Anthony Knyszek <mknyszek@google.com>
2021-07-08 08:47:53 -07:00
Alessandro Arzilli
1b0c4310c4
proc: give unique addresses to registerized variables (#2527)
We told clients that further loading of variables can be done by
specifying a type cast using the address of a variable that we
returned.
This does not work for registerized variables (or, in general,
variables that have a complex location expression) because we don't
give them unique addresses and we throw away the compositeMemory object
we made to read them.

This commit changes proc so that:

1. variables with location expression divided in pieces do get a unique
   memory address
2. the compositeMemory object is saved somewhere
3. when an integer is cast back into a pointer type we look through our
   saved compositeMemory objects to see if there is one that covers the
   specified address and use it.

The unique memory addresses we generate have the MSB set to 1, as
specified by the Intel 86x64 manual addresses in this form are reserved
for kernel memory (which we can not read anyway) so we are guaranteed
to never generate a fake memory address that overlaps a real memory
address of the application.

The unfortunate side effect of this is that it will break clients that
do not deserialize the address to a 64bit integer. This practice is
contrary to how we defined our types and contrary to the specification
of the JSON format, as of json.org, however it is also fairly common,
due to javascript itself having only 53bit integers.

We could come up with a new mechanism but then even more old clients
would have to be changed.
2021-07-02 18:37:55 +02:00
Alessandro Arzilli
7c82164264
terminal,service: Add filtering and grouping to goroutines command (#2504)
Adds filtering and grouping to the goroutines command.

The current implementation of the goroutines command is modeled after
the threads command of gdb. It works well for programs that have up to
a couple dozen goroutines but becomes unusable quickly after that.

This commit adds the ability to filter and group goroutines by several
different properties, allowing a better debugging experience on
programs that have hundreds or thousands of goroutines.
2021-07-01 11:25:33 -07:00
Suzy Mueller
1ebfc5c37b
service/dap: add throw reason to exception info (#2524)
We can get the throw reason by looking at the argument "s" in runtime.throw. This is not currently working in Go 1.16 or Go 1.17 (see golang/go#46425), but does work in Go 1.15 and Go 1.14
2021-06-28 08:39:34 -07:00
polinasok
0a19c78021
service/dap: increase default string loading limit and refactor (#2546)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-06-22 08:14:47 -07:00
polinasok
69485a639d
service/dap: separate tests for var value truncation and str loading limits (#2539)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-06-16 13:27:38 -07:00
Alessandro Arzilli
585c711ce5
proc: when converting registers to slices set Base address (#2517)
If the base address isn't set then indexing and slicing will not work.
Large floating point registers already had the base set but small
general purpose registers did not.
2021-06-02 13:46:24 -07:00
Alessandro Arzilli
d552b33822
gdbserial,regnum: support ZMM registers, disable 'g' on debugserver (#2498)
Adds DWARF register number and support for AVX-512 registers.
Changes proc/gdbserial so that the 'g' and 'G' commands are never used
with debugserver since they seem to corrupt the thread state when used
on AVX-512 capable hardware.
Also changes TestClientServer_FpRegisters to be simpler and more
resilient to changes to the Go runtime.

Fixes #2479
2021-05-26 08:21:03 -07:00
Hyang-Ah Hana Kim
5e12091da2
dap: use larger string type variable load limits in 'repl', 'variables' context (#2418)
* dap: use larger variable load limits in 'repl', 'variables' context

When evaluate requests are triggered in the context of 'repl'
(DEBUG CONSOLE in VSCode) or 'variables' (copy values from VARIABLES
section in VSCode), they are the result of human action and have
more rooms to display. So it is not too bad to apply longer limits.

Variable auto-loading for strings or arrays is nice but currently
it's unclear to me how this should be integrated in the DEBUG
CONSOLE or with the Copy Value feature. Until we have better ideas
and tools, let's go with these larger limits.

Unfortunately, the "Copy Value" from WATCH section triggers evaluate
requests with "watch" context and we don't want to load large data
automatically for "watch". So, users who want to query a large value
should first copy the expression to DEBUG CONSOLE and evaluate it.
Not ideal but not the end of the world either.

Updates golang/vscode-go#1318

* dap: apply large limit only to the string type result

* dap: move string reload logic to convertVariable* where other reload logic is

Currently we are thinking string reload for evaluation as a temporary
workaround until we figure out an intutitive way to present long strings.
So, I hope moving this logic near other reload logic may be better.

And, use the address based expression when reloading - when handling the
function return values, we may not have an expression to use.

* dap: make deep source check happy

* dap: move string reevaluation logic back to onEvaluateRequest

Reloading string variables is tricky if they are in registers.
We don't attempt to reload them but for clarity, move this up
to the onEvaluateRequest handler.

For function call, use a generous limit for string load
since the results are volatile.

* dap: check variable isn't affected by evaluate in other context
2021-05-25 10:23:41 -07:00
Hyang-Ah Hana Kim
c8934dc33b
dap: handle SetVariable requests (#2440)
* dap: handle SetVariable requests

The handler invokes debugger.SetVariableInScope, except for
string type variables. For which, we rely on the `call` command.
Moved the call expression handling logic to the new `doCall`
function, so it can be reused by the SetVariable requenst
handler.

With this PR, every successful SetVariable request triggers
a StoppedEvent - that's a hack to reset the variablesHandle
map internally and notify the client of this change. It will
be nice if we can just update cached data corresponding to
the updated variable.  But I cannot find an easy and safe way
to achieve this yet.

Also fixed a small bug in the call expression evaluation -
Previously, dlv dap returned an error "Unable to evaluate
expression: call stopped" if the call expression is for
variable assignment.  (e.g. "call animal = "rabbit").

* dap: address comments from aarzilli

resetHandlesForStop & sendStoppedEvent unconditionally after
call command is left as a TODO - This is an existing code path
(just refactored) and an preexisting bug. Fixing it here
requires updates in TestEvaluateCallRequest and I prefer
addressing it in a separate cl.

Disabled call injection testing on arm64. Separated TestSetVariable
into two, one that doesn't involve call injection and another that
may involve call injection.

Fixed variableByName by removing unnecessary recursion.

* dap: address polina's comments

- removed the hard reset for every variable set
- added tests for various variable types
- added tests that involves interrupted function calls. (breakpoint/panic)

And,
- changed to utilize EvalVariableInScope to access the variable instead
of searching the children by name.
- changed to utilize evaluate requests when verifying whether the variable
is changed as expected in testing. Since now we avoid resetting the variable
handles after variable reset, either we need to trigger scope changes
explicitly, or stop depending on the variables request.

* dap: address comments

- Discuss the problem around the current doCall implementation
and the implication.
- Refine the description on how VS Code handles after setVariable
and evaluate request (there could be followup scopes/evaluate requests).
- Use the explicit line numbers for breakpoints in the SetVariable tests.
- Do not use errors.Is - we could've used golang.org/x/xerrors polyfill
but that's an additional dependency, and we will remove this check once
tests that depend on old behavior are fixed.

* dap: remove errTerminated and adjust the test

* dap: evaluate in the outer frame, instead of advancing to the next bp
2021-05-20 10:05:47 -07:00
Alessandro Arzilli
58762685e3
proc/native: low level support for watchpoints in linux/amd64 (#2301)
Adds the low-level support for watchpoints (aka data breakpoints) to
the native linux/amd64 backend.

Does not add user interface or functioning support for watchpoints
on stack variables.

Updates #279
2021-05-06 10:33:56 -07:00
polinasok
32f646e3e8
service/dap: auto-loading for fully missing children of nested vars (#2455)
* service/dap: auto-loading for fully missing pointers, structs, maps, slices and arrays

* Add call test

* Add TODO

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-05-04 12:53:42 -07:00
polinasok
2414dcdd30
service/dap: truncate long compound map keys and use unique address suffix for uniqueness (#2399)
* Truncate long compound map keys and use address suffix only for those

* Remove test typo that causes failures

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-03-25 09:44:32 -07:00
Álex Sáez
f5d2e132bc
*: Adds toggle command (#2208)
* Adds toggle command

Also adds two rpc2 tests for testing the new functionality

* Removes Debuggers' ToggleBreakpoint method

rpc2's ToggleBreakpoint now calls AmendBreakpoint
Refactors the ClearBreakpoint to avoid a lock.
2021-03-19 11:02:23 -07:00
polinasok
90fb0a535f
service/dap: support auto-loading of unloaded interfaces (#2362)
* service/dap: support auto-loading of unloaded interfaces

* Make DeepSource happy

* Don't set reference if data failed to auto-load

* Use frame-less expressions

* Refine interface recursion capping test case

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-03-08 09:41:47 -08:00
polinasok
129a9fe46c
service/dap: detect and report unloaded variables (#2353)
* Detect and report unloaded variables

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-02-23 08:29:06 -08:00
polinasok
98728d8fe1
service/dap: improve test coverage for evaluateName feature based on known vscode-go bugs (#2341)
* DAP test for github.com/golang/vscode-go/issues/1056

* DAP test for github.com/golang/vscode-go/issues/884

* DAP test for github.com/golang/vscode-go/issues/851

* DAP test for github.com/golang/vscode-go/issues/1053

* DAP test for github.com/golang/vscode-go/issues/1054

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-02-21 08:00:05 -08:00
Alessandro Arzilli
f19d5e5c13
proc: fix embedded field search (#2320)
Both structMember and findMethod implemented a depth-first search in
embedded fields but the Go specification requires a breadth-first
search. They also allowed promotion of fields in the concrete type of
embedded interfaces even though this is not allowed by Go.
Furthermore they both lacked protection from infinite recursion
when a type embeds itself and the user requests a non-existent field.

Fixes #2316
2021-01-29 09:25:31 -08:00
Than McIntosh
dceffacb89
pkg/proc: fix for file reference handling with DWARF 5 compilation units (#2327)
Add a helper method for collecting line table file references that
does the correct thing for DWARF 5 vs DWARF 4 (in the latter case you
have an implicit 0 entry which is the comp dir, whereas in the former
case you do not). This is to avoid out-of-bounds errors when examining
the file table section of a DWARF 5 compilation unit's line table.

Included is a new linux/amd-only test that includes a precompiled C
object file with a DWARF-5 section that triggers the bug in question.

Fixes #2319
2021-01-29 09:23:52 -08:00
Alessandro Arzilli
d3e9158e9e
proc: make nested function calls work when stopped at a sw breakpoint (#2232)
evalFunctionCall needs to remove the breakpoint from the current thread
after starting the function call injection, otherwise Continue will
think that the thread is stopped at a breakpoint and return to the user
instead of continuing the call injection.
2020-12-10 09:03:11 -08:00
aarzilli
6ab6929b9a proc/gdbserial: fix redirect bugs with debugserver
1. Forward stdin/stdout/stderr to the target process when in foreground
   mode instead of always forwarding the current tty (issue #1964)
2. When redirecting a file descriptor make sure to also specify
   something for all three otherwise debugserver will misbehave (either
   exit on launch or run but giving the target process a closed file
   descriptor).

Fixes #1964
2020-11-17 16:00:22 +01:00
polinasok
c94db60d27
service/dap: support evaluate requests with expressions and calls (#2185)
* Support evaluate request

* Fix failing tests

* Call support

* Remove debugger.CurrentThread() that got accidentally reintroduced during merge

* Address review comments

* Function to stringify stop reason

* Add resetHandlesForStop

* Handle stop inside call

* More tests

* Address review comments

* Check all threads to determine if call completed

* Fix test

* Fix test

* Fix test

* Address review comments

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-11-12 15:24:31 -08:00
chainhelen
0bbf9e4eaf
*: Closer close and use assertNoError for testcase (#2204) 2020-10-19 14:44:04 -07:00
Alessandro Arzilli
a31631b5f9
proc/test: fix TestStepCallPtr on linux/386 (#2193)
The test needs to set a breakpoint on main.CallFn after the prologue,
on linux/386 this function does not have any instruction after the
prologue on the function header line  because it doesn't need to
allocate space for local variables. Change the fixture so that this
isn't a problem.

This bug results on the test failing a small percentage of the time.

Co-authored-by: a <a@kra>
2020-10-12 15:07:24 -07:00
chainhelen
e07bfd3180
pkg/proc: fix dlv panic when sameGCond is nil. (#2164)
`sameFrameCond` should not be constructed as one `And Express` when
`sameGCond` which is the first child of `BinaryExpr` is nil.

Fixes: #2162
2020-09-08 15:18:49 -07:00
aarzilli
7555d1c063 cmd,proc,terminal,debugger: Support default file descriptor redirects
Adds features to support default file descriptor redirects for the
target process:

1. A new command line flag '--redirect' and '-r' are added to specify
   file redirects for the target process
2. New syntax is added to the 'restart' command to specify file
   redirects.
3. Interactive instances will check if stdin/stdout and stderr are
   terminals and print a helpful error message if they aren't.
2020-09-01 21:50:27 +02:00
aarzilli
f90134eb4d proc: prevent internal breakpoint conditions from failing
An internal breakpoint condition shouldn't ever error:
* use a ThreadContext to evaluate conditions if a goroutine isn't
  available
* evaluate runtime.curg to a fake g variable containing only
  `goid == 0` when there is no current goroutine

Fixes #2113
2020-09-01 15:01:39 +02:00
Alessandro Arzilli
5a5d5f9e68
proc: fix support for AVX registers (#2139)
Recent changes to the way registers are handled broke reporting of AVX
registers (i.e. YMMx). This change restores the functionality by:

- concatenating the higher half of the YMMx registers to their
  corresponding XMMx lower half (YMMx registers do not have an
  independent DWARF register number)
- modifying the formatSSEReg function to handle them when they are
  present.

Fixes #2033
2020-08-31 10:55:43 -07:00