fix: reference leaks and incorrect C-API usage in _stacktrace.c [backport 2.0] (#7148)

Backport ac10377 from #7112 to 2.0.

I was just reading this file and found many problems and leaks so I
thought that a PR may be welcomed 😉

There are several problems and possible improvements with
"_stacktrace.c":

* The macros for 3.11+ have some nested calls such as:

#define GET_FILENAME(frame)
PyObject_GetAttrString(PyFrame_GetCode(frame), "co_filename")

  The problem with these nested calls is that the nested functions
  return strong references that are never cleaned and also the call can
fail, which will crash the interpreter or raise SystemError and doesn't
  allow the code to properly do error management.

* The macros for 3.11+ that return PyObject* are strong references that
  must be properly managed or otherwise the code will leak.

* There are many calls that are not checked for errors such as
  PyBytes_AsString, PyFrame_GetBack, PyThreadState_GetFrame, ...

* When PyThreadState_Get or PyThreadState_GetFrame return NULL, that's
  considered a hard error (for instance this can happen at interpreter
  teardown) and returning None is incorrect. The error must be
  propagated.

* There invalid paths are innecesarily obscured and the only situation
  where None should be returned (although I would advise to return an
error instead) is if the argument passed is not a unicode/string object.

* The function just receives one argument, so it should use METH_O
  instead of adding the extra complication of using METH_VARARGS.

The leaks can be confirmed using `memray` or a debug version of python.
Just use this example `setup.py` to compile the extension:

```
from setuptools import setup, Extension

extension_module = Extension(
    '_stacktrace',
    sources=['_stacktrace.c'],
)

setup(
    name='_stacktrace',
    version='1.0',
    ext_modules=[extension_module],
)
```

To check for leaks one possibility is using a debug version of the
interpreter and print the total reference count bere and after:

```
import _stacktrace as s
import sys, gc

for _ in range(100):
    gc.collect()
    a = sys.gettotalrefcount()
    s.get_info_frame(b"")
    gc.collect()
    print(sys.gettotalrefcount() -a)
```

This prints:

```
7
7
7
7
```

because there are 7 references being leaked (at least).

The other is using `memray` or another memory profieler. Use this drive
script:

```
# Test script
import _stacktrace as s
import gc

for _ in range(1_000_000):
    s.get_info_frame(b"")

gc.collect()
```

Then use memray:

```
$ memray run --trace-python-allocators --native -o lel.bin -f test.py
$ python -m memray flamegraph lel.bin --leaks -f
```


![leaks](https://github.com/DataDog/dd-trace-py/assets/11718525/18965b56-5159-4723-aa81-6a5f7242a1ef)

Note that frame objects do not appear in `memray` because we only leak
~7 frames and they use a freelist:


https://github.com/python/cpython/blob/97ce15c5f8743fb8b8967c6a05d6aec9fef9cbc7/Objects/frameobject.c#L810

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

Co-authored-by: Pablo Galindo Salgado <[email protected]>
Co-authored-by: Emmett Butler <[email protected]>

Author: 3 people

ddtrace/appsec/_iast/_stacktrace.c

@@ -17,11 +17,27 @@
 #define GET_LINENO(frame) PyFrame_GetLineNumber((PyFrameObject*)frame)
 #define GET_FRAME(tstate) PyThreadState_GetFrame(tstate)
 #define GET_PREVIOUS(frame) PyFrame_GetBack(frame)
-#define GET_FILENAME(frame) PyObject_GetAttrString(PyFrame_GetCode(frame), "co_filename")
+#define FRAME_DECREF(frame) Py_DECREF(frame)
+#define FRAME_XDECREF(frame) Py_XDECREF(frame)
+#define FILENAME_DECREF(filename) Py_DECREF(filename)
+#define FILENAME_XDECREF(filename) Py_XDECREF(filename)
+static inline PyObject*
+GET_FILENAME(PyFrameObject* frame)
+{
+    PyCodeObject* code = PyFrame_GetCode(frame);
+    if (!code) {
+        return NULL;
+    }
+    return PyObject_GetAttrString((PyObject*)code, "co_filename");
+}
 #else
 #define GET_FRAME(tstate) tstate->frame
 #define GET_PREVIOUS(frame) frame->f_back
 #define GET_FILENAME(frame) frame->f_code->co_filename
+#define FRAME_DECREF(frame)
+#define FRAME_XDECREF(frame)
+#define FILENAME_DECREF(filename)
+#define FILENAME_XDECREF(filename)
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 10
 /* See: https://bugs.python.org/issue44964 */
 #define GET_LINENO(frame) PyCode_Addr2Line(frame->f_code, frame->f_lasti * 2)
@@ -39,54 +55,69 @@
  * @return Tuple, string and integer.
  **/
 static PyObject*
-get_file_and_line(PyObject* Py_UNUSED(module), PyObject* args)
+get_file_and_line(PyObject* Py_UNUSED(module), PyObject* cwd_obj)
 {
-    PyThreadState* tstate = PyThreadState_GET();
-    PyFrameObject* frame;
-    PyObject* filename_o;
+    PyThreadState* tstate = PyThreadState_Get();
+    if (!tstate) {
+        return NULL;
+    }
+
     int line;
+    PyObject* filename_o = NULL;
+    PyObject* result = NULL;
+    PyObject* cwd_bytes = NULL;
+    char* cwd = NULL;
 
-    PyObject *cwd_obj = Py_None, *cwd_bytes;
-    char* cwd;
-    if (!PyArg_ParseTuple(args, "O", &cwd_obj))
+    if (!PyUnicode_FSConverter(cwd_obj, &cwd_bytes)) {
         return NULL;
-    if (cwd_obj != Py_None) {
-        if (!PyUnicode_FSConverter(cwd_obj, &cwd_bytes))
-            return NULL;
-        cwd = PyBytes_AsString(cwd_bytes);
-    } else {
+    }
+    cwd = PyBytes_AsString(cwd_bytes);
+    if (!cwd) {
+        Py_DECREF(cwd_bytes);
         return NULL;
     }
 
-    if (NULL != tstate && NULL != GET_FRAME(tstate)) {
-        frame = GET_FRAME(tstate);
-        while (NULL != frame) {
-            filename_o = GET_FILENAME(frame);
-            const char* filename = PyUnicode_AsUTF8(filename_o);
-            if (((strstr(filename, DD_TRACE_INSTALLED_PREFIX) != NULL && strstr(filename, TESTS_PREFIX) == NULL)) ||
-                (strstr(filename, SITE_PACKAGES_PREFIX) != NULL || strstr(filename, cwd) == NULL)) {
+    PyFrameObject* frame = GET_FRAME(tstate);
+    if (!frame) {
+        Py_DECREF(cwd_bytes);
+        return NULL;
+    }
 
-                frame = GET_PREVIOUS(frame);
-                continue;
-            }
-            /*
-             frame->f_lineno will not always return the correct line number
-             you need to call PyCode_Addr2Line().
-            */
-            line = GET_LINENO(frame);
-            return PyTuple_Pack(2, filename_o, Py_BuildValue("i", line));
+    while (NULL != frame) {
+        filename_o = GET_FILENAME(frame);
+        if (!filename_o) {
+            goto exit;
+        }
+        const char* filename = PyUnicode_AsUTF8(filename_o);
+        if (((strstr(filename, DD_TRACE_INSTALLED_PREFIX) != NULL && strstr(filename, TESTS_PREFIX) == NULL)) ||
+            (strstr(filename, SITE_PACKAGES_PREFIX) != NULL || strstr(filename, cwd) == NULL)) {
+            PyFrameObject* prev_frame = GET_PREVIOUS(frame);
+            FRAME_DECREF(frame);
+            FILENAME_DECREF(filename_o);
+            frame = prev_frame;
+            continue;
         }
+        /*
+         frame->f_lineno will not always return the correct line number
+         you need to call PyCode_Addr2Line().
+        */
+        line = GET_LINENO(frame);
+        PyObject* line_obj = Py_BuildValue("i", line);
+        if (!line_obj) {
+            goto exit;
+        }
+        result = PyTuple_Pack(2, filename_o, line_obj);
+        break;
     }
-#if PY_MAJOR_VERSION > 3 || PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 10
-    return Py_NewRef(Py_None);
-#else
-    Py_INCREF(Py_None);
-    return Py_None;
-#endif
+exit:
+    Py_DECREF(cwd_bytes);
+    FRAME_XDECREF(frame);
+    FILENAME_XDECREF(filename_o);
+    return result;
 }
 
 static PyMethodDef StacktraceMethods[] = {
-    { "get_info_frame", (PyCFunction)get_file_and_line, METH_VARARGS, "stacktrace functions" },
+    { "get_info_frame", (PyCFunction)get_file_and_line, METH_O, "stacktrace functions" },
     { NULL, NULL, 0, NULL }
 };
 
@@ -99,8 +130,7 @@ static struct PyModuleDef stacktrace = { PyModuleDef_HEAD_INIT,
 PyMODINIT_FUNC
 PyInit__stacktrace(void)
 {
-    PyObject* m;
-    m = PyModule_Create(&stacktrace);
+    PyObject* m = PyModule_Create(&stacktrace);
     if (m == NULL)
         return NULL;
     return m;

releasenotes/notes/fix-iast-mem-leak-25b7ae83cea24bd6.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    This fix eliminates some reference leaks and C-API usage in the ``_iast`` module.