From: Pedro Alves Date: Fri, 6 Nov 2020 17:19:02 +0000 (+0000) Subject: Split macro_buffer in two classes, fix Clang build X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ac3d4064096ecd4b012e5db91b1530df5cb4a7cc;p=binutils-gdb.git Split macro_buffer in two classes, fix Clang build GDB currently fails to build with (at least) Clang 10 and 11, due to: $ make CXX macroexp.o ../../src/gdb/macroexp.c:125:3: error: definition of implicit copy constructor for 'macro_buffer' is deprecated because it has a user-declared destructor [-Werror,-Wdeprecated-copy-dtor] ~macro_buffer () ^ Now, we could just add the copy constructor, like we already have a copy assignment operator. And like that assignment operator, we would assert that only shared buffers can be copied from. However, it is hard to see why only shared buffers need to be copied. I mean, it must be true, otherwise macro support would be broken, since currently GDB is relying on the default implementation of the copy constructor, which just copies the fields, which can't work correctly for the non-shared version. Still, it's not easy to tell from the code that that is indeed correct, that there isn't some corner case that would require copying a non-shared buffer. Or to put it simply - the tangling of shared and non-shared buffers in the same macro_buffer struct makes this structure hard to understand. My reaction was -- try splitting the macro_buffer class into two classes, one for non-shared buffers, and another for shared buffers. Comments and asserts like these: ... SRC must be a shared buffer; DEST must not be one. */ static void scan (struct macro_buffer *dest, struct macro_buffer *src, struct macro_name_list *no_loop, const macro_scope &scope) { gdb_assert (src->shared); gdb_assert (! dest->shared); ... made me suspect it should be possible. Then after the split it should be easier to reimplement either of the classes if we want. So I decided to try splitting the struct in two distinct types, and see where that leads. It turns out that there is really no good reason for a single struct, no code that wants to work with either shared or non-shared buffers. It's always shared for input being parsed, and non-shared for output. This commit is the result. I named the new classes shared_macro_buffer and growable_macro_buffer. A future direction could be for example to make shared_macro_buffer wrap a string_view and growable_macro_buffer a std::string. With that in mind, other than text/len, only the 'last_token' field is common to both classes. I didn't feel like creating a base class just for that single field. I constified shared_macro_buffer's 'text' field, which of course had some knock-on effects, fixed in the patch. On the original warning issued by Clang -- now it is clear that only the shared version needs to be copied. Since this class doesn't need a user-declared destructor, the default implementations of the copy assign/ctor can be used, and Clang no longer warns. The growable version doesn't need to be copied, so I disabled copy/assign for it. gdb/ChangeLog: * macroexp.c (struct macro_buffer): Split in two classes. Add uses adjusted. (struct shared_macro_buffer): New, factored out from struct macro_buffer. (struct growable_macro_buffer): New, factored out from struct macro_buffer. (set_token, get_comment, get_identifier, get_pp_number) (get_character_constant, get_string_literal, get_punctuator) (get_next_token_for_substitution): Constify parameters. (substitute_args): Constify locals. Change-Id: I5712e30e826d949715703b2e9172adf04e63b152 --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 8a05f20ca90..b846426f89e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2020-11-06 Pedro Alves + + * macroexp.c (struct macro_buffer): Split in two classes. Add + uses adjusted. + (struct shared_macro_buffer): New, factored out from struct + macro_buffer. + (struct growable_macro_buffer): New, factored out from struct + macro_buffer. + (set_token, get_comment, get_identifier, get_pp_number) + (get_character_constant, get_string_literal, get_punctuator) + (get_next_token_for_substitution): Constify parameters. + (substitute_args): Constify locals. + 2020-11-05 Tom Tromey * dwarf2/read.c (read_cutu_die_from_dwo) diff --git a/gdb/macroexp.c b/gdb/macroexp.c index 68fb1967161..9b15af36735 100644 --- a/gdb/macroexp.c +++ b/gdb/macroexp.c @@ -26,37 +26,20 @@ -/* A resizeable, substringable string type. */ +/* A string type that we can use to refer to substrings of other + strings. */ -/* A string type that we can resize, quickly append to, and use to - refer to substrings of other strings. */ -struct macro_buffer +struct shared_macro_buffer { - /* An array of characters. The first LEN bytes are the real text, - but there are SIZE bytes allocated to the array. If SIZE is - zero, then this doesn't point to a malloc'ed block. If SHARED is - non-zero, then this buffer is actually a pointer into some larger - string, and we shouldn't append characters to it, etc. Because - of sharing, we can't assume in general that the text is + /* An array of characters. This buffer is a pointer into some + larger string and thus we can't assume in that the text is null-terminated. */ - char *text; + const char *text; /* The number of characters in the string. */ int len; - /* The number of characters allocated to the string. If SHARED is - non-zero, this is meaningless; in this case, we set it to zero so - that any "do we have room to append something?" tests will fail, - so we don't always have to check SHARED before using this field. */ - int size; - - /* Zero if TEXT can be safely realloc'ed (i.e., it's its own malloc - block). Non-zero if TEXT is actually pointing into the middle of - some other block, or to a string literal, and we shouldn't - reallocate it. */ - bool shared; - /* For detecting token splicing. This is the index in TEXT of the first character of the token @@ -71,33 +54,15 @@ struct macro_buffer is non-zero if it is an identifier token, zero otherwise. */ int is_identifier = 0; - - macro_buffer () + shared_macro_buffer () : text (NULL), - len (0), - size (0), - shared (false) - { - } - - /* Set the macro buffer to the empty string, guessing that its - final contents will fit in N bytes. (It'll get resized if it - doesn't, so the guess doesn't have to be right.) Allocate the - initial storage with xmalloc. */ - explicit macro_buffer (int n) - : len (0), - size (n), - shared (false) + len (0) { - if (n > 0) - text = (char *) xmalloc (n); - else - text = NULL; } /* Set the macro buffer to refer to the LEN bytes at ADDR, as a shared substring. */ - macro_buffer (const char *addr, int len) + shared_macro_buffer (const char *addr, int len) { set_shared (addr, len); } @@ -106,45 +71,68 @@ struct macro_buffer shared substring. */ void set_shared (const char *addr, int len_) { - text = (char *) addr; + text = addr; len = len_; - size = 0; - shared = true; } +}; + +/* A string type that we can resize and quickly append to. */ + +struct growable_macro_buffer +{ + /* An array of characters. The first LEN bytes are the real text, + but there are SIZE bytes allocated to the array. */ + char *text; + + /* The number of characters in the string. */ + int len; + + /* The number of characters allocated to the string. */ + int size; + + /* For detecting token splicing. - macro_buffer& operator= (const macro_buffer &src) + This is the index in TEXT of the first character of the token + that abuts the end of TEXT. If TEXT contains no tokens, then we + set this equal to LEN. If TEXT ends in whitespace, then there is + no token abutting the end of TEXT (it's just whitespace), and + again, we set this equal to LEN. We set this to -1 if we don't + know the nature of TEXT. */ + int last_token = -1; + + /* Set the macro buffer to the empty string, guessing that its + final contents will fit in N bytes. (It'll get resized if it + doesn't, so the guess doesn't have to be right.) Allocate the + initial storage with xmalloc. */ + explicit growable_macro_buffer (int n) + : len (0), + size (n) { - gdb_assert (src.shared); - gdb_assert (shared); - set_shared (src.text, src.len); - last_token = src.last_token; - is_identifier = src.is_identifier; - return *this; + if (n > 0) + text = (char *) xmalloc (n); + else + text = NULL; } - ~macro_buffer () + DISABLE_COPY_AND_ASSIGN (growable_macro_buffer); + + ~growable_macro_buffer () { - if (! shared && size) - xfree (text); + xfree (text); } /* Release the text of the buffer to the caller. */ gdb::unique_xmalloc_ptr release () { - gdb_assert (! shared); gdb_assert (size); char *result = text; text = NULL; return gdb::unique_xmalloc_ptr (result); } - /* Resize the buffer to be at least N bytes long. Raise an error if - the buffer shouldn't be resized. */ + /* Resize the buffer to be at least N bytes long. */ void resize_buffer (int n) { - /* We shouldn't be trying to resize shared strings. */ - gdb_assert (! shared); - if (size == 0) size = n; else @@ -212,7 +200,7 @@ macro_is_identifier_nondigit (int c) static void -set_token (struct macro_buffer *tok, char *start, char *end) +set_token (shared_macro_buffer *tok, const char *start, const char *end) { tok->set_shared (start, end - start); tok->last_token = 0; @@ -223,14 +211,14 @@ set_token (struct macro_buffer *tok, char *start, char *end) static int -get_comment (struct macro_buffer *tok, char *p, char *end) +get_comment (shared_macro_buffer *tok, const char *p, const char *end) { if (p + 2 > end) return 0; else if (p[0] == '/' && p[1] == '*') { - char *tok_start = p; + const char *tok_start = p; p += 2; @@ -249,7 +237,7 @@ get_comment (struct macro_buffer *tok, char *p, char *end) else if (p[0] == '/' && p[1] == '/') { - char *tok_start = p; + const char *tok_start = p; p += 2; for (; p < end; p++) @@ -265,12 +253,12 @@ get_comment (struct macro_buffer *tok, char *p, char *end) static int -get_identifier (struct macro_buffer *tok, char *p, char *end) +get_identifier (shared_macro_buffer *tok, const char *p, const char *end) { if (p < end && macro_is_identifier_nondigit (*p)) { - char *tok_start = p; + const char *tok_start = p; while (p < end && (macro_is_identifier_nondigit (*p) @@ -287,7 +275,7 @@ get_identifier (struct macro_buffer *tok, char *p, char *end) static int -get_pp_number (struct macro_buffer *tok, char *p, char *end) +get_pp_number (shared_macro_buffer *tok, const char *p, const char *end) { if (p < end && (macro_is_digit (*p) @@ -295,7 +283,7 @@ get_pp_number (struct macro_buffer *tok, char *p, char *end) && p + 2 <= end && macro_is_digit (p[1])))) { - char *tok_start = p; + const char *tok_start = p; while (p < end) { @@ -326,7 +314,8 @@ get_pp_number (struct macro_buffer *tok, char *p, char *end) Signal an error if it contains a malformed or incomplete character constant. */ static int -get_character_constant (struct macro_buffer *tok, char *p, char *end) +get_character_constant (shared_macro_buffer *tok, + const char *p, const char *end) { /* ISO/IEC 9899:1999 (E) Section 6.4.4.4 paragraph 1 But of course, what really matters is that we handle it the same @@ -337,7 +326,7 @@ get_character_constant (struct macro_buffer *tok, char *p, char *end) && (p[0] == 'L' || p[0] == 'u' || p[0] == 'U') && p[1] == '\'')) { - char *tok_start = p; + const char *tok_start = p; int char_count = 0; if (*p == '\'') @@ -387,7 +376,7 @@ get_character_constant (struct macro_buffer *tok, char *p, char *end) literal, and return 1. Otherwise, return zero. Signal an error if it contains a malformed or incomplete string literal. */ static int -get_string_literal (struct macro_buffer *tok, char *p, char *end) +get_string_literal (shared_macro_buffer *tok, const char *p, const char *end) { if ((p + 1 <= end && *p == '"') @@ -395,7 +384,7 @@ get_string_literal (struct macro_buffer *tok, char *p, char *end) && (p[0] == 'L' || p[0] == 'u' || p[0] == 'U') && p[1] == '"')) { - char *tok_start = p; + const char *tok_start = p; if (*p == '"') p++; @@ -437,7 +426,7 @@ get_string_literal (struct macro_buffer *tok, char *p, char *end) static int -get_punctuator (struct macro_buffer *tok, char *p, char *end) +get_punctuator (shared_macro_buffer *tok, const char *p, const char *end) { /* Here, speed is much less important than correctness and clarity. */ @@ -493,18 +482,16 @@ get_punctuator (struct macro_buffer *tok, char *p, char *end) /* Peel the next preprocessor token off of SRC, and put it in TOK. Mutate TOK to refer to the first token in SRC, and mutate SRC to - refer to the text after that token. SRC must be a shared buffer; - the resulting TOK will be shared, pointing into the same string SRC - does. Initialize TOK's last_token field. Return non-zero if we - succeed, or 0 if we didn't find any more tokens in SRC. */ + refer to the text after that token. The resulting TOK will point + into the same string SRC does. Initialize TOK's last_token field. + Return non-zero if we succeed, or 0 if we didn't find any more + tokens in SRC. */ + static int -get_token (struct macro_buffer *tok, - struct macro_buffer *src) +get_token (shared_macro_buffer *tok, shared_macro_buffer *src) { - char *p = src->text; - char *end = p + src->len; - - gdb_assert (src->shared); + const char *p = src->text; + const char *end = p + src->len; /* From the ISO C standard, ISO/IEC 9899:1999 (E), section 6.4: @@ -583,11 +570,11 @@ get_token (struct macro_buffer *tok, fine to return with DEST set to "x y". Similarly, "<" and "<" must yield "< <", not "<<", etc. */ static void -append_tokens_without_splicing (struct macro_buffer *dest, - struct macro_buffer *src) +append_tokens_without_splicing (growable_macro_buffer *dest, + shared_macro_buffer *src) { int original_dest_len = dest->len; - struct macro_buffer dest_tail, new_token; + shared_macro_buffer dest_tail, new_token; gdb_assert (src->last_token != -1); gdb_assert (dest->last_token != -1); @@ -653,7 +640,7 @@ append_tokens_without_splicing (struct macro_buffer *dest, stringify; it is LEN bytes long. */ static void -stringify (struct macro_buffer *dest, const char *arg, int len) +stringify (growable_macro_buffer *dest, const char *arg, int len) { /* Trim initial whitespace from ARG. */ while (len > 0 && macro_is_whitespace (*arg)) @@ -702,7 +689,7 @@ gdb::unique_xmalloc_ptr macro_stringify (const char *str) { int len = strlen (str); - struct macro_buffer buffer (len); + growable_macro_buffer buffer (len); stringify (&buffer, str, len); buffer.appendc ('\0'); @@ -780,17 +767,17 @@ currently_rescanning (struct macro_name_list *list, const char *name) following the invocation. */ static bool -gather_arguments (const char *name, struct macro_buffer *src, int nargs, - std::vector *args_ptr) +gather_arguments (const char *name, shared_macro_buffer *src, int nargs, + std::vector *args_ptr) { - struct macro_buffer tok; - std::vector args; + shared_macro_buffer tok; + std::vector args; /* Does SRC start with an opening paren token? Read from a copy of SRC, so SRC itself is unaffected if we don't find an opening paren. */ { - struct macro_buffer temp (src->text, src->len); + shared_macro_buffer temp (src->text, src->len); if (! get_token (&tok, &temp) || tok.len != 1 @@ -803,7 +790,7 @@ gather_arguments (const char *name, struct macro_buffer *src, int nargs, for (;;) { - struct macro_buffer *arg; + shared_macro_buffer *arg; int depth; /* Initialize the next argument. */ @@ -874,8 +861,8 @@ gather_arguments (const char *name, struct macro_buffer *src, int nargs, /* The `expand' and `substitute_args' functions both invoke `scan' recursively, so we need a forward declaration somewhere. */ -static void scan (struct macro_buffer *dest, - struct macro_buffer *src, +static void scan (growable_macro_buffer *dest, + shared_macro_buffer *src, struct macro_name_list *no_loop, const macro_scope &scope); @@ -891,8 +878,8 @@ static void scan (struct macro_buffer *dest, index. If TOK is not an argument, return -1. */ static int -find_parameter (const struct macro_buffer *tok, - int is_varargs, const struct macro_buffer *va_arg_name, +find_parameter (const shared_macro_buffer *tok, + int is_varargs, const shared_macro_buffer *va_arg_name, int argc, const char * const *argv) { int i; @@ -916,11 +903,11 @@ find_parameter (const struct macro_buffer *tok, updates the passed-in state variables. */ static void -get_next_token_for_substitution (struct macro_buffer *replacement_list, - struct macro_buffer *token, - char **start, - struct macro_buffer *lookahead, - char **lookahead_start, +get_next_token_for_substitution (shared_macro_buffer *replacement_list, + shared_macro_buffer *token, + const char **start, + shared_macro_buffer *lookahead, + const char **lookahead_start, int *lookahead_valid, bool *keep_going) { @@ -952,27 +939,27 @@ get_next_token_for_substitution (struct macro_buffer *replacement_list, NO_LOOP. */ static void -substitute_args (struct macro_buffer *dest, +substitute_args (growable_macro_buffer *dest, struct macro_definition *def, - int is_varargs, const struct macro_buffer *va_arg_name, - const std::vector &argv, + int is_varargs, const shared_macro_buffer *va_arg_name, + const std::vector &argv, struct macro_name_list *no_loop, const macro_scope &scope) { /* The token we are currently considering. */ - struct macro_buffer tok; + shared_macro_buffer tok; /* The replacement list's pointer from just before TOK was lexed. */ - char *original_rl_start; + const char *original_rl_start; /* We have a single lookahead token to handle token splicing. */ - struct macro_buffer lookahead; + shared_macro_buffer lookahead; /* The lookahead token might not be valid. */ int lookahead_valid; /* The replacement list's pointer from just before LOOKAHEAD was lexed. */ - char *lookahead_rl_start; + const char *lookahead_rl_start; /* A macro buffer for the macro's replacement list. */ - struct macro_buffer replacement_list (def->replacement, + shared_macro_buffer replacement_list (def->replacement, strlen (def->replacement)); gdb_assert (dest->len == 0); @@ -1190,7 +1177,7 @@ substitute_args (struct macro_buffer *dest, mutates its source, so we need to scan a new buffer referring to the argument's text, not the argument itself. */ - struct macro_buffer arg_src (argv[arg].text, argv[arg].len); + shared_macro_buffer arg_src (argv[arg].text, argv[arg].len); scan (dest, &arg_src, no_loop, scope); substituted = 1; } @@ -1217,9 +1204,9 @@ substitute_args (struct macro_buffer *dest, we don't expand it.) If we return zero, leave SRC unchanged. */ static int expand (const char *id, - struct macro_definition *def, - struct macro_buffer *dest, - struct macro_buffer *src, + struct macro_definition *def, + growable_macro_buffer *dest, + shared_macro_buffer *src, struct macro_name_list *no_loop, const macro_scope &scope) { @@ -1236,7 +1223,7 @@ expand (const char *id, /* What kind of macro are we expanding? */ if (def->kind == macro_object_like) { - struct macro_buffer replacement_list (def->replacement, + shared_macro_buffer replacement_list (def->replacement, strlen (def->replacement)); scan (dest, &replacement_list, &new_no_loop, scope); @@ -1244,7 +1231,7 @@ expand (const char *id, } else if (def->kind == macro_function_like) { - struct macro_buffer va_arg_name; + shared_macro_buffer va_arg_name; int is_varargs = 0; if (def->argc >= 1) @@ -1272,7 +1259,7 @@ expand (const char *id, } } - std::vector argv; + std::vector argv; /* If we couldn't find any argument list, then we don't expand this macro. */ if (!gather_arguments (id, src, is_varargs ? def->argc : -1, @@ -1304,21 +1291,21 @@ expand (const char *id, splicing operator "##" don't get macro references expanded, so we can't really tell whether it's appropriate to macro- expand an argument until we see how it's being used. */ - struct macro_buffer substituted (0); + growable_macro_buffer substituted (0); substitute_args (&substituted, def, is_varargs, &va_arg_name, argv, no_loop, scope); /* Now `substituted' is the macro's replacement list, with all - argument values substituted into it properly. Re-scan it for + argument values substituted into it properly. Re-scan it for macro references, but don't expand invocations of this macro. We create a new buffer, `substituted_src', which points into - `substituted', and scan that. We can't scan `substituted' + `substituted', and scan that. We can't scan `substituted' itself, since the tokenization process moves the buffer's text pointer around, and we still need to be able to find `substituted's original text buffer after scanning it so we can free it. */ - struct macro_buffer substituted_src (substituted.text, substituted.len); + shared_macro_buffer substituted_src (substituted.text, substituted.len); scan (dest, &substituted_src, &new_no_loop, scope); return 1; @@ -1333,19 +1320,14 @@ expand (const char *id, expansion to DEST and return non-zero. Otherwise, return zero, and leave DEST unchanged. - SRC_FIRST and SRC_REST must be shared buffers; DEST must not be one. SRC_FIRST must be a string built by get_token. */ static int -maybe_expand (struct macro_buffer *dest, - struct macro_buffer *src_first, - struct macro_buffer *src_rest, +maybe_expand (growable_macro_buffer *dest, + shared_macro_buffer *src_first, + shared_macro_buffer *src_rest, struct macro_name_list *no_loop, const macro_scope &scope) { - gdb_assert (src_first->shared); - gdb_assert (src_rest->shared); - gdb_assert (! dest->shared); - /* Is this token an identifier? */ if (src_first->is_identifier) { @@ -1371,22 +1353,19 @@ maybe_expand (struct macro_buffer *dest, /* Expand macro references in SRC, appending the results to DEST. Assume we are re-scanning the result of expanding the macros named - in NO_LOOP, and don't try to re-expand references to them. + in NO_LOOP, and don't try to re-expand references to them. */ - SRC must be a shared buffer; DEST must not be one. */ static void -scan (struct macro_buffer *dest, - struct macro_buffer *src, +scan (growable_macro_buffer *dest, + shared_macro_buffer *src, struct macro_name_list *no_loop, const macro_scope &scope) { - gdb_assert (src->shared); - gdb_assert (! dest->shared); for (;;) { - struct macro_buffer tok; - char *original_src_start = src->text; + shared_macro_buffer tok; + const char *original_src_start = src->text; /* Find the next token in SRC. */ if (! get_token (&tok, src)) @@ -1419,9 +1398,9 @@ scan (struct macro_buffer *dest, gdb::unique_xmalloc_ptr macro_expand (const char *source, const macro_scope &scope) { - struct macro_buffer src (source, strlen (source)); + shared_macro_buffer src (source, strlen (source)); - struct macro_buffer dest (0); + growable_macro_buffer dest (0); dest.last_token = 0; scan (&dest, &src, 0, scope); @@ -1441,13 +1420,13 @@ macro_expand_once (const char *source, const macro_scope &scope) gdb::unique_xmalloc_ptr macro_expand_next (const char **lexptr, const macro_scope &scope) { - struct macro_buffer tok; + shared_macro_buffer tok; /* Set up SRC to refer to the input text, pointed to by *lexptr. */ - struct macro_buffer src (*lexptr, strlen (*lexptr)); + shared_macro_buffer src (*lexptr, strlen (*lexptr)); /* Set up DEST to receive the expansion, if there is one. */ - struct macro_buffer dest (0); + growable_macro_buffer dest (0); dest.last_token = 0; /* Get the text's first preprocessing token. */