Patch: FYI: don't 'return' from TRY_CATCH

Patch: FYI: don't 'return' from TRY_CATCH

[Tom Tromey]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

I'm checking this in.

It is not ok to 'return' from inside a TRY_CATCH.  This messes up the
cleanups and will lead to problems later.

I found this problem using the attached script.  It is simplistic but I
think it should catch most cases of this bug.  You can run it using:

    emacs --script try-catch.el /path/to/src/gdb

Tom


[-- Attachment #2: try-catch.el --]
[-- Type: text/plain, Size: 1341 bytes --]

;;; Look for malformed TRY_CATCH

(defconst dir-to-scan (pop argv))

(defun scan-try-catch ()
  (unless (file-directory-p dir-to-scan)
    (error "Usage: emacs --script try-catch.el DIR"))

  ;; Lame subdir list.
  (dolist (subdir '("." "cli" "mi" "python" "tui"))
    (dolist (file (directory-files (expand-file-name subdir dir-to-scan)
				   t "[.][chy]$"))
      (save-excursion
	(find-file file)
	(goto-char (point-min))
	;; (message "Processing %s" file)
	;; Skip #define TRY_CATCH ..
	(while (re-search-forward "^[^#].*\\_<TRY_CATCH\\_>" nil 'move)
	  (let ((here (point)))
	    (skip-chars-forward " \t\n")
	    (if (not (looking-at "("))
		;; Probably a comment -- skip it.
		nil
	      (forward-sexp)
	      (skip-chars-forward " \t\n")
	      ;; It is hard to handle non-braced bodies in elisp.
	      (if (not (looking-at "{"))
		  (message "%s:%d: non-conforming TRY_CATCH"
			   file
			   (line-number-at-pos here))
		(let ((end (save-excursion
			     (forward-sexp)
			     (point))))
		  ;; Note that there's no need to scan nested
		  ;; TRY_CATCHes.
		  (while (re-search-forward "\\_<\\(goto\\|return\\)\\_>"
					    end 'move)
		    (message "%s:%d: %s in TRY_CATCH"
			     file
			     (line-number-at-pos (point))
			     (match-string 1))))))))
	(kill-buffer)))))

(byte-compile 'scan-try-catch)

(scan-try-catch)

[-- Attachment #3: Type: text/plain, Size: 1044 bytes --]


2009-08-24  Tom Tromey  <tromey@redhat.com>

	* python/python-value.c (valpy_richcompare): Don't return from
	inside a TRY_CATCH.

Index: python/python-value.c
===================================================================
RCS file: /cvs/src/src/gdb/python/python-value.c,v
retrieving revision 1.26
diff -u -r1.26 python-value.c
--- python/python-value.c	14 Aug 2009 00:32:33 -0000	1.26
+++ python/python-value.c	24 Aug 2009 18:50:31 -0000
@@ -693,7 +693,10 @@
     {
       value_other = convert_value_from_python (other);
       if (value_other == NULL)
-	return NULL;
+	{
+	  result = -1;
+	  break;
+	}
 
       switch (op) {
         case Py_LT:
@@ -720,11 +723,16 @@
 	  /* Can't happen.  */
 	  PyErr_SetString (PyExc_NotImplementedError,
 			   "Invalid operation on gdb.Value.");
-	  return NULL;
+	  result = -1;
+	  break;
       }
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
+  /* In this case, the Python exception has already been set.  */
+  if (result < 0)
+    return NULL;
+
   if (result == 1)
     Py_RETURN_TRUE;
 

Re: Patch: FYI: don't 'return' from TRY_CATCH

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Mon, 24 Aug 2009 12:54:22 -0600
> 
> I found this problem using the attached script.  It is simplistic but I
> think it should catch most cases of this bug.  You can run it using:
> 
>     emacs --script try-catch.el /path/to/src/gdb

Maybe we should have a special directory in the repository for this
stuff, which will not be tarred into the release tarballs.