gh-145792: Fix incorrect alloca allocation size in traceback.c#145814
gh-145792: Fix incorrect alloca allocation size in traceback.c#145814VanshAgarwal24036 wants to merge 4 commits intopython:mainfrom
Conversation
|
@vstinner Please review it when you are free. |
Lib/test/test_traceback.py
Outdated
| try: | ||
| recurse(50) | ||
| except RuntimeError as exc: | ||
| tb = traceback.format_exception(exc) |
There was a problem hiding this comment.
The test is unrelated to the fix, I suggest removing it. I tried to write a test using faulthandler.dump_c_stack() but I don't know how to create a long C stack. The Python recurse() reuses the same _PyEval_EvalFrameDefault frame for the 50 calls.
There was a problem hiding this comment.
I don't think there's a reliable test we can write for this at the moment. The code path in question is only triggered on compilers that don't support VLAs, which is likely only MSVC on our buildbots. But, since Windows doesn't support backtrace(), C stack dumps are disabled on MSVC anyway.
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
In the future, please give the issue author a chance to submit a PR before sending one.
Lib/test/test_traceback.py
Outdated
| try: | ||
| recurse(50) | ||
| except RuntimeError as exc: | ||
| tb = traceback.format_exception(exc) |
There was a problem hiding this comment.
I don't think there's a reliable test we can write for this at the moment. The code path in question is only triggered on compilers that don't support VLAs, which is likely only MSVC on our buildbots. But, since Windows doesn't support backtrace(), C stack dumps are disabled on MSVC anyway.
Python/traceback.c
Outdated
| #if defined(__STDC_NO_VLA__) && (__STDC_NO_VLA__ == 1) | ||
| /* Use alloca() for VLAs. */ | ||
| # define VLA(type, name, size) type *name = alloca(size) | ||
| # define VLA(type, name, size) type *name = (type *)alloca(sizeof(type) * (size)) |
There was a problem hiding this comment.
Please remove the (type *) change. In C, void * pointers can be implicitly casted.
Lib/test/test_traceback.py
Outdated
| def test_traceback_deep_recursion_alloca(self): | ||
|
|
||
| def recurse(n): | ||
| if n == 0: | ||
| raise RuntimeError("boom") | ||
| return recurse(n - 1) | ||
| try: | ||
| recurse(50) | ||
| except RuntimeError as exc: |
| Fix incorrect memory allocation in the VLA fallback macro in traceback.c | ||
| when using alloca(), preventing potential out-of-bounds access. |
There was a problem hiding this comment.
This is too technical. We should say something like "Fix out-of-bounds access when invoking faulthandler on a CPython build compiled without support for VLAs."
5 participants
Fix incorrect memory allocation when using the VLA fallback macro in traceback.c with alloca(). The previous implementation allocated only size bytes instead of sizeof(type) * size, which could lead to out-of-bounds access.
A regression test and NEWS entry are included.
traceback.cwhenalloca()is used over VLA #145792