From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25355 invoked by alias); 31 Dec 2008 18:26:29 -0000 Received: (qmail 25346 invoked by uid 22791); 31 Dec 2008 18:26:28 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 31 Dec 2008 18:25:38 +0000 Received: (qmail 12724 invoked from network); 31 Dec 2008 18:25:36 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 31 Dec 2008 18:25:36 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: PR9681 - gdb lets you set watchpoints on nonexistent struct members Date: Wed, 31 Dec 2008 18:26:00 -0000 User-Agent: KMail/1.9.10 Cc: Daniel Jacobowitz , pmaydell@chiark.greenend.org.uk MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_kk7WJHIM5FoLkGs" Message-Id: <200812311825.40733.pedro@codesourcery.com> 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: 2008-12/txt/msg00473.txt.bz2 --Boundary-00=_kk7WJHIM5FoLkGs Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Content-length: 1494 PR9681 is about a regression that makes gdb now let you set watchpoints on nonexistent struct members. =46rom the report: This snapshot of gdb will happily allow you to put watchpoints on structure members which don't exist: (gdb) watch myfoo.nosuchmember Watchpoint 2: myfoo.nosuchmember This bug was introduced with the support to setting watchpoints in inaccessible memory, back around February/March: http://sourceware.org/ml/gdb-patches/2008-02/msg00472.html The issue is that the watchpoint command is considering all errors to be of type inaccessible-memory, due to the usage of gdb_evaluate_expression. Prior to that change, we used evaluate_expression, which did let exceptions probagate. In this particular case of a nonexistent member, it's at evaluation time that the member is found to be non-existent, although but it's at parse time that we find that myfoo does or not exist, so 'watch mybar' didn't regress. The root issue here, is that we're interested in letting watchpoints the evaluate to inaccessible memory to still succeed in being created. All other errors should be treated as real errors. So, this patch adds a new MEMORY_ERROR exception/error type --- this way we can more accuratelly filter the error type we want to ignore. There may be places throwing a generic error instead of using memory_error, I didn't do a general audit, but those would better be fixed anyway. No regressions on x86_64-linux, new test included. Ok? --=20 Pedro Alves --Boundary-00=_kk7WJHIM5FoLkGs Content-Type: text/x-diff; charset="iso 8859-15"; name="pr9681.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pr9681.diff" Content-length: 5249 2008-12-31 Pedro Alves PR breakpoints/9681: * exceptions.h (enum errors): New error type, MEMORY_ERROR. * corefile.c (memory_error): Rewrite to throw a MEMORY_ERROR. * breakpoint.c (fetch_watchpoint_value): Ignore MEMORY_ERRORs, but retrow all other exceptions. 2008-12-31 Pedro Alves PR breakpoints/9681: * gdb.base/watchpoint.exp: Add test regression. --- gdb/breakpoint.c | 24 ++++++++++++++++++++++-- gdb/corefile.c | 28 ++++++++++------------------ gdb/exceptions.h | 3 +++ gdb/testsuite/gdb.base/watchpoint.exp | 12 ++++++++++++ 4 files changed, 47 insertions(+), 20 deletions(-) Index: src/gdb/exceptions.h =================================================================== --- src.orig/gdb/exceptions.h 2008-12-31 17:59:21.000000000 +0000 +++ src/gdb/exceptions.h 2008-12-31 18:03:18.000000000 +0000 @@ -72,6 +72,9 @@ enum errors { /* Problem parsing an XML document. */ XML_PARSE_ERROR, + /* Error accessing memory. */ + MEMORY_ERROR, + /* Add more errors here. */ NR_ERRORS }; Index: src/gdb/corefile.c =================================================================== --- src.orig/gdb/corefile.c 2008-12-31 17:59:21.000000000 +0000 +++ src/gdb/corefile.c 2008-12-31 18:03:18.000000000 +0000 @@ -205,30 +205,22 @@ Use the \"file\" or \"exec-file\" comman } -/* Report a memory error with error(). */ +/* Report a memory error by throwing a MEMORY_ERROR error. */ void memory_error (int status, CORE_ADDR memaddr) { - struct ui_file *tmp_stream = mem_fileopen (); - make_cleanup_ui_file_delete (tmp_stream); - if (status == EIO) - { - /* Actually, address between memaddr and memaddr + len - was out of bounds. */ - fprintf_unfiltered (tmp_stream, "Cannot access memory at address "); - fputs_filtered (paddress (memaddr), tmp_stream); - } + /* Actually, address between memaddr and memaddr + len was out of + bounds. */ + throw_error (MEMORY_ERROR, + _("Cannot access memory at address %s"), + paddress (memaddr)); else - { - fprintf_filtered (tmp_stream, "Error accessing memory address "); - fputs_filtered (paddress (memaddr), tmp_stream); - fprintf_filtered (tmp_stream, ": %s.", - safe_strerror (status)); - } - - error_stream (tmp_stream); + throw_error (MEMORY_ERROR, + _("Error accessing memory address %s: %s."), + paddress (memaddr), + safe_strerror (status)); } /* Same as target_read_memory, but report an error if can't read. */ Index: src/gdb/breakpoint.c =================================================================== --- src.orig/gdb/breakpoint.c 2008-12-31 17:59:21.000000000 +0000 +++ src/gdb/breakpoint.c 2008-12-31 18:03:18.000000000 +0000 @@ -755,7 +755,7 @@ is_hardware_watchpoint (struct breakpoin in *VAL_CHAIN. RESULTP and VAL_CHAIN may be NULL if the caller does not need them. - If an error occurs while evaluating the expression, *RESULTP will + If a memory error occurs while evaluating the expression, *RESULTP will be set to NULL. *RESULTP may be a lazy value, if the result could not be read from memory. It is used to determine whether a value is user-specified (we should watch the whole value) or intermediate @@ -776,6 +776,7 @@ fetch_watchpoint_value (struct expressio struct value **resultp, struct value **val_chain) { struct value *mark, *new_mark, *result; + volatile struct gdb_exception ex; *valp = NULL; if (resultp) @@ -786,7 +787,26 @@ fetch_watchpoint_value (struct expressio /* Evaluate the expression. */ mark = value_mark (); result = NULL; - gdb_evaluate_expression (exp, &result); + + TRY_CATCH (ex, RETURN_MASK_ALL) + { + result = evaluate_expression (exp); + } + if (ex.reason < 0) + { + /* Ignore memory errors, we want watchpoints pointing at + inaccessible memory to still be created; otherwise, throw the + error to some higher catcher. */ + switch (ex.error) + { + case MEMORY_ERROR: + break; + default: + throw_exception (ex); + break; + } + } + new_mark = value_mark (); if (mark == new_mark) return; Index: src/gdb/testsuite/gdb.base/watchpoint.exp =================================================================== --- src.orig/gdb/testsuite/gdb.base/watchpoint.exp 2008-12-31 18:03:17.000000000 +0000 +++ src/gdb/testsuite/gdb.base/watchpoint.exp 2008-12-31 18:22:50.000000000 +0000 @@ -649,6 +649,18 @@ proc test_inaccessible_watchpoint {} { # valid) memory. if [runto func4] then { + # Make sure we only allow memory access errors. + set msg "watchpoint refused to insert on nonexistent struct member" + gdb_test_multiple "watch struct1.nosuchmember" "watchpoint in nonexisting struct member" { + -re ".*atchpoint \[0-9\]+: struct1.nosuchmember.*$gdb_prompt $" { + # PR breakpoints/9681 + fail $msg + } + -re "There is no member named nosuchmember\\..*$gdb_prompt $" { + pass $msg + } + } + gdb_test "watch *global_ptr" ".*atchpoint \[0-9\]+: \\*global_ptr" gdb_test "next" ".*global_ptr = buf.*" gdb_test_multiple "next" "next over ptr init" { --Boundary-00=_kk7WJHIM5FoLkGs--