erl_debugger: Add BIFs to list available line breakpoints and their status #9784
Conversation
In several BIFs we were constructing tuples manually, it is more clear with TUPLEX macros
These make it easier to use them in BIFs, especially those that need to build maps of arbitrary sizes
CT Test Results 4 files 201 suites 1h 52m 48s ⏱️ For more details on these failures, see this check. Results for commit e01435e. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
bjorng
force-pushed
the
list_line_breakpoints
branch
from
8e36e0c to
35fb56a
Compare
|
Thanks for the pull request. I have cleaned up indentation in the last commit and force-pushed and the PR to our daily builds. |
erts/emulator/beam/erl_debugger.c
Outdated
| default: | ||
| ASSERT(0); | ||
| continue; |
There was a problem hiding this comment.
Nitpick, all enum values are handled already and the compiler should know that:
| default: | |
| ASSERT(0); | |
| continue; |
There was a problem hiding this comment.
the compiler should know that
True but it currently emits a warning, not an error, if I comment out the default and an additional case. At least for me, I get lots of warnings when building the VM (mostly -Warray-bounds, I think because foo[1] is used throughout instead of foo[] for "flexible arrays"), so easy to miss. Maybe -Wswitch should be treated as error?
erts/emulator/beam/erl_debugger.c
Outdated
| *hp++ = stack_frame_fun_info(c_p, catch_addr, NULL, 0); | ||
| hp = HAlloc(c_p, tup2_sz); | ||
| result = TUPLE2(hp, | ||
| ERTS_MAKE_AM("catch"), |
There was a problem hiding this comment.
Nitpick: 'catch' is already defined in the standard library, we may as well add it to atom.names :-)
| } else { | ||
| *hp++ = ERTS_MAKE_AM("too_large"); | ||
| *hp++ = make_small(val_size); | ||
| result = TUPLE2(hp, ERTS_MAKE_AM("too_large"), make_small(val_size)); |
There was a problem hiding this comment.
Nitpick: maybe am_system_limit? Users are already familiar with that terminology.
There was a problem hiding this comment.
Notice that this is not a system limit: the limit is specified by the caller. The motivation is to give the caller some control on whether they will hit their heap limit by copying a gigantic term while peeking, or avoid the serialization overhead, etc.
There was a problem hiding this comment.
How about size then -- you either return a {value, Value} or a {size, Size}?
| stack_frame_fun_info(BIF_P, | ||
| code_ptr, | ||
| rp, | ||
| is_return_addr), |
There was a problem hiding this comment.
Nitpick: indentation looks a little off here.
| #define MAP0(hp) \ | ||
| (MAP_HEADER(hp, 0, TUPLE0), \ | ||
| make_flatmap(hp)) |
There was a problem hiding this comment.
This ought to be a global literal just like TUPLE0.
There was a problem hiding this comment.
I agree, but iiuc this is not something I can do as part of this change, correct? The deduplication needs to happen somewhere, etc
There was a problem hiding this comment.
It's actually a small change: since the key tuple of empty maps must be ERTS_GLOBAL_LIT_EMPTY_TUPLE we can identify all affected places just by searching for ERTS_GLOBAL_LIT_EMPTY_TUPLE and blindly returning ERTS_GLOBAL_LIT_EMPTY_MAP instead of constructing a fresh empty map.
That and expanding the two lines in copy.c saying if (obj == empty_tuple_literal || ...) to also include the empty map should be enough. All in all it's just a handful places.
At the moment, the erl_debugger module allows to toggle line-breakpoints on and off, but there is no way for the caller to know: 1. What lines of a module/function admit breakpoints, 2. What breakpoints are currently enabled So we add two BIFs, `erl_debugger:breakpoints/1` and `erl_debugger:breakpoints/3` that expose this information.
It is already defined in the standard library, so just adding to atom.names
jcpetruzza
force-pushed
the
list_line_breakpoints
branch
from
35fb56a to
e01435e
Compare
|
Pushed new version addressing feedback and fixing corner-case where the module has no line-table |
bjorng
added
testing
3 participants
The
erl_debuggermodule currently exposes a BIF to toggle line-breakpoints, but there was no mechanism to query the status. We add BIFs that, for a give module or for a given MFA, will return all lines that allow breakpoints being set and what their status is.The main motivation is to allow the edb debugger to implement stepping-over and stepping-out without requiring the module to be available on disk, and compiled with
debug_info.