* [PATCH 0/7] Standarize int-wrapping approach in Python code
@ 2020-09-11 14:33 Tom Tromey
2020-09-11 14:33 ` [PATCH 1/7] Don't use PyInt_FromSsize_t Tom Tromey
` (7 more replies)
0 siblings, 8 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches
A while back I fixed a bug where a CORE_ADDR was wrapped for Python
using PyLong_FromLong, rather than using an unsigned conversion.
After doing this, I thought it would be good to audit the code for
other oddities. Mostly what I found were possible overflows from code
that converted specifically to "int" (an issue only for Python 2 I
suppose).
This series changes the Python code in gdb to prefer two of its own
functions -- one signed and one unsigned -- which should avoid all
issues both now and in the future. The idea here is ban PyInt_From*
and PyLong_From*, and simply always use our own wrappers.
Let me know what you think.
Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/7] Don't use PyInt_FromSsize_t
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-11 14:33 ` [PATCH 2/7] Don't use gdb_py_long_from_longest Tom Tromey
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Change the Python layer to avoid PyInt_FromSsize_t, and remove the
compatibility define.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/python-internal.h (PyInt_FromSsize_t): Remove define.
* python/py-record.c (recpy_element_number): Use
gdb_py_object_from_longest.
(recpy_gap_number): Likewise.
---
gdb/ChangeLog | 7 +++++++
gdb/python/py-record.c | 4 ++--
gdb/python/python-internal.h | 1 -
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index a081ca942f3..a6b08dcdd83 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -374,7 +374,7 @@ recpy_element_number (PyObject *self, void* closure)
{
const recpy_element_object * const obj = (recpy_element_object *) self;
- return PyInt_FromSsize_t (obj->number);
+ return gdb_py_object_from_longest (obj->number).release ();
}
/* Implementation of RecordInstruction.__hash__ [int] and
@@ -454,7 +454,7 @@ recpy_gap_number (PyObject *self, void *closure)
{
const recpy_gap_object * const obj = (const recpy_gap_object *) self;
- return PyInt_FromSsize_t (obj->number);
+ return gdb_py_object_from_longest (obj->number).release ();
}
/* Implementation of RecordGap.error_code [int]. */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 6874543441b..e406f37533e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -97,7 +97,6 @@
#define PyInt_Check PyLong_Check
#define PyInt_FromLong PyLong_FromLong
-#define PyInt_FromSsize_t PyLong_FromSsize_t
#define PyInt_AsLong PyLong_AsLong
#define PyInt_AsSsize_t PyLong_AsSsize_t
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/7] Don't use gdb_py_long_from_longest
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
2020-09-11 14:33 ` [PATCH 1/7] Don't use PyInt_FromSsize_t Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-11 14:33 ` [PATCH 3/7] Don't use gdb_py_long_from_ulongest Tom Tromey
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Change the Python layer to avoid gdb_py_long_from_longest, and remove
the defines.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/python-internal.h (gdb_py_long_from_longest): Remove
defines.
* python/py-value.c (valpy_long): Use gdb_py_object_from_longest.
* python/py-type.c (convert_field, typy_get_sizeof): Use
gdb_py_object_from_longest.
* python/py-record-btrace.c (btpy_list_index): Use
gdb_py_object_from_longest.
---
gdb/ChangeLog | 10 ++++++++++
gdb/python/py-record-btrace.c | 2 +-
gdb/python/py-type.c | 8 +++-----
gdb/python/py-value.c | 2 +-
gdb/python/python-internal.h | 2 --
5 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 08613a856d0..84a3d9eae73 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -556,7 +556,7 @@ btpy_list_index (PyObject *self, PyObject *value)
if (index < 0)
return PyErr_Format (PyExc_ValueError, _("Not in list."));
- return gdb_py_long_from_longest (index);
+ return gdb_py_object_from_longest (index).release ();
}
/* Implementation of BtraceList.count (self, value) -> int. */
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index d0dfb52811b..c5a071fee21 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -183,8 +183,7 @@ convert_field (struct type *type, int field)
if (type->code () == TYPE_CODE_ENUM)
{
- arg.reset (gdb_py_long_from_longest (TYPE_FIELD_ENUMVAL (type,
- field)));
+ arg = gdb_py_object_from_longest (TYPE_FIELD_ENUMVAL (type, field));
attrstring = "enumval";
}
else
@@ -192,8 +191,7 @@ convert_field (struct type *type, int field)
if (TYPE_FIELD_LOC_KIND (type, field) == FIELD_LOC_KIND_DWARF_BLOCK)
arg = gdbpy_ref<>::new_reference (Py_None);
else
- arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
- field)));
+ arg = gdb_py_object_from_longest (TYPE_FIELD_BITPOS (type, field));
attrstring = "bitpos";
}
@@ -725,7 +723,7 @@ typy_get_sizeof (PyObject *self, void *closure)
if (size_varies)
Py_RETURN_NONE;
- return gdb_py_long_from_longest (TYPE_LENGTH (type));
+ return gdb_py_object_from_longest (TYPE_LENGTH (type)).release ();
}
/* Return the alignment of the type represented by SELF, in bytes. */
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 2ebbe0a3569..0e3b7025fae 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1733,7 +1733,7 @@ valpy_long (PyObject *self)
if (TYPE_UNSIGNED (type))
return gdb_py_long_from_ulongest (l);
else
- return gdb_py_long_from_longest (l);
+ return gdb_py_object_from_longest (l).release ();
}
/* Implements conversion to float. */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index e406f37533e..b93c78fd6e1 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -126,7 +126,6 @@
#define GDB_PY_LLU_ARG "K"
typedef PY_LONG_LONG gdb_py_longest;
typedef unsigned PY_LONG_LONG gdb_py_ulongest;
-#define gdb_py_long_from_longest PyLong_FromLongLong
#define gdb_py_long_from_ulongest PyLong_FromUnsignedLongLong
#define gdb_py_long_as_ulongest PyLong_AsUnsignedLongLong
@@ -136,7 +135,6 @@ typedef unsigned PY_LONG_LONG gdb_py_ulongest;
#define GDB_PY_LLU_ARG "K"
typedef long gdb_py_longest;
typedef unsigned long gdb_py_ulongest;
-#define gdb_py_long_from_longest PyLong_FromLong
#define gdb_py_long_from_ulongest PyLong_FromUnsignedLong
#define gdb_py_long_as_ulongest PyLong_AsUnsignedLong
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/7] Don't use gdb_py_long_from_ulongest
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
2020-09-11 14:33 ` [PATCH 1/7] Don't use PyInt_FromSsize_t Tom Tromey
2020-09-11 14:33 ` [PATCH 2/7] Don't use gdb_py_long_from_longest Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-11 14:33 ` [PATCH 4/7] Don't use PyLong_FromLong Tom Tromey
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Remove the gdb_py_long_from_ulongest defines and change the Python
layer to prefer gdb_py_object_from_ulongest. While writing this I
noticed that the error handling in archpy_disassemble was incorrect --
it could call PyDict_SetItemString with a NULL value. This patch also
fixes this bug.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/python-internal.h (gdb_py_long_from_ulongest): Remove
defines.
* python/py-value.c (valpy_long): Use
gdb_py_object_from_ulongest.
* python/py-symtab.c (salpy_get_pc): Use
gdb_py_object_from_ulongest.
(salpy_get_last): Likewise.
* python/py-record-btrace.c (recpy_bt_insn_pc): Use
gdb_py_object_from_ulongest.
* python/py-lazy-string.c (stpy_get_address): Use
gdb_py_object_from_ulongest.
* python/py-frame.c (frapy_pc): Use gdb_py_object_from_ulongest.
* python/py-arch.c (archpy_disassemble): Use
gdb_py_object_from_ulongest and gdb_py_object_from_longest. Fix
error handling.
---
gdb/ChangeLog | 18 ++++++++++++++++++
gdb/python/py-arch.c | 25 +++++++++++++++++--------
gdb/python/py-frame.c | 2 +-
gdb/python/py-lazy-string.c | 2 +-
gdb/python/py-record-btrace.c | 2 +-
gdb/python/py-symtab.c | 4 ++--
gdb/python/py-value.c | 2 +-
gdb/python/python-internal.h | 2 --
8 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index d9eaf81a30a..3f8e769ff59 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -209,14 +209,23 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
return NULL;
}
- if (PyDict_SetItemString (insn_dict.get (), "addr",
- gdb_py_long_from_ulongest (pc))
- || PyDict_SetItemString (insn_dict.get (), "asm",
- PyString_FromString (!stb.empty ()
- ? stb.c_str ()
- : "<unknown>"))
- || PyDict_SetItemString (insn_dict.get (), "length",
- PyInt_FromLong (insn_len)))
+ gdbpy_ref<> pc_obj = gdb_py_object_from_ulongest (pc);
+ if (pc_obj == nullptr)
+ return nullptr;
+
+ gdbpy_ref<> asm_obj (PyString_FromString (!stb.empty ()
+ ? stb.c_str ()
+ : "<unknown>"));
+ if (asm_obj == nullptr)
+ return nullptr;
+
+ gdbpy_ref<> len_obj = gdb_py_object_from_longest (insn_len);
+ if (len_obj == nullptr)
+ return nullptr;
+
+ if (PyDict_SetItemString (insn_dict.get (), "addr", pc_obj.get ())
+ || PyDict_SetItemString (insn_dict.get (), "asm", asm_obj.get ())
+ || PyDict_SetItemString (insn_dict.get (), "length", len_obj.get ()))
return NULL;
pc += insn_len;
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index e121afb222d..090705507e6 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -232,7 +232,7 @@ frapy_pc (PyObject *self, PyObject *args)
GDB_PY_HANDLE_EXCEPTION (except);
}
- return gdb_py_long_from_ulongest (pc);
+ return gdb_py_object_from_ulongest (pc).release ();
}
/* Implementation of gdb.Frame.read_register (self, register) -> gdb.Value.
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index f71bb1d613d..401c0a6fdf5 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -61,7 +61,7 @@ stpy_get_address (PyObject *self, void *closure)
{
lazy_string_object *self_string = (lazy_string_object *) self;
- return gdb_py_long_from_ulongest (self_string->address);
+ return gdb_py_object_from_ulongest (self_string->address).release ();
}
static PyObject *
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 84a3d9eae73..762c0bcbcc0 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -232,7 +232,7 @@ recpy_bt_insn_pc (PyObject *self, void *closure)
if (insn == NULL)
return NULL;
- return gdb_py_long_from_ulongest (insn->pc);
+ return gdb_py_object_from_ulongest (insn->pc).release ();
}
/* Implementation of RecordInstruction.size [int] for btrace.
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 6229bc5123b..b0e7618af7e 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -264,7 +264,7 @@ salpy_get_pc (PyObject *self, void *closure)
SALPY_REQUIRE_VALID (self, sal);
- return gdb_py_long_from_ulongest (sal->pc);
+ return gdb_py_object_from_ulongest (sal->pc).release ();
}
/* Implementation of the get method for the 'last' attribute of
@@ -278,7 +278,7 @@ salpy_get_last (PyObject *self, void *closure)
SALPY_REQUIRE_VALID (self, sal);
if (sal->end > 0)
- return gdb_py_long_from_ulongest (sal->end - 1);
+ return gdb_py_object_from_ulongest (sal->end - 1).release ();
else
Py_RETURN_NONE;
}
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 0e3b7025fae..a884d6b70b2 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1731,7 +1731,7 @@ valpy_long (PyObject *self)
}
if (TYPE_UNSIGNED (type))
- return gdb_py_long_from_ulongest (l);
+ return gdb_py_object_from_ulongest (l).release ();
else
return gdb_py_object_from_longest (l).release ();
}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index b93c78fd6e1..2c4195f3d03 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -126,7 +126,6 @@
#define GDB_PY_LLU_ARG "K"
typedef PY_LONG_LONG gdb_py_longest;
typedef unsigned PY_LONG_LONG gdb_py_ulongest;
-#define gdb_py_long_from_ulongest PyLong_FromUnsignedLongLong
#define gdb_py_long_as_ulongest PyLong_AsUnsignedLongLong
#else /* HAVE_LONG_LONG */
@@ -135,7 +134,6 @@ typedef unsigned PY_LONG_LONG gdb_py_ulongest;
#define GDB_PY_LLU_ARG "K"
typedef long gdb_py_longest;
typedef unsigned long gdb_py_ulongest;
-#define gdb_py_long_from_ulongest PyLong_FromUnsignedLong
#define gdb_py_long_as_ulongest PyLong_AsUnsignedLong
#endif /* HAVE_LONG_LONG */
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/7] Don't use PyLong_FromLong
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
` (2 preceding siblings ...)
2020-09-11 14:33 ` [PATCH 3/7] Don't use gdb_py_long_from_ulongest Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-11 14:33 ` [PATCH 5/7] Don't use PyLong_FromLongLong Tom Tromey
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes gdb to avoid PyLong_FromLong, preferring to
gdb_py_object_from_longest instead.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/python.c (gdbpy_parameter_value): Use
gdb_py_object_from_longest.
* python/py-type.c (convert_field, typy_range): Use
gdb_py_object_from_longest.
* python/py-tui.c (gdbpy_tui_width, gdbpy_tui_height): Use
gdb_py_object_from_longest.
* python/py-lazy-string.c (stpy_get_length): Use
gdb_py_object_from_longest.
* python/py-infthread.c (thpy_get_num, thpy_get_global_num): Use
gdb_py_object_from_longest.
* python/py-infevents.c (create_memory_changed_event_object): Use
gdb_py_object_from_longest.
* python/py-inferior.c (infpy_get_num): Use
gdb_py_object_from_longest.
(infpy_get_pid): Likewise.
---
gdb/ChangeLog | 18 ++++++++++++++++++
gdb/python/py-inferior.c | 4 ++--
gdb/python/py-infevents.c | 2 +-
gdb/python/py-infthread.c | 8 ++++++--
gdb/python/py-lazy-string.c | 2 +-
gdb/python/py-tui.c | 8 ++++++--
gdb/python/py-type.c | 6 +++---
gdb/python/python.c | 2 +-
8 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index a7699510db7..7204356ac73 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -423,7 +423,7 @@ infpy_get_num (PyObject *self, void *closure)
INFPY_REQUIRE_VALID (inf);
- return PyLong_FromLong (inf->inferior->num);
+ return gdb_py_object_from_longest (inf->inferior->num).release ();
}
static PyObject *
@@ -433,7 +433,7 @@ infpy_get_pid (PyObject *self, void *closure)
INFPY_REQUIRE_VALID (inf);
- return PyLong_FromLong (inf->inferior->pid);
+ return gdb_py_object_from_longest (inf->inferior->pid).release ();
}
static PyObject *
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index 3cf746103c7..f303c395c13 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -104,7 +104,7 @@ create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
if (evpy_add_attribute (event.get (), "address", addr_obj.get ()) < 0)
return NULL;
- gdbpy_ref<> len_obj (PyLong_FromLong (len));
+ gdbpy_ref<> len_obj = gdb_py_object_from_longest (len);
if (len_obj == NULL)
return NULL;
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index f350e367d5e..669b6d8b074 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -130,7 +130,9 @@ thpy_get_num (PyObject *self, void *closure)
THPY_REQUIRE_VALID (thread_obj);
- return PyLong_FromLong (thread_obj->thread->per_inf_num);
+ gdbpy_ref<> result
+ = gdb_py_object_from_longest (thread_obj->thread->per_inf_num);
+ return result.release ();
}
/* Getter for InferiorThread.global_num. */
@@ -142,7 +144,9 @@ thpy_get_global_num (PyObject *self, void *closure)
THPY_REQUIRE_VALID (thread_obj);
- return PyLong_FromLong (thread_obj->thread->global_num);
+ gdbpy_ref<> result
+ = gdb_py_object_from_longest (thread_obj->thread->global_num);
+ return result.release ();
}
/* Getter for InferiorThread.ptid -> (pid, lwp, tid).
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index 401c0a6fdf5..aaee94f5e1f 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -88,7 +88,7 @@ stpy_get_length (PyObject *self, void *closure)
{
lazy_string_object *self_string = (lazy_string_object *) self;
- return PyLong_FromLong (self_string->length);
+ return gdb_py_object_from_longest (self_string->length).release ();
}
static PyObject *
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 95c71f1d2dd..8a24f2f3af4 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -392,7 +392,9 @@ gdbpy_tui_width (PyObject *self, void *closure)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
REQUIRE_WINDOW (win);
- return PyLong_FromLong (win->window->viewport_width ());
+ gdbpy_ref<> result
+ = gdb_py_object_from_longest (win->window->viewport_width ());
+ return result.release ();
}
/* Return the height of the TUI window. */
@@ -401,7 +403,9 @@ gdbpy_tui_height (PyObject *self, void *closure)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
REQUIRE_WINDOW (win);
- return PyLong_FromLong (win->window->viewport_height ());
+ gdbpy_ref<> result
+ = gdb_py_object_from_longest (win->window->viewport_height ());
+ return result.release ();
}
/* Return the title of the TUI window. */
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index c5a071fee21..8825b0749ce 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -233,7 +233,7 @@ convert_field (struct type *type, int field)
if (PyObject_SetAttrString (result.get (), "is_base_class", arg.get ()) < 0)
return NULL;
- arg.reset (PyLong_FromLong (TYPE_FIELD_BITSIZE (type, field)));
+ arg = gdb_py_object_from_longest (TYPE_FIELD_BITSIZE (type, field));
if (arg == NULL)
return NULL;
if (PyObject_SetAttrString (result.get (), "bitsize", arg.get ()) < 0)
@@ -596,11 +596,11 @@ typy_range (PyObject *self, PyObject *args)
break;
}
- gdbpy_ref<> low_bound (PyLong_FromLong (low));
+ gdbpy_ref<> low_bound = gdb_py_object_from_longest (low);
if (low_bound == NULL)
return NULL;
- gdbpy_ref<> high_bound (PyLong_FromLong (high));
+ gdbpy_ref<> high_bound = gdb_py_object_from_longest (high);
if (high_bound == NULL)
return NULL;
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7787dce4b4c..9cc8af63338 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -496,7 +496,7 @@ gdbpy_parameter_value (enum var_types type, void *var)
/* Fall through. */
case var_zinteger:
case var_zuinteger_unlimited:
- return PyLong_FromLong (* (int *) var);
+ return gdb_py_object_from_longest (* (int *) var).release ();
case var_uinteger:
{
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/7] Don't use PyLong_FromLongLong
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
` (3 preceding siblings ...)
2020-09-11 14:33 ` [PATCH 4/7] Don't use PyLong_FromLong Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-11 14:33 ` [PATCH 6/7] Don't use PyLong_FromUnsignedLong Tom Tromey
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes gdb to avoid PyLong_FromLongLong, preferring to use
gdb_py_object_from_longest instead.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/py-infevents.c (create_register_changed_event_object):
Use gdb_py_object_from_longest.
* python/py-exitedevent.c (create_exited_event_object): Use
gdb_py_object_from_longest.
---
gdb/ChangeLog | 7 +++++++
gdb/python/py-exitedevent.c | 2 +-
gdb/python/py-infevents.c | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 9912b26e909..b19cc959884 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -30,7 +30,7 @@ create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
if (exit_code)
{
- gdbpy_ref<> exit_code_obj (PyLong_FromLongLong (*exit_code));
+ gdbpy_ref<> exit_code_obj = gdb_py_object_from_longest (*exit_code);
if (exit_code_obj == NULL)
return NULL;
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index f303c395c13..b653e4a3229 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -76,7 +76,7 @@ create_register_changed_event_object (struct frame_info *frame,
if (evpy_add_attribute (event.get (), "frame", frame_obj.get ()) < 0)
return NULL;
- gdbpy_ref<> regnum_obj (PyLong_FromLongLong (regnum));
+ gdbpy_ref<> regnum_obj = gdb_py_object_from_longest (regnum);
if (regnum_obj == NULL)
return NULL;
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 6/7] Don't use PyLong_FromUnsignedLong
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
` (4 preceding siblings ...)
2020-09-11 14:33 ` [PATCH 5/7] Don't use PyLong_FromLongLong Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-11 14:33 ` [PATCH 7/7] Don't use PyInt_FromLong Tom Tromey
2020-09-12 16:16 ` [PATCH 0/7] Standarize int-wrapping approach in Python code Joel Brobecker
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes gdb to avoid PyLong_FromUnsignedLong, preferring
gdb_py_object_from_ulongest instead.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/python.c (gdbpy_parameter_value): Use
gdb_py_object_from_ulongest.
---
gdb/ChangeLog | 5 +++++
gdb/python/python.c | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9cc8af63338..878602712d8 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -504,13 +504,13 @@ gdbpy_parameter_value (enum var_types type, void *var)
if (val == UINT_MAX)
Py_RETURN_NONE;
- return PyLong_FromUnsignedLong (val);
+ return gdb_py_object_from_ulongest (val).release ();
}
case var_zuinteger:
{
unsigned int val = * (unsigned int *) var;
- return PyLong_FromUnsignedLong (val);
+ return gdb_py_object_from_ulongest (val).release ();
}
}
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 7/7] Don't use PyInt_FromLong
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
` (5 preceding siblings ...)
2020-09-11 14:33 ` [PATCH 6/7] Don't use PyLong_FromUnsignedLong Tom Tromey
@ 2020-09-11 14:33 ` Tom Tromey
2020-09-12 16:16 ` [PATCH 0/7] Standarize int-wrapping approach in Python code Joel Brobecker
7 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-11 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Avoid the use of PyInt_FromLong, preferring gdb_py_object_from_longest
instead. I found found another spot that was incorrectly handling
errors (see gdbpy_create_ptid_object) while writing this patch; it is
fixed here.
2020-09-11 Tom Tromey <tromey@adacore.com>
* python/python-internal.h (PyInt_FromLong): Remove define.
* python/py-value.c (convert_value_from_python): Use
gdb_py_object_from_longest.
* python/py-type.c (typy_get_code): Use
gdb_py_object_from_longest.
* python/py-symtab.c (salpy_get_line): Use
gdb_py_object_from_longest.
* python/py-symbol.c (sympy_get_addr_class, sympy_line): Use
gdb_py_object_from_longest.
* python/py-record.c (recpy_gap_reason_code): Use
gdb_py_object_from_longest.
* python/py-record-btrace.c (recpy_bt_insn_size)
(recpy_bt_func_level, btpy_list_count): Use
gdb_py_object_from_longest.
* python/py-infthread.c (gdbpy_create_ptid_object): Use
gdb_py_object_from_longest. Fix error handling.
* python/py-framefilter.c (bootstrap_python_frame_filters): Use
gdb_py_object_from_longest.
* python/py-frame.c (frapy_type, frapy_unwind_stop_reason): Use
gdb_py_object_from_longest.
* python/py-breakpoint.c (bppy_get_type, bppy_get_number)
(bppy_get_thread, bppy_get_task, bppy_get_hit_count)
(bppy_get_ignore_count): Use gdb_py_object_from_longest.
---
gdb/ChangeLog | 26 ++++++++++++++++++++++++++
gdb/python/py-breakpoint.c | 12 ++++++------
gdb/python/py-frame.c | 4 ++--
gdb/python/py-framefilter.c | 4 ++--
gdb/python/py-infthread.c | 19 +++++++++++++++----
gdb/python/py-record-btrace.c | 8 +++++---
gdb/python/py-record.c | 2 +-
gdb/python/py-symbol.c | 4 ++--
gdb/python/py-symtab.c | 2 +-
gdb/python/py-type.c | 2 +-
gdb/python/py-value.c | 2 +-
gdb/python/python-internal.h | 1 -
12 files changed, 62 insertions(+), 24 deletions(-)
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 11345ad3052..7369c91ad90 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -552,7 +552,7 @@ bppy_get_type (PyObject *self, void *closure)
BPPY_REQUIRE_VALID (self_bp);
- return PyInt_FromLong (self_bp->bp->type);
+ return gdb_py_object_from_longest (self_bp->bp->type).release ();
}
/* Python function to get the visibility of the breakpoint. */
@@ -613,7 +613,7 @@ bppy_get_number (PyObject *self, void *closure)
BPPY_REQUIRE_VALID (self_bp);
- return PyInt_FromLong (self_bp->number);
+ return gdb_py_object_from_longest (self_bp->number).release ();
}
/* Python function to get the breakpoint's thread ID. */
@@ -627,7 +627,7 @@ bppy_get_thread (PyObject *self, void *closure)
if (self_bp->bp->thread == -1)
Py_RETURN_NONE;
- return PyInt_FromLong (self_bp->bp->thread);
+ return gdb_py_object_from_longest (self_bp->bp->thread).release ();
}
/* Python function to get the breakpoint's task ID (in Ada). */
@@ -641,7 +641,7 @@ bppy_get_task (PyObject *self, void *closure)
if (self_bp->bp->task == 0)
Py_RETURN_NONE;
- return PyInt_FromLong (self_bp->bp->task);
+ return gdb_py_object_from_longest (self_bp->bp->task).release ();
}
/* Python function to get the breakpoint's hit count. */
@@ -652,7 +652,7 @@ bppy_get_hit_count (PyObject *self, void *closure)
BPPY_REQUIRE_VALID (self_bp);
- return PyInt_FromLong (self_bp->bp->hit_count);
+ return gdb_py_object_from_longest (self_bp->bp->hit_count).release ();
}
/* Python function to get the breakpoint's ignore count. */
@@ -663,7 +663,7 @@ bppy_get_ignore_count (PyObject *self, void *closure)
BPPY_REQUIRE_VALID (self_bp);
- return PyInt_FromLong (self_bp->bp->ignore_count);
+ return gdb_py_object_from_longest (self_bp->bp->ignore_count).release ();
}
/* Internal function to validate the Python parameters/keywords
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 090705507e6..24d4fae6242 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -165,7 +165,7 @@ frapy_type (PyObject *self, PyObject *args)
GDB_PY_HANDLE_EXCEPTION (except);
}
- return PyInt_FromLong (type);
+ return gdb_py_object_from_longest (type).release ();
}
/* Implementation of gdb.Frame.architecture (self) -> gdb.Architecture.
@@ -209,7 +209,7 @@ frapy_unwind_stop_reason (PyObject *self, PyObject *args)
stop_reason = get_frame_unwind_stop_reason (frame);
- return PyInt_FromLong (stop_reason);
+ return gdb_py_object_from_longest (stop_reason).release ();
}
/* Implementation of gdb.Frame.pc (self) -> Long.
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 45bf51b935c..d0348b5aeff 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1098,11 +1098,11 @@ bootstrap_python_frame_filters (struct frame_info *frame,
if (sort_func == NULL)
return NULL;
- gdbpy_ref<> py_frame_low (PyInt_FromLong (frame_low));
+ gdbpy_ref<> py_frame_low = gdb_py_object_from_longest (frame_low);
if (py_frame_low == NULL)
return NULL;
- gdbpy_ref<> py_frame_high (PyInt_FromLong (frame_high));
+ gdbpy_ref<> py_frame_high = gdb_py_object_from_longest (frame_high);
if (py_frame_high == NULL)
return NULL;
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 669b6d8b074..fec7bcad30f 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -307,10 +307,21 @@ gdbpy_create_ptid_object (ptid_t ptid)
lwp = ptid.lwp ();
tid = ptid.tid ();
- PyTuple_SET_ITEM (ret, 0, PyInt_FromLong (pid));
- PyTuple_SET_ITEM (ret, 1, PyInt_FromLong (lwp));
- PyTuple_SET_ITEM (ret, 2, PyInt_FromLong (tid));
-
+ gdbpy_ref<> pid_obj = gdb_py_object_from_longest (pid);
+ if (pid_obj == nullptr)
+ return nullptr;
+ gdbpy_ref<> lwp_obj = gdb_py_object_from_longest (lwp);
+ if (lwp_obj == nullptr)
+ return nullptr;
+ gdbpy_ref<> tid_obj = gdb_py_object_from_longest (tid);
+ if (tid_obj == nullptr)
+ return nullptr;
+
+ /* Note that these steal references, hence the use of 'release'. */
+ PyTuple_SET_ITEM (ret, 0, pid_obj.release ());
+ PyTuple_SET_ITEM (ret, 1, lwp_obj.release ());
+ PyTuple_SET_ITEM (ret, 2, tid_obj.release ());
+
return ret;
}
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 762c0bcbcc0..15cd15bb0dc 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -246,7 +246,7 @@ recpy_bt_insn_size (PyObject *self, void *closure)
if (insn == NULL)
return NULL;
- return PyInt_FromLong (insn->size);
+ return gdb_py_object_from_longest (insn->size).release ();
}
/* Implementation of RecordInstruction.is_speculative [bool] for btrace.
@@ -342,7 +342,8 @@ recpy_bt_func_level (PyObject *self, void *closure)
return NULL;
tinfo = ((recpy_element_object *) self)->thread;
- return PyInt_FromLong (tinfo->btrace.level + func->level);
+ return gdb_py_object_from_longest (tinfo->btrace.level
+ + func->level).release ();
}
/* Implementation of RecordFunctionSegment.symbol [gdb.Symbol] for btrace.
@@ -566,7 +567,8 @@ btpy_list_count (PyObject *self, PyObject *value)
{
/* We know that if an element is in the list, it is so exactly one time,
enabling us to reuse the "is element of" check. */
- return PyInt_FromLong (btpy_list_contains (self, value));
+ return gdb_py_object_from_longest (btpy_list_contains (self,
+ value)).release ();
}
/* Python rich compare function to allow for equality and inequality checks
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index a6b08dcdd83..38634081224 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -464,7 +464,7 @@ recpy_gap_reason_code (PyObject *self, void *closure)
{
const recpy_gap_object * const obj = (const recpy_gap_object *) self;
- return PyInt_FromLong (obj->reason_code);
+ return gdb_py_object_from_longest (obj->reason_code).release ();
}
/* Implementation of RecordGap.error_string [str]. */
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index d683505c8e5..0adcb0bc9ad 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -131,7 +131,7 @@ sympy_get_addr_class (PyObject *self, void *closure)
SYMPY_REQUIRE_VALID (self, symbol);
- return PyInt_FromLong (SYMBOL_CLASS (symbol));
+ return gdb_py_object_from_longest (SYMBOL_CLASS (symbol)).release ();
}
static PyObject *
@@ -221,7 +221,7 @@ sympy_line (PyObject *self, void *closure)
SYMPY_REQUIRE_VALID (self, symbol);
- return PyInt_FromLong (SYMBOL_LINE (symbol));
+ return gdb_py_object_from_longest (SYMBOL_LINE (symbol)).release ();
}
/* Implementation of gdb.Symbol.is_valid (self) -> Boolean.
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index b0e7618af7e..579662f92d2 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -290,7 +290,7 @@ salpy_get_line (PyObject *self, void *closure)
SALPY_REQUIRE_VALID (self, sal);
- return PyInt_FromLong (sal->line);
+ return gdb_py_object_from_longest (sal->line).release ();
}
static PyObject *
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8825b0749ce..5f09795fd52 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -157,7 +157,7 @@ typy_get_code (PyObject *self, void *closure)
{
struct type *type = ((type_object *) self)->type;
- return PyInt_FromLong (type->code ());
+ return gdb_py_object_from_longest (type->code ()).release ();
}
/* Helper function for typy_fields which converts a single field to a
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index a884d6b70b2..c8e51c14b41 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1860,7 +1860,7 @@ convert_value_from_python (PyObject *obj)
if (PyErr_ExceptionMatches (PyExc_OverflowError))
{
gdbpy_err_fetch fetched_error;
- gdbpy_ref<> zero (PyInt_FromLong (0));
+ gdbpy_ref<> zero = gdb_py_object_from_longest (0);
/* Check whether obj is positive. */
if (PyObject_RichCompareBool (obj, zero.get (), Py_GT) > 0)
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 2c4195f3d03..36c1ab281f8 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -96,7 +96,6 @@
#define Py_TPFLAGS_CHECKTYPES 0
#define PyInt_Check PyLong_Check
-#define PyInt_FromLong PyLong_FromLong
#define PyInt_AsLong PyLong_AsLong
#define PyInt_AsSsize_t PyLong_AsSsize_t
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/7] Standarize int-wrapping approach in Python code
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
` (6 preceding siblings ...)
2020-09-11 14:33 ` [PATCH 7/7] Don't use PyInt_FromLong Tom Tromey
@ 2020-09-12 16:16 ` Joel Brobecker
2020-09-15 17:06 ` Tom Tromey
7 siblings, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2020-09-12 16:16 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi Tom,
On Fri, Sep 11, 2020 at 08:33:02AM -0600, Tom Tromey wrote:
> A while back I fixed a bug where a CORE_ADDR was wrapped for Python
> using PyLong_FromLong, rather than using an unsigned conversion.
>
> After doing this, I thought it would be good to audit the code for
> other oddities. Mostly what I found were possible overflows from code
> that converted specifically to "int" (an issue only for Python 2 I
> suppose).
>
> This series changes the Python code in gdb to prefer two of its own
> functions -- one signed and one unsigned -- which should avoid all
> issues both now and in the future. The idea here is ban PyInt_From*
> and PyLong_From*, and simply always use our own wrappers.
>
> Let me know what you think.
Makes sense to me. I scanned the patch series, and it looked good.
--
Joel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/7] Standarize int-wrapping approach in Python code
2020-09-12 16:16 ` [PATCH 0/7] Standarize int-wrapping approach in Python code Joel Brobecker
@ 2020-09-15 17:06 ` Tom Tromey
0 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-09-15 17:06 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches
>> This series changes the Python code in gdb to prefer two of its own
>> functions -- one signed and one unsigned -- which should avoid all
>> issues both now and in the future. The idea here is ban PyInt_From*
>> and PyLong_From*, and simply always use our own wrappers.
>>
>> Let me know what you think.
Joel> Makes sense to me. I scanned the patch series, and it looked good.
Thanks. I'm going to check this in.
Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-15 17:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 14:33 [PATCH 0/7] Standarize int-wrapping approach in Python code Tom Tromey
2020-09-11 14:33 ` [PATCH 1/7] Don't use PyInt_FromSsize_t Tom Tromey
2020-09-11 14:33 ` [PATCH 2/7] Don't use gdb_py_long_from_longest Tom Tromey
2020-09-11 14:33 ` [PATCH 3/7] Don't use gdb_py_long_from_ulongest Tom Tromey
2020-09-11 14:33 ` [PATCH 4/7] Don't use PyLong_FromLong Tom Tromey
2020-09-11 14:33 ` [PATCH 5/7] Don't use PyLong_FromLongLong Tom Tromey
2020-09-11 14:33 ` [PATCH 6/7] Don't use PyLong_FromUnsignedLong Tom Tromey
2020-09-11 14:33 ` [PATCH 7/7] Don't use PyInt_FromLong Tom Tromey
2020-09-12 16:16 ` [PATCH 0/7] Standarize int-wrapping approach in Python code Joel Brobecker
2020-09-15 17:06 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).