Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the debug mode #142

Merged
merged 70 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
1cd887a
temporarily comment this out, see issue #140
antocuni Dec 15, 2020
4551d10
Add two fields to HPyContext:
antocuni Dec 15, 2020
97803d8
WIP: introduce hpy.debug.
antocuni Dec 15, 2020
2d868ec
refactor _ctx.c to use a statically allocated debug_ctx/debug_info, a…
antocuni Dec 15, 2020
c379b34
rename global_ctx into g_universal_ctx: I think it's clearer and we w…
antocuni Dec 16, 2020
c3c97c1
WIP 1
antocuni Dec 16, 2020
0f939ee
WIP: introduce a new function hpy.universal.load, which will be used …
antocuni Dec 16, 2020
3141873
kill hpy.universal.load_from_spec: rename it into load(), which takes…
antocuni Dec 16, 2020
ba33924
Partially undo some of the changes introduced by PR #124 (merged by c…
antocuni Dec 16, 2020
18c4a34
Implement hpy.universal.load(..., debug=True)
antocuni Dec 16, 2020
85ae3ae
blindly try to fix the tests
antocuni Dec 16, 2020
dd0c650
try again
antocuni Dec 16, 2020
fa5849f
Completely change the approach and greatly simplify the code.
antocuni Dec 17, 2020
d8a1736
some compilers complain that I can't use a static const in a static i…
antocuni Dec 17, 2020
d40d85d
test_FatalError needs __file__: make sure it is available also when w…
antocuni Dec 17, 2020
2caaa3b
distutils uses -DNDEBUG by default: make sure to re-enable asserts wh…
antocuni Dec 17, 2020
cd0f0ea
Merge branch 'master' into antocuni/debug-mode
hodgestar Dec 21, 2020
d6fcfe7
Move .load_module to test_importing and add some protection against m…
hodgestar Dec 21, 2020
b6fffec
autogenerate the debug wrappers and the definition of the debug ctx. …
antocuni Dec 30, 2020
099bedb
ignore these two files
antocuni Dec 31, 2020
ee51500
Introduce a _debug module which will contain the python-level interfa…
antocuni Dec 31, 2020
616d2c2
introduce DHPy, which is the debug mode's version of a handle, and mo…
antocuni Dec 31, 2020
34bb3d0
start to write and to test the logic to create debug handles
antocuni Dec 31, 2020
cc439cf
initialize debug_ctx->h_None & co.: test_noop passes again
antocuni Jan 11, 2021
95770ee
add a helper script to invoke pytest inside gdb
antocuni Jan 11, 2021
dca923a
fix indentation
antocuni Jan 11, 2021
a0a2302
Merge branch 'antocuni/debugging-tips' into antocuni/debug-mode
antocuni Jan 12, 2021
f58cda4
typo
antocuni Jan 12, 2021
f7270fc
make autogen after merging master
antocuni Jan 12, 2021
d0f7fb7
Big refactoring in hpy.universal handles
antocuni Jan 12, 2021
59184b2
use +1/-1 instead of ~ for _py2h and _h2py: it's easier to look at th…
antocuni Jan 15, 2021
484e28d
Fix this XXX: and properly close the handle
antocuni Jan 15, 2021
92216b1
add a helper function to help gdb sessions
antocuni Jan 15, 2021
51bb8de
WIP: refactor everything again.
antocuni Jan 15, 2021
12b6311
rename original_ctx into uctx
antocuni Jan 15, 2021
eadef67
Start to write the debug mode version of ctx_CallRealFunctionFromTram…
antocuni Jan 15, 2021
d1818ff
implement HPyFunc_O
antocuni Jan 15, 2021
d770971
initialize the debug context at runtime instead of compile time. This…
antocuni Jan 15, 2021
6a481ea
implement the logic for HPy_Close, and add a python function to get t…
antocuni Jan 17, 2021
2213191
Associate a generation to each open handle, and make it possible to g…
antocuni Jan 17, 2021
53b78ba
NULL DHPys need a bit of special casing. This is enough to make test_…
antocuni Jan 17, 2021
1a44f3d
implement the HPyFunc_VARARGS case inside debug_ctx_CallRealFunctionF…
antocuni Jan 17, 2021
f731b95
implement HPyFunc_KEYWORDS
antocuni Jan 17, 2021
00db14d
special case HPy_NULL also inside DHPy_wrap: this is needed to correc…
antocuni Jan 17, 2021
3799046
add a note
antocuni Jan 12, 2021
651aaaf
add some sanity check to ensure that we don't mix UHPy and DHPy wrongly
antocuni Jan 17, 2021
2e50a13
Use dctx and uctx everywhere, to make it clearer what is the ctx we e…
antocuni Jan 17, 2021
f94104c
implement HPyFunct_INITPROC for the debug mode
antocuni Jan 25, 2021
24e7588
implement debug_ctx_Tuple_FromArray
antocuni Jan 25, 2021
a186e66
implement debug_ctx_Type_GenericNew
antocuni Jan 25, 2021
a670b73
autogen the rest of debug_ctx_CallRealFunctionFromTrampoline; test_sl…
antocuni Jan 25, 2021
0a660c3
implement debug_ctx_Type_FromSpec manually, since the autogenerated w…
antocuni Jan 25, 2021
11974b8
add a note
antocuni Jan 25, 2021
5b8d102
introduce the applevel view of DebugHandle, and use it in _debug.get_…
antocuni Jan 25, 2021
abe4cc4
check the result of these two mallocs
antocuni Jan 26, 2021
b1df2ae
use HPyErr_NoMemory and check whether get_context() fails
antocuni Jan 27, 2021
cd8cd6e
Tell infer to ignore this file, as it reports a spurious memory leak.
antocuni Jan 27, 2021
77fdaf8
introduce DebugHandle.id
antocuni Jan 27, 2021
f5d415d
Merge branch 'master' into antocuni/debug-mode
antocuni Jan 27, 2021
64f2e36
fix autogen after the merge
antocuni Jan 27, 2021
4f1f406
Merge branch 'antocuni/hpy_type' into antocuni/debug-mode
antocuni Jan 28, 2021
239885a
explain what is a DebugHandleObject
antocuni Jan 28, 2021
c336f02
make autogen after the merge of the branch
antocuni Jan 28, 2021
cb13b9e
add ctx->h_NotImplemented and ctx->h_Ellipsis
antocuni Jan 28, 2021
c6174be
implement comparisons between DebugHandleObjects
antocuni Jan 28, 2021
ae25d49
Merge remote-tracking branch 'origin/master' into antocuni/debug-mode
antocuni Jan 28, 2021
bbd00ac
implement DebugHandle.repr
antocuni Jan 29, 2021
ebf7c77
implement a simple LeakDetector on top of the low-level debug-mode pr…
antocuni Jan 29, 2021
d18c016
make it possible to use LeakDetector as a context manager
antocuni Jan 29, 2021
beb2d07
avoid the ... syntax, because this file needs to be importable also b…
antocuni Jan 29, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ hpy/tools/autogen/autogen_pypy.txt
hpy/devel/include/common/version.h
hpy/devel/version.py

# stubs created by setup.py when doing build_ext --inplace
proof-of-concept/pof.py
proof-of-concept/pofpackage/foo.py

# Byte-compiled / optimized / DLL files
__pycache__/
*.py[cod]
Expand Down
10 changes: 7 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
all:
.PHONY: all
all: hpy.universal

.PHONY: hpy.universal
hpy.universal:
python3 setup.py build_ext -if

debug:
HPY_DEBUG=1 python3 setup.py build_ext -if
HPY_DEBUG=1 make all

autogen:
python3 -m hpy.tools.autogen .
Expand All @@ -15,7 +19,7 @@ cppcheck: cppcheck-build-dir

infer:
python3 setup.py build_ext -if -U NDEBUG | compiledb
@infer --fail-on-issue --compilation-database compile_commands.json
@infer --fail-on-issue --compilation-database compile_commands.json --report-blacklist-path-regex "hpy/debug/src/debug_ctx.c"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we perhaps leave a comment on why debug_ctx.c had to be blacklisted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good idea. The full explanation is in the log message of cd8cd6e, I added a comment which redirects to it


valgrind:
PYTHONMALLOC=malloc valgrind --suppressions=hpy/tools/valgrind/python.supp --suppressions=hpy/tools/valgrind/hpy.supp --leak-check=full --show-leak-kinds=definite,indirect --log-file=/tmp/valgrind-output python -m pytest --valgrind --valgrind-log=/tmp/valgrind-output test/
1 change: 1 addition & 0 deletions hpy/debug/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .leakdetector import HPyError, HPyLeak, LeakDetector
41 changes: 41 additions & 0 deletions hpy/debug/leakdetector.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
from hpy.universal import _debug

class HPyError(Exception):
pass

class HPyLeak(HPyError):
def __init__(self, leaks):
super().__init__()
self.leaks = leaks

def __str__(self):
lines = []
lines.append('%s handles have not been closed properly:' % len(self.leaks))
for dh in self.leaks:
lines.append(' %r' % dh)
return '\n'.join(lines)


class LeakDetector:

def __init__(self):
self.generation = None

def start(self):
if self.generation is not None:
raise ValueError('LeakDetector already started')
self.generation = _debug.new_generation()

def stop(self):
if self.generation is None:
raise ValueError('LeakDetector not started yet')
leaks = _debug.get_open_handles(self.generation)
if leaks:
raise HPyLeak(leaks)

def __enter__(self):
self.start()
return self

def __exit__(self, etype, evalue, tb):
self.stop()
212 changes: 212 additions & 0 deletions hpy/debug/src/_debugmod.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
// Python-level interface for the _debug module. Written in HPy itself, the
// idea is that it should be reusable by other implementations

// NOTE: hpy.debug._debug is loaded using the UNIVERSAL ctx. To make it
// clearer, we will use "uctx" and "dctx" to distinguish them.

#include "hpy.h"
#include "debug_internal.h"

static UHPy new_DebugHandleObj(HPyContext uctx, UHPy u_DebugHandleType,
DebugHandle *handle);


HPyDef_METH(new_generation, "new_generation", new_generation_impl, HPyFunc_NOARGS)
static UHPy new_generation_impl(HPyContext uctx, UHPy self)
{
HPyContext dctx = hpy_debug_get_ctx(uctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

Obtaining a new debug ctx from the uctx requires that the uctx be the same in this module as in the module being debugged, yes? Which means there essentially needs to be only one ctx per interpreter? This is already true in the CPython and PyPy (I think) implementations, but it's not currently required by HPy.

I'm not quite sure how to remove this constraint right now -- the ctx is fairly invisible from Python code, and that likely needs to stay that way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried to explain more what are the implicit and explicit assumptions in commit 6bad6c3. If it's still unclear, please comment/review/reword directly on PR #164

HPyDebugInfo *info = get_info(dctx);
info->current_generation++;
return HPyLong_FromLong(uctx, info->current_generation);
}


HPyDef_METH(get_open_handles, "get_open_handles", get_open_handles_impl, HPyFunc_O, .doc=
"Return a list containing all the open handles whose generation is >= "
"of the given arg")
static UHPy get_open_handles_impl(HPyContext uctx, UHPy u_self, UHPy u_gen)
{
HPyContext dctx = hpy_debug_get_ctx(uctx);
HPyDebugInfo *info = get_info(dctx);

UHPy u_DebugHandleType = HPy_GetAttr_s(uctx, u_self, "DebugHandle");
if (HPy_IsNull(u_DebugHandleType))
return HPy_NULL;

long gen = HPyLong_AsLong(uctx, u_gen);
if (HPyErr_Occurred(uctx))
return HPy_NULL;

UHPy u_result = HPyList_New(uctx, 0);
if (HPy_IsNull(u_result))
return HPy_NULL;

DebugHandle *dh = info->open_handles;
while(dh != NULL) {
if (dh->generation >= gen) {
UHPy u_item = new_DebugHandleObj(uctx, u_DebugHandleType, dh);
if (HPy_IsNull(u_item))
return HPy_NULL;
if (HPyList_Append(uctx, u_result, u_item) == -1) {
HPy_Close(uctx, u_item);
HPy_Close(uctx, u_result);
return HPy_NULL;
}
HPy_Close(uctx, u_item);
}
dh = dh->next;
}
return u_result;
}

/* ~~~~~~ DebugHandleType and DebugHandleObject ~~~~~~~~

This is the applevel view of a DebugHandle/DHPy.

Note that there are two different ways to expose DebugHandle to applevel:

1. make DebugHandle itself a Python object: this is simple but means that
you have to pay the PyObject_HEAD overhead (16 bytes) for all of them

2. make DebugHandle a plain C struct, and expose them through a
Python-level wrapper.

We choose to implement solution 2 because we expect to have many
DebugHandle around, but to expose only few of them to applevel, when you
call get_open_handles. This way, we save 16 bytes per DebugHandle.

This means that you can have different DebugHandleObjects wrapping the same
DebugHandle. To make it easier to compare them, they expose the .id
attribute, which is the address of the wrapped DebugHandle. Also,
DebugHandleObjects compare equal if their .id is equal.
*/

typedef struct {
HPyObject_HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make this a pure type once the cast support branch lands?

Copy link
Collaborator Author

@antocuni antocuni Feb 9, 2021

Choose a reason for hiding this comment

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

yes!
Not only we "could", but we should. HPyObject_HEAD will be killed anyway, won't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. HPyObject_HEAD is already gone in the cast support branch.

DebugHandle *handle;
} DebugHandleObject;

HPyDef_GET(DebugHandle_obj, "obj", DebugHandle_obj_get,
.doc="The object which the handle points to")
static UHPy DebugHandle_obj_get(HPyContext uctx, UHPy self, void *closure)
{
DebugHandleObject *dh = HPy_CAST(uctx, DebugHandleObject, self);
return HPy_Dup(uctx, dh->handle->uh);
}

HPyDef_GET(DebugHandle_id, "id", DebugHandle_id_get,
.doc="A numeric identifier representing the underlying universal handle")
static UHPy DebugHandle_id_get(HPyContext uctx, UHPy self, void *closure)
{
DebugHandleObject *dh = HPy_CAST(uctx, DebugHandleObject, self);
return HPyLong_FromSsize_t(uctx, (HPy_ssize_t)dh->handle);
}

HPyDef_SLOT(DebugHandle_cmp, DebugHandle_cmp_impl, HPy_tp_richcompare)
static UHPy DebugHandle_cmp_impl(HPyContext uctx, UHPy self, UHPy o, HPy_RichCmpOp op)
{
UHPy T = HPy_Type(uctx, self);
if (!HPy_TypeCheck(uctx, o, T))
return HPy_Dup(uctx, uctx->h_NotImplemented);
DebugHandleObject *dh_self = HPy_CAST(uctx, DebugHandleObject, self);
DebugHandleObject *dh_o = HPy_CAST(uctx, DebugHandleObject, o);

switch(op) {
case HPy_EQ:
return HPyBool_FromLong(uctx, dh_self->handle == dh_o->handle);
case HPy_NE:
return HPyBool_FromLong(uctx, dh_self->handle != dh_o->handle);
default:
return HPy_Dup(uctx, uctx->h_NotImplemented);
}
}

HPyDef_SLOT(DebugHandle_repr, DebugHandle_repr_impl, HPy_tp_repr)
static UHPy DebugHandle_repr_impl(HPyContext uctx, UHPy self)
{
DebugHandleObject *dh = HPy_CAST(uctx, DebugHandleObject, self);
UHPy uh_fmt = HPy_NULL;
UHPy uh_id = HPy_NULL;
UHPy uh_args = HPy_NULL;
UHPy uh_result = HPy_NULL;

uh_fmt = HPyUnicode_FromString(uctx, "<DebugHandle 0x%x for %r>");
Copy link
Contributor

Choose a reason for hiding this comment

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

We could do with PyUnicode_FromFormat here.

Copy link
Collaborator Author

@antocuni antocuni Feb 9, 2021

Choose a reason for hiding this comment

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

good point, although we don't have HPyUnicode_FromFormat here. And we don't want to use the Py_* version because this is supposed to be a pure-hpy module, remember it will be used also by pypy and hopefully graalpython.
I added an XXX in e2b548a

if (HPy_IsNull(uh_fmt))
goto exit;

uh_id = HPyLong_FromSsize_t(uctx, (HPy_ssize_t)dh->handle);
if (HPy_IsNull(uh_id))
goto exit;

uh_args = HPyTuple_FromArray(uctx, (UHPy[]){uh_id, dh->handle->uh}, 2);
if (HPy_IsNull(uh_args))
goto exit;

uh_result = HPy_Remainder(uctx, uh_fmt, uh_args);

exit:
HPy_Close(uctx, uh_fmt);
HPy_Close(uctx, uh_id);
HPy_Close(uctx, uh_args);
return uh_result;
}



static HPyDef *DebugHandleType_defs[] = {
&DebugHandle_obj,
&DebugHandle_id,
&DebugHandle_cmp,
&DebugHandle_repr,
NULL
};

static HPyType_Spec DebugHandleType_spec = {
.name = "hpy.debug._debug.DebugHandle",
.basicsize = sizeof(DebugHandleObject),
.flags = HPy_TPFLAGS_DEFAULT,
.defines = DebugHandleType_defs,
};


static UHPy new_DebugHandleObj(HPyContext uctx, UHPy u_DebugHandleType,
DebugHandle *handle)
{
DebugHandleObject *dhobj;
UHPy u_result = HPy_New(uctx, u_DebugHandleType, &dhobj);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to put the DebugHandle type on the context like we do for other build-in types like List?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is also a good illustration of the slight hopping one has to do at the moment since global type objects are gone -- and the hopping is actually not too bad in simple cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would it make sense to put the DebugHandle type on the context like we do for other build-in types like List?

no, because it's not a builtin. Why should it be different than, say, ndarray?

I guess this is also a good illustration of the slight hopping one has to do at the moment since global type objects are gone -- and the hopping is actually not too bad in simple cases.

exactly, that's the point. We need a way to have per-module "globals" at the C level, which we still don't have :(

dhobj->handle = handle;
return u_result;
}


/* ~~~~~~ definition of the module hpy.debug._debug ~~~~~~~ */

static HPyDef *module_defines[] = {
&new_generation,
&get_open_handles,
NULL
};

static HPyModuleDef moduledef = {
HPyModuleDef_HEAD_INIT,
.m_name = "hpy.debug._debug",
.m_doc = "HPy debug mode",
.m_size = -1,
.defines = module_defines
};


HPy_MODINIT(_debug)
static UHPy init__debug_impl(HPyContext uctx)
{
UHPy m = HPyModule_Create(uctx, &moduledef);
if (HPy_IsNull(m))
return HPy_NULL;

UHPy h_DebugHandleType = HPyType_FromSpec(uctx, &DebugHandleType_spec, NULL);
if (HPy_IsNull(h_DebugHandleType))
return HPy_NULL;
HPy_SetAttr_s(uctx, m, "DebugHandle", h_DebugHandleType);
HPy_Close(uctx, h_DebugHandleType);
return m;
}
Loading