Skip to content

Commit

Permalink
Adopt a standard naming convention and signture for debug functions
Browse files Browse the repository at this point in the history
To get the "dd" debugger command to work without having to create an
explicit mapping from type to function either by hand or by runtime
inspection (the latter preventing setting up the command at debugger
startup), the debug functions that dd calls should have a type of
the form

     foo_debug(FILE *fp, foo_t const *)

We add the qualifier becausen
 * some support functions with extra parameters are meant to be
   called by these functions, which pass the additional parameters;
   the functions we do call can pass fp along, or in the case of
   src/lib/util/dict_print.c, add fp to the context
 * fe_dict_attr_t * has three debug functions
 * fr_pair_validate_debug() takes a pointer to an array, and
   thus can't follow the convention
 * virtual_server_{listen, process}_debug() and module_rlm_list_debug()
   have *no* parameters
  • Loading branch information
jejones3141 committed Jan 30, 2025
1 parent 56576cd commit e896b90
Show file tree
Hide file tree
Showing 19 changed files with 261 additions and 219 deletions.
66 changes: 27 additions & 39 deletions debugger/dd.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
#
# Syntax: dd <variable name>
#
# where the variable named either has one of the types
# shown in the _howTo list or whose address has one of the
# types shown in the _howTo list.
# where <variable name> has a type foo * or foo for which
# there is a function foo_debug(FILE *, foo_t *) that presumably
# displays values of type foo_t in a useful, legible format.
#
# When the command is run, it calls the appropriate FreeRADIUS
# function to display the value the variable points at (or
Expand Down Expand Up @@ -42,29 +42,25 @@
# we're running under gdb or lldb, we can have a single Python
# source that can do the right thing for the debugger it's running
# under.
#
# NOTE: this function cheerfully assumes such functions exist, but
# will report if the call fails (modulo an issue with lldb and
# fr_value_box_t being defined twice--see below). The idea is that
# these reports will point out where an appropriate foo_debug()
# function needs to exist or to be modified to fit the signature.

import sys

_howTo = {
'fr_value_box_t *' : ('fr_value_box_debug', True),
'fr_value_box_list_t *' : ('fr_value_box_list_debug', True),
'tmpl_t *' : ('tmpl_debug', True),
'CONF_ITEM *' : ('_cf_debug', True),
'dl_loader_t *' : ('dl_loader_debug', False),
'fr_dict_gctx_t * ' : ('fr_dict_global_ctx_debug', True),
'fr_pair_t *' : ('fr_pair_debug', True),
'fr_pair_list_t *' : ('fr_pair_list_debug', True),
'fr_sbuff_term_t *' : ('fr_sbuff_terminal_debug', True),
'fr_sbuff_parse_rules_t *' : ('fr_sbuff_parse_rules_debug', True),
'fr_sbuff_unescape_rules_t *': ('fr_sbuff_unescape_debug', True),
'tmpl_attr_list_head_t *' : ('tmpl_attr_ref_list_debug', True),
'tmpl_attr_rules_t *' : ('tmpl_attr_rules_debug', True),
'fr_dlist_head_t *' : ('tmpl_extents_debug', False),
'tmpl_request_list_head_t *' : ('tmpl_request_ref_list_debug', True),
'tmpl_rules_t *' : ('tmpl_rules_debug', True),
'lua_State *' : ('_util_log_debug', False),
'xlat_exp_t *' : ('xlat_debug', True)
}
def debug_function(ptr_foo_name):
# ditch trailng ' *' (will always be there)
ptr_foo_name = ptr_foo_name[:-2]
# ditch trailing ' const'
if ptr_foo_name.endswith(' const'):
ptr_foo_name = ptr_foo_name[:-6]
# ditch trailing '_t' (again should always be there)
if ptr_foo_name.endswith('_t'):
ptr_foo_name = ptr_foo_name[:-2]
return f'{ptr_foo_name}_debug'

try:
import gdb # Attempt to import GDB module
Expand All @@ -85,17 +81,12 @@ def dd(debugger, command, exe_ctx, result, internal_dict):
if not sb_var.IsValid():
sb_var = target.FindFirstGlobalVariable(command)
if not sb_var.IsValid():
result.SetError('{} is not a variable'.format(command))
return
result.SetError(f'{command} is not a variable')
arg = sb_var if sb_var.type.is_pointer else sb_var.address_of
type = arg.GetDisplayTypeName()
if not (type in _howTo):
result.SetError('unsupported type "{}"'.format(type))
return
function, const = _howTo[type]
cast = '({} const *)'.format(type[0:-2]) if const else ''
argName = arg.GetName()
cmd = 'expr {0}({1}({2}))'.format(function, cast, argName)
function = debug_function(type)
cast = f'({type[:-2]} const *)'
cmd = f'expr {function}(stderr, {cast}({arg.GetName()}))'
interpreter = debugger.GetCommandInterpreter()
if not interpreter.IsValid():
result.SetError("can't set up SBCommandInterpreter")
Expand All @@ -106,7 +97,7 @@ def dd(debugger, command, exe_ctx, result, internal_dict):
for i in range(2):
if (cmdStatus := interpreter.HandleCommand(cmd, result)) == lldb.eReturnStatusSuccessFinishResult:
return
result.SetError("command {} failed, status {}".format(cmd, cmdStatus))
result.SetError(f'command "{cmd}" failed, status {cmdStatus}')

# And then some boilerplate to set up the command and announce its availability.
# I'm guessing that the -f option is <file name without extension>.<name of function>,
Expand Down Expand Up @@ -138,12 +129,9 @@ def invoke (self, arg, from_tty):
argMod = '&'
var = var.address
varType = str(var.type)
if not (varType in _howTo):
print('unsupported type "{}"'.format(varType))
return
function, const = _howTo[varType]
cast = '({} const *)'.format(varType[0:-2]) if const else ''
command = 'call {0}({1}{2}({3}))'.format(function, cast, argMod, arg)
function = debug_function(varType)
cast = f'({varType[0:-2]} const *)'
command = f'call {function}(stderr, {cast}{argMod}({arg}))'
try:
gdb.execute(command)
except:
Expand Down
4 changes: 2 additions & 2 deletions src/bin/unit_test_attribute.c
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ static int dictionary_load_common(command_result_t *result, command_file_ctx_t *
/*
* Dump the dictionary if we're in super debug mode
*/
if (fr_debug_lvl > 5) fr_dict_debug(cc->tmpl_rules.attr.dict_def);
if (fr_debug_lvl > 5) fr_dict_debug(stderr, cc->tmpl_rules.attr.dict_def);

RETURN_OK(0);
}
Expand Down Expand Up @@ -1756,7 +1756,7 @@ static size_t command_dictionary_attribute_parse(command_result_t *result, comma
static size_t command_dictionary_dump(command_result_t *result, command_file_ctx_t *cc,
UNUSED char *data, size_t data_used, UNUSED char *in, UNUSED size_t inlen)
{
fr_dict_debug(dictionary_current(cc));
fr_dict_debug(stderr, dictionary_current(cc));

/*
* Don't modify the contents of the data buffer
Expand Down
10 changes: 5 additions & 5 deletions src/lib/server/tmpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ typedef enum {
#define tmpl_aexpand_type(_ctx, _out, _type, _request, _vpt, _escape, _escape_ctx) \
_tmpl_to_atype(_ctx, (void *)(_out), _request, _vpt, _escape, _escape_ctx, _type)

void tmpl_debug(tmpl_t const *vpt) CC_HINT(nonnull);
void tmpl_debug(FILE *fp, tmpl_t const *vpt) CC_HINT(nonnull);

fr_pair_list_t *tmpl_list_head(request_t *request, fr_dict_attr_t const *list);

Expand Down Expand Up @@ -1181,11 +1181,11 @@ void tmpl_set_xlat(tmpl_t *vpt, xlat_exp_head_t *xlat) CC_HINT(nonnull);

int tmpl_afrom_value_box(TALLOC_CTX *ctx, tmpl_t **out, fr_value_box_t *data, bool steal) CC_HINT(nonnull);

void tmpl_attr_ref_debug(const tmpl_attr_t *ar, int idx) CC_HINT(nonnull);
void tmpl_attr_ref_debug(FILE *fp, const tmpl_attr_t *ar, int idx) CC_HINT(nonnull);

void tmpl_attr_ref_list_debug(FR_DLIST_HEAD(tmpl_attr_list) const *ar_head) CC_HINT(nonnull);
void tmpl_attr_ref_list_debug(FILE *fp, FR_DLIST_HEAD(tmpl_attr_list) const *ar_head) CC_HINT(nonnull);

void tmpl_attr_debug(tmpl_t const *vpt) CC_HINT(nonnull);
void tmpl_attr_debug(FILE *fp, tmpl_t const *vpt) CC_HINT(nonnull);

int tmpl_attr_copy(tmpl_t *dst, tmpl_t const *src) CC_HINT(nonnull);

Expand Down Expand Up @@ -1328,7 +1328,7 @@ int tmpl_extents_find(TALLOC_CTX *ctx,
int tmpl_extents_build_to_leaf_parent(fr_dlist_head_t *leaf, fr_dlist_head_t *interior,
tmpl_t const *vpt) CC_HINT(nonnull);

void tmpl_extents_debug(fr_dlist_head_t *head) CC_HINT(nonnull);
void tmpl_extents_debug(FILE *fp, fr_dlist_head_t *head) CC_HINT(nonnull);

int tmpl_eval_pair(TALLOC_CTX *ctx, fr_value_box_list_t *out, request_t *request, tmpl_t const *vpt);

Expand Down
18 changes: 9 additions & 9 deletions src/lib/server/tmpl_dcursor.c
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ int tmpl_extents_build_to_leaf_parent(fr_dlist_head_t *existing, fr_dlist_head_t
return 0;
}

void tmpl_extents_debug(fr_dlist_head_t *head)
void tmpl_extents_debug(FILE *fp, fr_dlist_head_t *head)
{
tmpl_attr_extent_t const *extent = NULL;
fr_pair_t *vp = NULL;
Expand All @@ -754,25 +754,25 @@ void tmpl_extents_debug(fr_dlist_head_t *head)
char const *ctx_name;

if (ar) {
FR_FAULT_LOG("extent-interior-attr");
tmpl_attr_ref_debug(extent->ar, 0);
fprintf(fp, "extent-interior-attr\n");
tmpl_attr_ref_debug(fp, extent->ar, 0);
} else {
FR_FAULT_LOG("extent-leaf");
fprintf(fp, "extent-leaf\n");
}

ctx_name = talloc_get_name(extent->list_ctx);
if (strcmp(ctx_name, "fr_pair_t") == 0) {
FR_FAULT_LOG("list_ctx : %p (%s, %s)", extent->list_ctx, ctx_name,
fprintf(fp, "list_ctx : %p (%s, %s)\n", extent->list_ctx, ctx_name,
((fr_pair_t *)extent->list_ctx)->da->name);
} else {
FR_FAULT_LOG("list_ctx : %p (%s)", extent->list_ctx, ctx_name);
fprintf(fp, "list_ctx : %p (%s)\n", extent->list_ctx, ctx_name);
}
FR_FAULT_LOG("list : %p", extent->list);
fprintf(fp, "list : %p", extent->list);
if (fr_pair_list_empty(extent->list)) {
FR_FAULT_LOG("list (first) : none (%p)", extent->list);
fprintf(fp, "list (first) : none (%p)\n", extent->list);
} else {
vp = fr_pair_list_head(extent->list);
FR_FAULT_LOG("list (first) : %s (%p)", vp->da->name, extent->list);
fprintf(fp, "list (first) : %s (%p)\n", vp->da->name, extent->list);
}
}

Expand Down
Loading

0 comments on commit e896b90

Please sign in to comment.