libftdi Archives

Subject: Re: [PATCH] Use a separate Py_ssize_t var instead of casting an int pointer as a Py_ssize_t pointer.

From: Jordan Rupprecht <rupprecht@xxxxxxxxxx>
To: thomas.jarosch@xxxxxxxxxxxxx
Cc: libftdi@xxxxxxxxxxxxxxxxxxxxxxx
Date: Mon, 5 Nov 2018 15:01:51 -0800
On Mon, Nov 5, 2018 at 1:28 PM Thomas Jarosch
<thomas.jarosch@xxxxxxxxxxxxx> wrote:
>
> Hi Jordan,
>
> On 9/20/18 8:05 PM, Jordan Rupprecht wrote:
> > Casting is fine if Py_ssize_t == int, but not when Py_ssize_t == long.
> >
> > Signed-off-by: Jordan Rupprecht <rupprecht@xxxxxxxxxx>
> > ---
> >  python/ftdi1.i | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/ftdi1.i b/python/ftdi1.i
> > index 93793f8..d53ebb0 100644
> > --- a/python/ftdi1.i
> > +++ b/python/ftdi1.i
> > @@ -22,11 +22,13 @@ inline PyObject* charp2str(const char *v_, long len)
> >  inline char * str2charp_size(PyObject* pyObj, int * size)
> >  {
> >    char * v_ = 0;
> > +  Py_ssize_t len = 0;
> >  #if PY_MAJOR_VERSION >= 3
> > -  PyBytes_AsStringAndSize(pyObj, &v_, (Py_ssize_t*)size);
> > +  PyBytes_AsStringAndSize(pyObj, &v_, &len);
> >  #else
> > -  PyString_AsStringAndSize(pyObj, &v_, (Py_ssize_t*)size);
> > +  PyString_AsStringAndSize(pyObj, &v_, &len);
> >  #endif
> > +  *size = len;
> >    return v_;
> >  }
> >  %}
>
> patch applied, thanks Jordan!
Thanks!

>
> Were you hit by this during runtime?
Yep. It was caught in a unit test, but only with some very specific
flags (I think it was a combination of -O3 and -fPIE, among other
things).

I'm not actually familiar with the details of the test itself, but we
observed a strange segfault when bumping up the version of clang that
we use. We found the root cause for a few other failures we had (and
I'm not sure that we confirmed the libftdi error had the same root
cause, but it's likely) to a change that reduced unnecessary padding
around certain pieces of data. Usually the problems with the code were
a little more obvious, like:
const char foo[] = {'f', 'o', 'o'};
int bar() { return strlen(foo); }
Which previously returned 3 because there was "accidental" padding,
but now returns a much larger number depending on how long it takes to
get to null bytes in memory...
Anyway, I imagine something similar could be the explanation of doing
a C-style cast on different types here. But to be honest, once the
test started working again, I didn't bother digging too much :)

The llvm revision that gave us other failures, and probably this one,
is https://reviews.llvm.org/rL342053.

>
>
> @all: One patch from Uwe Bonnes and one from Eric L. Schott are still
> outstanding. I have to take care of them on another day,
> but they won't be forgotten.
>
> Cheers,
> Thomas

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Current Thread