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
smime.p7s
Description: S/MIME Cryptographic Signature
|