PR#23: Backport f639e6d435cb240c, see [RHELPLAN-41401] - rpms/Cython

rpms / Cython

#23 Backport f639e6d435cb240c, see [RHELPLAN-41401]

Closed 4 years ago by sergesanspaille. Opened 4 years ago by sergesanspaille.

rpms/ sergesanspaille/Cython master into master

Backport f639e6d435cb240c, see [RHELPLAN-41401]

serge-sans-paille • 4 years ago

a91d099

Cython.spec

file modified

+7 -1

		`@@ -6,13 +6,16 @@`
		`Name: Cython`
		`Version: 0.29.19`
		`%global upver %{version_no_tilde %{nil}}`
		`- Release: 1%{?dist}`
		`+ Release: 2%{?dist}`
		`Summary: Language for writing Python extension modules`

		`License: ASL 2.0`
		`URL: http://www.cython.org`
		`Source: https://github.com/cython/cython/archive/%{upver}/%{srcname}-%{version}.tar.gz`

		`+ # Backported from upstream, fixes an undefined behavior and allows clang interoperability`
		`+ Patch2705: f639e6d435cb240c5c21afe91a71915c4af23b7c.patch`
churchyard commented 4 years ago Is there an upstream backport to the 0.29.x branch as well?
		`+`
		`# Partially work around issues with class and static methods`
		`# See https://bugzilla.redhat.com/show_bug.cgi?id=1788506`
		`# Mostly backported from upstream: https://github.com/cython/cython/pull/3106`
		`@@ -118,6 +121,9 @@`
		`%{_emacs_sitestartdir}/cython.el`

		`%changelog`
		`+ * Wed May 27 2020 sguelton@redhat.com - 0.29.19-2`
		`+ - Backport f639e6d435cb240c, see [RHELPLAN-41401]`
		`+`
		`* Wed May 27 2020 sguelton@redhat.com - 0.29.19-1`
		`- Update to 0.29.19`

f639e6d435cb240c5c21afe91a71915c4af23b7c.patch

file added

+60

		`@@ -0,0 +1,60 @@`
		`+ From f639e6d435cb240c5c21afe91a71915c4af23b7c Mon Sep 17 00:00:00 2001`
		`+ From: serge-sans-paille <serge.guelton@telecom-bretagne.eu>`
		`+ Date: Tue, 26 May 2020 18:27:22 +0200`
		`+ Subject: [PATCH] Fix overflow handling for abs() calls on signed integer types`
		`+ (GH-3634)`
		`+`
		`+ Fixes #1911`
		`+ ---`
		`+ Cython/Compiler/ExprNodes.py \| 19 ++++++++++++-------`
		`+ tests/run/builtin_abs.pyx \| 1 +`
		`+ 2 files changed, 13 insertions(+), 7 deletions(-)`
		`+`
		`+ diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py`
		`+ index 48c7f721c7..25e84fd915 100644`
		`+ --- a/Cython/Compiler/ExprNodes.py`
		`+ +++ b/Cython/Compiler/ExprNodes.py`
		`+ @@ -5934,6 +5934,17 @@ def generate_evaluation_code(self, code):`
		`+ if function.is_name or function.is_attribute:`
		`+ code.globalstate.use_entry_utility_code(function.entry)`
		`+`
		`+ + abs_function_cnames = ('abs', 'labs', '__Pyx_abs_longlong')`
		`+ + is_signed_int = self.type.is_int and self.type.signed`
		`+ + if self.overflowcheck and is_signed_int and function.result() in abs_function_cnames:`
		`+ + code.globalstate.use_utility_code(UtilityCode.load_cached("Common", "Overflow.c"))`
		`+ + code.putln('if (unlikely(%s == __PYX_MIN(%s))) {\`
		`+ + PyErr_SetString(PyExc_OverflowError,\`
		`+ + "Trying to take the absolute value of the most negative integer is not defined."); %s; }' % (`
		`+ + self.args[0].result(),`
		`+ + self.args[0].type.empty_declaration_code(),`
		`+ + code.error_goto(self.pos)))`
		`+ +`
		`+ if not function.type.is_pyobject or len(self.arg_tuple.args) > 1 or (`
		`+ self.arg_tuple.args and self.arg_tuple.is_literal):`
		`+ super(SimpleCallNode, self).generate_evaluation_code(code)`
		`+ @@ -6036,13 +6047,7 @@ def generate_result_code(self, code):`
		`+ self.result() if self.type.is_pyobject else None,`
		`+ func_type.exception_value, self.nogil)`
		`+ else:`
		`+ - if (self.overflowcheck`
		`+ - and self.type.is_int`
		`+ - and self.type.signed`
		`+ - and self.function.result() in ('abs', 'labs', '__Pyx_abs_longlong')):`
		`+ - goto_error = 'if (unlikely(%s < 0)) { PyErr_SetString(PyExc_OverflowError, "value too large"); %s; }' % (`
		`+ - self.result(), code.error_goto(self.pos))`
		`+ - elif exc_checks:`
		`+ + if exc_checks:`
		`+ goto_error = code.error_goto_if(" && ".join(exc_checks), self.pos)`
		`+ else:`
		`+ goto_error = ""`
		`+ diff --git a/tests/run/builtin_abs.pyx b/tests/run/builtin_abs.pyx`
		`+ index 21bea7df20..5b35eb886a 100644`
		`+ --- a/tests/run/builtin_abs.pyx`
		`+ +++ b/tests/run/builtin_abs.pyx`
		`+ @@ -1,5 +1,6 @@`
		`+ # mode: run`
		`+ # ticket: 698`
		`+ +# distutils: extra_compile_args=-fwrapv`
		`+`
		`+ cdef extern from *:`
		`+ int INT_MAX`

no initial comment

churchyard commented 4 years ago

What is [RHELPLAN-41401]?

churchyard commented on line 14 of Cython.spec 4 years ago

Is there an upstream backport to the 0.29.x branch as well?

sergesanspaille commented 4 years ago

[RHELPLAN-41401] << https://projects.engineering.redhat.com/browse/RHELPLAN-41401

I don't think upstream plans to backport that patch.

sergesanspaille commented 4 years ago

[RHELPLAN-41401] << https://projects.engineering.redhat.com/browse/RHELPLAN-41401
It's an on-going effort to have package being compilable with both clang and gcc.

I don't think upstream plans to backport that patch.

churchyard commented 4 years ago

I don't feel very comfortable rationalizing changes in Fedora packages with internal Red Hat links.

From the Cython Fedora maintainer PoV, I don't understand why we backport this and upstream does not. Do you ave some details that can be shared publicly?

tstellar commented 4 years ago

It looks to me like the patch was been backported to the 0.29.x: https://github.com/cython/cython/commits/0.29.x

But anyway, there is no specific rationalization here, we just like to submit bug fixes when we have put in the time and effort to fix bugs in upstream packages. There are different policies across the Fedora maintainers about whether a patch needs to be upstream or not, so we just default to creating the pull request and leave it up to the maintainer to decided if the patch should be merged.