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

Cython incorrectly casts with ctypedef and bint #3066

Open
mbuesch opened this issue Aug 2, 2019 · 3 comments

Comments

@mbuesch
Copy link
Contributor

commented Aug 2, 2019

Given the following code:

ctypedef bint mybool

cdef f():
        cdef mybool c
        cdef mybool x
        c = True
        x = True
        x = not x if c else x
        print(x)

Results in this C code:

/* "test.pyx":3
 * ctypedef bint mybool
 * 
 * cdef f():             # <<<<<<<<<<<<<<
 * 	cdef mybool c
 * 	cdef mybool x
 */

static PyObject *__pyx_f_4test_f(void) {
  __pyx_t_4test_mybool __pyx_v_c;
  __pyx_t_4test_mybool __pyx_v_x;
  PyObject *__pyx_r = NULL;
  __Pyx_RefNannyDeclarations
  PyObject *__pyx_t_1 = NULL;
  PyObject *__pyx_t_2 = NULL;
  __Pyx_RefNannySetupContext("f", 0);

  /* "test.pyx":6
 * 	cdef mybool c
 * 	cdef mybool x
 * 	c = True             # <<<<<<<<<<<<<<
 * 	x = True
 * 	x = not x if c else x
 */
  __pyx_v_c = 1;

  /* "test.pyx":7
 * 	cdef mybool x
 * 	c = True
 * 	x = True             # <<<<<<<<<<<<<<
 * 	x = not x if c else x
 * 	print(x)
 */
  __pyx_v_x = 1;

  /* "test.pyx":8
 * 	c = True
 * 	x = True
 * 	x = not x if c else x             # <<<<<<<<<<<<<<
 * 	print(x)
 */
  if ((__pyx_v_c != 0)) {
    __pyx_t_1 = ((PyObject *)(!(__pyx_v_x != 0)));
  } else {
    __pyx_t_1 = ((PyObject *)__pyx_v_x);
  }
  __pyx_v_x = ((__pyx_t_4test_mybool)__pyx_t_1);
  __pyx_t_1 = 0;

  /* "test.pyx":9
 * 	x = True
 * 	x = not x if c else x
 * 	print(x)             # <<<<<<<<<<<<<<
 */
  __pyx_t_1 = __Pyx_PyBool_FromLong(__pyx_v_x); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 9, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_t_2 = __Pyx_PyObject_CallOneArg(__pyx_builtin_print, __pyx_t_1); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 9, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_2);
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
  __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;

  /* "test.pyx":3
 * ctypedef bint mybool
 * 
 * cdef f():             # <<<<<<<<<<<<<<
 * 	cdef mybool c
 * 	cdef mybool x
 */

  /* function exit code */
  __pyx_r = Py_None; __Pyx_INCREF(Py_None);
  goto __pyx_L0;
  __pyx_L1_error:;
  __Pyx_XDECREF(__pyx_t_1);
  __Pyx_XDECREF(__pyx_t_2);
  __Pyx_AddTraceback("test.f", __pyx_clineno, __pyx_lineno, __pyx_filename);
  __pyx_r = 0;
  __pyx_L0:;
  __Pyx_XGIVEREF(__pyx_r);
  __Pyx_RefNannyFinishContext();
  return __pyx_r;
}

Which throws these compiler warnings:

$ gcc -shared -pthread -fPIC -fwrapv -O2 -Wall -fno-strict-aliasing -I/usr/include/python3.7 -o test test.c 
test.c: In function ‘__pyx_f_4test_f’:
test.c:1136:18: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     __pyx_t_1 = ((PyObject *)(!(__pyx_v_x != 0)));
                  ^
test.c:1138:18: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     __pyx_t_1 = ((PyObject *)__pyx_v_x);
                  ^
test.c:1140:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   __pyx_v_x = ((__pyx_t_4test_mybool)__pyx_t_1);

The C boolean variable __pyx_v_x is incorrectly casted to PyObject * and back to bool.

This does not happen, if bint is used directly in the cdefs.

@scoder

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

Yes, that looks wrong. I guess we're too strict about special-casing bint, and should also include typedefs there. PR welcome that fixes up at least some of those places. Look for c_bint_type, mostly.

@nsmanville

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

I would like to take a stab at this issue, I may need some help though @scoder.

  1. Correct me if I'm wrong, but independent_spanning_type() in PyrexTypes.py is where I should make the change? Maybe something like calling .resolve() on each type before comparing them to each other?
  2. When writing a test for this case, how do I check that the value is not being cast to PyObject? If you could point me to an existing test that does something similar, that would help a lot I think.
@scoder

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

how do I check that the value is not being cast to PyObject

You could add # distutils: extra_compile_args=-Werror. Doesn't look like that's done anywhere else yet, but it should make the C compilation fail if there are any warnings.

Maybe something like calling .resolve() on each type before comparing

That doesn't sound wrong, at least. Let's see if it solves the issue at hand.

nsmanville added a commit to nsmanville/cython that referenced this issue Aug 19, 2019

Add ctypedef bint test
Check that Cython does not incorrectly cast with ctypedef and bint.

Issue: cython#3066
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.