From 4895f384b47628c8c354dccbb0bfab45b8c33984 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 21 Aug 2018 16:48:30 +0100 Subject: [PATCH] Don't throw Scheme exceptions with live std::vector objects A complication with the Guile code is that we have two types of exceptions to consider: GDB/C++ exceptions, and Guile/SJLJ exceptions. Because Guile exceptions are SJLJ based, we must make sure to not have live local variables of types with non-trivial dtors when a Guile exception is thrown, because the dtors won't be run when a Guile exceptions is thrown. gdbscm_parse_function_args currently violates this: void gdbscm_parse_function_args (const char *func_name, int beginning_arg_pos, const SCM *keywords, const char *format, ...) { ... /* Keep track of malloc'd strings. We need to free them upon error. */ std::vector allocated_strings; ... for (char *ptr : allocated_strings) xfree (ptr); gdbscm_throw (status); /// dtor of "allocated_strings" is not run! } This commit fixes the above making using of gdbscm_wrap. It would be nice if we had a way to make it impossible to write such code. PR guile/23429 has an idea for that, if someone's interested. gdb/ChangeLog: 2018-08-21 Pedro Alves * guile/scm-utils.c (gdbscm_parse_function_args_1): New, factored out from gdbscm_parse_function_args. (gdbscm_parse_function_args): Rework to use gdbscm_wrap and gdbscm_parse_function_args_1. --- gdb/ChangeLog | 7 ++ gdb/guile/scm-utils.c | 175 +++++++++++++++++++++++------------------- 2 files changed, 105 insertions(+), 77 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 9173bf9461..96bf87b8a2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2018-08-21 Pedro Alves + + * guile/scm-utils.c (gdbscm_parse_function_args_1): New, factored + out from gdbscm_parse_function_args. + (gdbscm_parse_function_args): Rework to use gdbscm_wrap and + gdbscm_parse_function_args_1. + 2018-08-21 Simon Marchi PR gdb/17816 diff --git a/gdb/guile/scm-utils.c b/gdb/guile/scm-utils.c index c24ff20483..c514c829e6 100644 --- a/gdb/guile/scm-utils.c +++ b/gdb/guile/scm-utils.c @@ -306,80 +306,18 @@ lookup_keyword (const SCM *keyword_list, SCM keyword) return -1; } -/* Utility to parse required, optional, and keyword arguments to Scheme - functions. Modelled on PyArg_ParseTupleAndKeywords, but no attempt is made - at similarity or functionality. - There is no result, if there's an error a Scheme exception is thrown. - - Guile provides scm_c_bind_keyword_arguments, and feel free to use it. - This is for times when we want a bit more parsing. - - BEGINNING_ARG_POS is the position of the first argument passed to this - routine. It should be one of the SCM_ARGn values. It could be > SCM_ARG1 - if the caller chooses not to parse one or more required arguments. - - KEYWORDS may be NULL if there are no keywords. - - FORMAT: - s - string -> char *, malloc'd - t - boolean (gdb uses "t", for biT?) -> int - i - int - u - unsigned int - l - long - n - unsigned long - L - longest - U - unsigned longest - O - random scheme object - | - indicates the next set is for optional arguments - # - indicates the next set is for keyword arguments (must follow |) - . - indicates "rest" arguments are present, this character must appear last - - FORMAT must match the definition from scm_c_{make,define}_gsubr. - Required and optional arguments appear in order in the format string. - Afterwards, keyword-based arguments are processed. There must be as many - remaining characters in the format string as their are keywords. - Except for "|#.", the number of characters in the format string must match - #required + #optional + #keywords. - - The function is required to be defined in a compatible manner: - #required-args and #optional-arguments must match, and rest-arguments - must be specified if keyword args are desired, and/or regular "rest" args. - - Example: For this function, - scm_c_define_gsubr ("execute", 2, 3, 1, foo); - the format string + keyword list could be any of: - 1) "ss|ttt#tt", { "key1", "key2", NULL } - 2) "ss|ttt.", { NULL } - 3) "ss|ttt#t.", { "key1", NULL } - - For required and optional args pass the SCM of the argument, and a - pointer to the value to hold the parsed result (type depends on format - char). After that pass the SCM containing the "rest" arguments followed - by pointers to values to hold parsed keyword arguments, and if specified - a pointer to hold the remaining contents of "rest". - - For keyword arguments pass two pointers: the first is a pointer to an int - that will contain the position of the argument in the arg list, and the - second will contain result of processing the argument. The int pointed - to by the first value should be initialized to -1. It can then be used - to tell whether the keyword was present. - - If both keyword and rest arguments are present, the caller must pass a - pointer to contain the new value of rest (after keyword args have been - removed). - There's currently no way, that I know of, to specify default values for - optional arguments in C-provided functions. At the moment they're a - work-in-progress. The caller should test SCM_UNBNDP for each optional - argument. Unbound optional arguments are ignored. */ +/* Helper for gdbscm_parse_function_args that does most of the work, + in a separate function wrapped with gdbscm_wrap so that we can use + non-trivial-dtor objects here. The result is #f upon success or a + object otherwise. */ -void -gdbscm_parse_function_args (const char *func_name, - int beginning_arg_pos, - const SCM *keywords, - const char *format, ...) +static SCM +gdbscm_parse_function_args_1 (const char *func_name, + int beginning_arg_pos, + const SCM *keywords, + const char *format, va_list args) { - va_list args; const char *p; int i, have_rest, num_keywords, position; int have_optional = 0; @@ -391,8 +329,6 @@ gdbscm_parse_function_args (const char *func_name, have_rest = validate_arg_format (format); num_keywords = count_keywords (keywords); - va_start (args, format); - p = format; position = beginning_arg_pos; @@ -511,15 +447,100 @@ gdbscm_parse_function_args (const char *func_name, } } - va_end (args); - return; + /* Return anything not-an-exception. */ + return SCM_BOOL_F; fail: - va_end (args); for (char *ptr : allocated_strings) xfree (ptr); - gdbscm_throw (status); + + /* Return the exception, which gdbscm_wrap takes care of + throwing. */ + return status; } + +/* Utility to parse required, optional, and keyword arguments to Scheme + functions. Modelled on PyArg_ParseTupleAndKeywords, but no attempt is made + at similarity or functionality. + There is no result, if there's an error a Scheme exception is thrown. + + Guile provides scm_c_bind_keyword_arguments, and feel free to use it. + This is for times when we want a bit more parsing. + + BEGINNING_ARG_POS is the position of the first argument passed to this + routine. It should be one of the SCM_ARGn values. It could be > SCM_ARG1 + if the caller chooses not to parse one or more required arguments. + + KEYWORDS may be NULL if there are no keywords. + + FORMAT: + s - string -> char *, malloc'd + t - boolean (gdb uses "t", for biT?) -> int + i - int + u - unsigned int + l - long + n - unsigned long + L - longest + U - unsigned longest + O - random scheme object + | - indicates the next set is for optional arguments + # - indicates the next set is for keyword arguments (must follow |) + . - indicates "rest" arguments are present, this character must appear last + + FORMAT must match the definition from scm_c_{make,define}_gsubr. + Required and optional arguments appear in order in the format string. + Afterwards, keyword-based arguments are processed. There must be as many + remaining characters in the format string as their are keywords. + Except for "|#.", the number of characters in the format string must match + #required + #optional + #keywords. + + The function is required to be defined in a compatible manner: + #required-args and #optional-arguments must match, and rest-arguments + must be specified if keyword args are desired, and/or regular "rest" args. + + Example: For this function, + scm_c_define_gsubr ("execute", 2, 3, 1, foo); + the format string + keyword list could be any of: + 1) "ss|ttt#tt", { "key1", "key2", NULL } + 2) "ss|ttt.", { NULL } + 3) "ss|ttt#t.", { "key1", NULL } + + For required and optional args pass the SCM of the argument, and a + pointer to the value to hold the parsed result (type depends on format + char). After that pass the SCM containing the "rest" arguments followed + by pointers to values to hold parsed keyword arguments, and if specified + a pointer to hold the remaining contents of "rest". + + For keyword arguments pass two pointers: the first is a pointer to an int + that will contain the position of the argument in the arg list, and the + second will contain result of processing the argument. The int pointed + to by the first value should be initialized to -1. It can then be used + to tell whether the keyword was present. + + If both keyword and rest arguments are present, the caller must pass a + pointer to contain the new value of rest (after keyword args have been + removed). + + There's currently no way, that I know of, to specify default values for + optional arguments in C-provided functions. At the moment they're a + work-in-progress. The caller should test SCM_UNBNDP for each optional + argument. Unbound optional arguments are ignored. */ + +void +gdbscm_parse_function_args (const char *func_name, + int beginning_arg_pos, + const SCM *keywords, + const char *format, ...) +{ + va_list args; + va_start (args, format); + + gdbscm_wrap (gdbscm_parse_function_args_1, func_name, + beginning_arg_pos, keywords, format, args); + + va_end (args); +} + /* Return longest L as a scheme object. */ -- 2.34.1