From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5114 invoked by alias); 18 Sep 2009 23:05:13 -0000 Received: (qmail 5103 invoked by uid 22791); 18 Sep 2009 23:05:12 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Sep 2009 23:05:06 +0000 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8IN55xA017934 for ; Fri, 18 Sep 2009 19:05:05 -0400 Received: from [IPv6:::1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx08.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n8IN50bD003375 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 18 Sep 2009 19:05:03 -0400 Message-ID: <4AB4121C.3060803@redhat.com> Date: Fri, 18 Sep 2009 23:05:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.1) Gecko/20090814 Fedora/3.0-2.6.b3.fc11 Lightning/1.0pre Thunderbird/3.0b3 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: [RFA] Real C++ operator validation Content-Type: multipart/mixed; boundary="------------040207080102080603070807" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-09/txt/msg00617.txt.bz2 This is a multi-part message in MIME format. --------------040207080102080603070807 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2226 Hi, C++ operators have been sorely neglected in gdb's C++ code, especially the parser and decode_line, the two biggest interfaces to the user. This patch is an attempt to mitigate the later by validating operator names. For example, one cannot break/list on a function named "operator_1", since gdb assumes that anything starting with "operator" is an operator. Originally, I set out (in CVS HEAD) to fix an annoying bug concerning an error message when the user attempted to call decode_line with an undefined operator. Turns out the fix for this is a little more complex than I was hoping, but since I am trying to get all my branch patches accepted/committed here, I thought, "What the heck?" :-) As a result, this patch from my archer-keiths-expr-cumulative branch has several (good) side-effects: 1) It properly ignores spaces in operator names. "break operator ++" is now valid. [NOTE: No single quotes!] 2) It adds the ability to list/break on conversion operators, e.g., "break foo::operator char*". [NOTE: No single quotes!] 3) (The bug I was attempting to fix) It now outputs "the class foo does not have any method named operator int*" instead of "the class foo does not have any method named operator ator". 4) It removes the assumption that functions starting with the word "operator" are really operators. If they are not, it treats them as "normal" functions. I have regression tested this on x86 linux, and there are no regressions. [I thought I would explicitly mention this. I always do this anyway.] Ok?/Comments?/Concerns? Keith PS. Shortly I'm planning to submit my "realcpp" tests which offers substantially more thorough testing of this and a lot more. Consequently I've intentionally kept the testing in this patch restricted to the bug I originally set out to fix. ChangeLog 2009-09-18 Keith Seitz * cp-support.h (cp_validate_operator): Declare new function. * cp-support.c (cp_validate_operator): New function. * linespec.c (decode_compound): For C++ check for a valid operator. testsuite/ChangeLog 2009-09-18 Keith Seitz * gdb.cp/cplusfuncs.exp (do_tests): Add check for proper error message with invalid operator. --------------040207080102080603070807 Content-Type: text/plain; name="cp_validate_operator.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="cp_validate_operator.patch" Content-length: 7563 Index: cp-support.h =================================================================== RCS file: /cvs/src/src/gdb/cp-support.h,v retrieving revision 1.29 diff -u -p -r1.29 cp-support.h --- cp-support.h 7 Jul 2009 17:25:11 -0000 1.29 +++ cp-support.h 18 Sep 2009 22:39:55 -0000 @@ -72,6 +72,8 @@ extern struct symbol **make_symbol_overl extern struct type *cp_lookup_rtti_type (const char *name, struct block *block); +extern int cp_validate_operator (const char *input); + /* Functions/variables from cp-namespace.c. */ extern int cp_is_anonymous (const char *namespace); Index: cp-support.c =================================================================== RCS file: /cvs/src/src/gdb/cp-support.c,v retrieving revision 1.33 diff -u -p -r1.33 cp-support.c --- cp-support.c 7 Jul 2009 17:25:11 -0000 1.33 +++ cp-support.c 18 Sep 2009 22:39:56 -0000 @@ -32,6 +32,9 @@ #include "block.h" #include "complaints.h" #include "gdbtypes.h" +#include "exceptions.h" +#include "expression.h" +#include "value.h" #include "safe-ctype.h" @@ -70,6 +73,18 @@ struct cmd_list_element *maint_cplus_cmd static void maint_cplus_command (char *arg, int from_tty); static void first_component_command (char *arg, int from_tty); +/* Operator validation. + NOTE: Multi-byte operators (usually the assignment variety operator) + must appear before the single byte version, i.e., "+=" before "+". */ +static const char *operator_tokens[] = + { + "++", "+=", "+", "->*", "->", "--", "-=", "-", "*=", "*", "/=", "/", + "%=", "%", "!=", "==", "!", "&&", "<<=", "<<", ">>=", ">>", + "<=", "<", ">=", ">", "~", "&=", "&", "|=", "||", "|", "^=", "^", + "=", "()", "[]", ",", "new", "delete" + /* new[] and delete[] require special whitespace handling */ + }; + /* Return 1 if STRING is clearly already in canonical form. This function is conservative; things which it does not recognize are assumed to be non-canonical, and the parser will sort them out @@ -909,6 +924,109 @@ first_component_command (char *arg, int extern initialize_file_ftype _initialize_cp_support; /* -Wmissing-prototypes */ +#define SKIP_SPACE(P) \ + do \ + { \ + while (*(P) == ' ' || *(P) == '\t') \ + ++(P); \ + } \ + while (0) + +/* Returns the length of the operator name or 0 if INPUT does not + point to a valid C++ operator. INPUT should start with "operator". */ +int +cp_validate_operator (const char *input) +{ + int i; + char *copy; + const char *p; + struct expression *expr; + struct value *val; + struct gdb_exception except; + struct cleanup *old_chain; + + p = input; + + /* Most callers appear to do this already, but since this is not + really time-critical code, it is double-checked here. */ + if (strncmp (p, "operator", 8) == 0) + { + int valid = 0; + p += 8; + + SKIP_SPACE (p); + for (i = 0; i < sizeof (operator_tokens) / sizeof (operator_tokens[0]); + ++i) + { + int length = strlen (operator_tokens[i]); + /* By using strncmp here, we MUST have operator_tokens ordered! + See additional notes where operator_tokens is defined above. */ + if (strncmp (p, operator_tokens[i], length) == 0) + { + const char *op = p; + valid = 1; + p += length; + + if (strncmp (op, "new", 3) == 0 + || strncmp (op, "delete", 6) == 0) + { + + /* Special case: new[] and delete[]. We must be careful + to swallow whitespace before/in "[]". */ + SKIP_SPACE (p); + + if (*p == '[') + { + ++p; + SKIP_SPACE (p); + if (*p == ']') + ++p; + else + valid = 0; + } + } + + if (valid) + return (p - input); + } + } + + /* Check input for a conversion operator. */ + + /* Skip past base typename */ + while (*p != '*' && *p != '&' && *p != 0 && *p != ' ') + ++p; + SKIP_SPACE (p); + + /* Add modifiers '*'/'&' */ + while (*p == '*' || *p == '&') + { + ++p; + SKIP_SPACE (p); + } + + /* Check for valid type. [Remember: input starts with + "operator".] */ + copy = savestring (input + 8, p - input - 8); + expr = NULL; + val = NULL; + TRY_CATCH (except, RETURN_MASK_ALL) + { + expr = parse_expression (copy); + val = evaluate_type (expr); + } + + xfree (copy); + if (expr) + xfree (expr); + + if (val != NULL && value_type (val) != NULL) + return (p - input); + } + + return 0; +} + void _initialize_cp_support (void) { Index: linespec.c =================================================================== RCS file: /cvs/src/src/gdb/linespec.c,v retrieving revision 1.89 diff -u -p -r1.89 linespec.c --- linespec.c 14 Aug 2009 00:32:32 -0000 1.89 +++ linespec.c 18 Sep 2009 22:39:56 -0000 @@ -30,6 +30,7 @@ #include "value.h" #include "completer.h" #include "cp-abi.h" +#include "cp-support.h" #include "parser-defs.h" #include "block.h" #include "objc-lang.h" @@ -1257,6 +1258,9 @@ decode_compound (char **argptr, int funf /* Move pointer ahead to next double-colon. */ while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')) { + if (current_language->la_language == language_cplus) + p += cp_validate_operator (p); + if (p[0] == '<') { temp_end = find_template_name_end (p); @@ -1334,6 +1338,15 @@ decode_compound (char **argptr, int funf while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':') p++; /* At this point p->"". String ended. */ + /* Nope, C++ operators could have spaces in them + ("foo::operator <" or "foo::operator delete []"). + I apologize, this is a bit hacky... */ + if (current_language->la_language == language_cplus + && *p == ' ' && p - 8 - *argptr + 1 > 0) + { + /* The above loop has already swallowed "operator". */ + p += cp_validate_operator (p - 8) - 8; + } } /* Allocate our own copy of the substring between argptr and @@ -1474,26 +1487,16 @@ find_method (int funfirstline, char ***c } else { - char *tmp; - - if (is_operator_name (copy)) - { - tmp = (char *) alloca (strlen (copy + 3) + 9); - strcpy (tmp, "operator "); - strcat (tmp, copy + 3); - } - else - tmp = copy; if (not_found_ptr) *not_found_ptr = 1; - if (tmp[0] == '~') + if (copy[0] == '~') cplusplus_error (saved_arg, "the class `%s' does not have destructor defined\n", SYMBOL_PRINT_NAME (sym_class)); else cplusplus_error (saved_arg, "the class %s does not have any method named %s\n", - SYMBOL_PRINT_NAME (sym_class), tmp); + SYMBOL_PRINT_NAME (sym_class), copy); } } Index: testsuite/gdb.cp/cplusfuncs.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/cplusfuncs.exp,v retrieving revision 1.7 diff -u -p -r1.7 cplusfuncs.exp --- testsuite/gdb.cp/cplusfuncs.exp 3 Jan 2009 05:58:04 -0000 1.7 +++ testsuite/gdb.cp/cplusfuncs.exp 18 Sep 2009 22:39:56 -0000 @@ -534,6 +534,7 @@ proc do_tests {} { global srcdir global binfile global gdb_prompt + global dm_type_int_star set prms_id 0 set bug_id 0 @@ -557,6 +558,10 @@ proc do_tests {} { test_paddr_operator_functions test_paddr_hairy_functions test_lookup_operator_functions + + # A regression test on errors involving operators + gdb_test "list foo::operator $dm_type_int_star" \ + ".*the class foo does not have any method named operator $dm_type_int_star.*" } do_tests --------------040207080102080603070807--