| msg152007 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2012-01-26 13:34 |
| In non-debug mode the read_null test fails with clang-3.0: ====================================================================== FAIL: test_disable (test.test_faulthandler.FaultHandlerTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/home/stefan/hg/cpython/Lib/test/test_faulthandler.py", line 235, in test_disable self.assertNotEqual(exitcode, 0) AssertionError: 0 == 0 clang "optimizes" the undefined behavior into a simple assignment: $ ~/usr/bin/clang --version clang version 3.0 (tags/RELEASE_30/final) Target: x86_64-unknown-freebsd9.0 Thread model: posix $ $ cat read_null.c #include <stdio.h> int main(void) { int *x = NULL, y; y = *x; printf("%d\n", y); return 0; } $ $ ~/usr/bin/clang -Wall -O0 -g -o read_null read_null.c $ ./read_null Segmentation fault: 11 (core dumped) $ ~/usr/bin/clang -Wall -O3 -g -o read_null read_null.c $ ./read_null 0 |
|
|
| msg152008 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-01-26 14:02 |
| Can you try x = (int *)1; instead of x = NULL; ? |
|
|
| msg152011 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-01-26 14:32 |
| STINNER Victor <report@bugs.python.org> wrote: > Can you try x = (int *)1; instead of x = NULL; ? This works. - I must say that I find this new behavior of clang slightly dangerous... |
|
|
| msg152025 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-01-26 17:10 |
| There was a similar bug which was declared as a vulnerability in the Linux kernel: http://isc.sans.edu/diary.html?storyid=6820 GCC has an option to disable this optimization: -fno-delete-null-pointer-checks. |
|
|
| msg152101 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-01-27 16:19 |
| Oh, you can also try something else: add the volatile keyword. |
|
|
| msg152130 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-01-27 22:15 |
| Yes, volatile works, too. That's probably the best solution. |
|
|
| msg152132 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-01-27 22:17 |
| Well, to be completely unambiguous. This works: diff -r d2cf8a34ddf9 Modules/faulthandler.c --- a/Modules/faulthandler.c Thu Jan 26 00:15:07 2012 -0800 +++ b/Modules/faulthandler.c Fri Jan 27 23:16:27 2012 +0100 @@ -943,7 +943,7 @@ static PyObject * faulthandler_read_null(PyObject *self, PyObject *args) { - int *x = NULL, y; + volatile int *x = NULL, y; int release_gil = 0; if (!PyArg_ParseTuple(args, "|i:_read_null", &release_gil)) |
|
|
| msg152148 - (view) |
Author: Jesús Cea Avión (jcea) * |
Date: 2012-01-28 03:48 |
| That makes "x" and "y" volatile. |
|
|
| msg152154 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-01-28 10:35 |
| Jes??s Cea Avi??n <report@bugs.python.org> wrote: > That makes "x" and "y" volatile. Well yes, but is that a problem? |
|
|
| msg152165 - (view) |
Author: Jesús Cea Avión (jcea) * |
Date: 2012-01-28 15:37 |
| Nothing. I just read the sourcecode :-). I would add an "y=0" in the declaration too, just for aestetics. |
|
|
| msg152278 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-01-29 23:07 |
| New changeset f71249d785d6 by Victor Stinner in branch 'default': Issue #13874: read_null() of faulthandler uses volatile to avoid optimisation http://hg.python.org/cpython/rev/f71249d785d6 |
|
|
| msg152279 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-01-29 23:07 |
| Does my commit fix the issue? |
|
|
| msg152291 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-01-29 23:55 |
| STINNER Victor <report@bugs.python.org> wrote: > Does my commit fix the issue? Yes, perfectly. |
|
|