Subject: bug#70182: [PATCH] Fix + ert for segmentation fault in get-variable-watchers (1 of 9)

From: Robert Burks <[email-masked]>
Date: Thu, 4 Apr 2024 04:44:22 -0400

(1 of 9)
** All patches in this and following emails have been re-based and tested on
   7a41de3d515 04/03/2024 **

The following segmentation faults are repeatable on version 27.1 that is
obtained through debian apt as well the current 30.0.50 master
branch (I pull and rebuild regularly).

I have attached a patch which includes a fix and ert.
(Please note the BUG# placeholder in six(6) places will need to be updated.)
I have rebased on master then rebuilt and tested the results.
The function that caused the problem had no tests previously.

The c DEFUN get_variable_watchers used the function SYMBOL_TRAPPED_WRITE_P
without guarding its input.  This inline function is an interface to
the XSYMBOL function for accessing symbol structure data and has no
provision for type checking its inputs.  XSYMBOL changed between
27.1 and 30.0.50 (2128cd8c08d). So while they will both crash with numbers
or lists a string or a variable with a string value will not crash the later
version, though it wrongly returns nil.  However, the XSYMBOL function is
not
the cause of the issue, it merely concealed the issue in some situations.
The
patch included fixes the issue in get_variable_watchers going back much
further.
***Update, builds from the end of January on now also seg fault.

The other functions in the variable-watcher family do not suffer the
same flaw as they catch invalid inputs with CHECK_SYMBOL or rely on the
error checking in Fget.

Also, while simply putting CHECK_SYMBOL at the top would have solved this
one
problem, there is a reasoning behind me moving Findirect_variable to the
top as
well. One, is to make it more like the other functions in it's family.
Second, is
that it is actually required to fix other bugs that I discovered and will
layout
over the course of several bug reports, associated patches, and ert's.

Bug Recreation Follows------------------------------------------------

WARNING: This crash requires just the 1 command below.
         Do not repeat in an Emacs session you are not
         willing to lose immediately. (run under gdb control ideally)

Session 1-------------------------------------------------------------

emacs -Q
(get-variable-watchers load-path) ;; opps forgot the apostrophe....

Fatal error 11: Segmentation fault

Session 2-------------------------------------------------------------
Update *****These Bugs Now All Crash in 30.0.50 after the end of
January******

emacs -Q
(get-variable-watchers nil)
nil
(defvar test-str "test")
test-str
(get-variable-watchers test-str)    ;; no apostrophe, value = string
nil                                 ;; no error?, no crash at least (this
crashes 27.1)
(get-variable-watchers 'test-str)
nil                                 ;; expected result
(get-variable-watchers "test")
nil                                 ;; no error?, no crash (this crashes
27.1)
(defvar test-num 5)                 ;; let's try a number with watcher
test-num
(add-variable-watcher 'test-num (lambda () nil))
nil
(get-variable-watchers 'test-num)
((closure (t) nil nil))             ;; expected result
(get-variable-watchers "test-num")  ;; string = symbol name
nil                                 ;; no error?, no crash (this crashes
27.1)
(get-variable-watchers test-num)    ;; no apostrophe, value = number
---seg fault---                     ;;crash

Session 3-------------------------------------------------------------

;; A number as parameter also crashes.
;; It also happens in batch mode.

>emacs --batch --eval "(get-variable-watchers 5)"
Fatal error 11: Segmentation fault

Other Crashes --------------------------------------------------------

;; Session 1 was a symbol with a list of strings as it's value.
;; So I also tried some direct lists.

(get-variable-watchers '(1 2 3))
---seg fault---

(get-variable-watchers '("test" "test"))
---seg fault---