Skip to content

bpo-38965: Fix stuck in test_stack_overflow tests.#17462

Closed
marxin wants to merge 1 commit into
python:masterfrom
marxin:gcc10-fix-stack-overflow-test
Closed

bpo-38965: Fix stuck in test_stack_overflow tests.#17462
marxin wants to merge 1 commit into
python:masterfrom
marxin:gcc10-fix-stack-overflow-test

Conversation

@marxin

@marxin marxin commented Dec 4, 2019

Copy link
Copy Markdown

After update to GCC10, the testcase is stuck and
a pragma needs to be added.

https://bugs.python.org/issue38965

@marxin marxin force-pushed the gcc10-fix-stack-overflow-test branch from 7bee273 to 38206ab Compare December 4, 2019 08:34
@marxin marxin changed the title Fix stuck in test_stack_overflow tests. bpo-38965: Fix stuck in test_stack_overflow tests. Dec 4, 2019
After update to GCC10, the testcase is stuck and
a pragma needs to be added.
@marxin marxin force-pushed the gcc10-fix-stack-overflow-test branch from 38206ab to 78a2825 Compare December 4, 2019 08:37

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to modify stack_overflow() so GCC couldn't turn the tail call into a loop?

@marxin

marxin commented Dec 4, 2019

Copy link
Copy Markdown
Author

Would it be possible to modify stack_overflow() so GCC couldn't turn the tail call into a loop?

Well, based on the discussion for https://bugs.python.org/issue23654
I would do the same for GCC, just disable optimizations.

@marxin

marxin commented Dec 4, 2019

Copy link
Copy Markdown
Author

An alternative fix would be someting like:

diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c
index d1280532ae..7a48a39be4 100644
--- a/Modules/faulthandler.c
+++ b/Modules/faulthandler.c
@@ -1167,6 +1167,9 @@ faulthandler_fatal_error_py(PyObject *self, PyObject *args)
     * a loop. */
 #  pragma intel optimization_level 0
 #endif
+
+unsigned char *buffer_ptr;
+
 static
 uintptr_t
 stack_overflow(uintptr_t min_sp, uintptr_t max_sp, size_t *depth)
@@ -1179,6 +1182,7 @@ stack_overflow(uintptr_t min_sp, uintptr_t max_sp, size_t *depth)
         return sp;
     buffer[0] = 1;
     buffer[4095] = 0;
+    buffer_ptr = &buffer[0];
     return stack_overflow(min_sp, max_sp, depth);
 }

@vstinner What do you prefer?

@vstinner

vstinner commented Dec 4, 2019

Copy link
Copy Markdown
Member

If "+ buffer_ptr = &buffer[0];" prevents compiler specific pragma and fix the bug, yes: I prefer code working on any compiler ;-)

@vstinner

vstinner commented Dec 4, 2019

Copy link
Copy Markdown
Member

I proposed PR #17467 which is a different approach. @marxin: would you be able to test it on GCC 10?

@marxin

marxin commented Dec 4, 2019

Copy link
Copy Markdown
Author

I proposed PR #17467 which is a different approach. @marxin: would you be able to test it on GCC 10?

Yes, I can confirm that it works fine with GCC 10. That means the function recurses and one will reach a stack overflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants