Fix Ruby opaque pointer handling (bug #1299502)

This commit is contained in:
Jitka Plesnikova 2016-05-25 16:52:28 +02:00
parent 0600de995e
commit 38ebab1998
2 changed files with 221 additions and 3 deletions

View File

@ -17,7 +17,6 @@
%{!?Rlang:%global Rlang 0}
%else
%{!?golang:%global golang 1}
# R tests failed (since 3.0.4)
%{!?Rlang:%global Rlang 1}
%endif
%{!?javalang:%global javalang 1}
@ -32,7 +31,7 @@
Summary: Connects C/C++/Objective C to some high-level programming languages
Name: swig
Version: 3.0.8
Release: 6%{?dist}
Release: 7%{?dist}
License: GPLv3+ and BSD
URL: http://swig.sourceforge.net/
Source0: http://downloads.sourceforge.net/project/swig/swig/swig-%{version}/swig-%{version}.tar.gz
@ -49,7 +48,7 @@ Patch0: swig308-Fix-li_boost_array-test.patch
Patch1: swig308-Do-not-use-isystem.patch
Patch2: swig308-Python-Use-std-isfinite-under-C-11.patch
Patch3: swig308-Add-support-Go-1_6.patch
Patch4: swig308-Ruby-opaque-pointer-handling-regression-fix.patch
BuildRequires: perl, python2-devel, pcre-devel
BuildRequires: autoconf, automake, gawk, dos2unix
@ -131,6 +130,7 @@ This package contains documentation for SWIG and useful examples
%patch1 -p1 -b .isystem
%patch2 -p1 -b .isfinite
%patch3 -p1
%patch4 -p1 -b .ruby
for all in CHANGES README; do
iconv -f ISO88591 -t UTF8 < $all > $all.new
@ -266,6 +266,9 @@ install -pm 644 %{SOURCE3} %{SOURCE4} %{buildroot}%{_sysconfdir}/profile.d
%doc Doc Examples COPYRIGHT
%changelog
* Wed May 25 2016 Jitka Plesnikova <jplesnik@redhat.com> - 3.0.8-7
- Fix Ruby opaque pointer handling (bug #1299502)
* Mon Apr 18 2016 Jitka Plesnikova <jplesnik@redhat.com> - 3.0.8-6
- Add support for Go 1.6

View File

@ -0,0 +1,215 @@
From 763827c2e1fffd34e871c66d5d4c9e492c1cecec Mon Sep 17 00:00:00 2001
From: William S Fulton <wsf@fultondesigns.co.uk>
Date: Tue, 24 May 2016 07:33:25 +0100
Subject: [PATCH] Ruby opaque pointer handling regression fix
This bug was introduced in swig-3.0.8 in #146252 adding shared_ptr
support. An ObjectPreviouslyDeleted error was incorrectly thrown
when the pointer was used as a parameter after being set to zero
via a call to 'DATA_PTR(self) = 0'.
It isn't clear to me which approach is better in this corner case,
so I've gone for backwards compatibility and restored the old behaviour.
Closes #602
---
CHANGES.current | 6 ++
Examples/test-suite/ruby/Makefile.in | 3 +-
.../test-suite/ruby/ruby_manual_proxy_runme.rb | 49 ++++++++++++++++
Examples/test-suite/ruby_manual_proxy.i | 66 ++++++++++++++++++++++
Lib/ruby/rubyrun.swg | 12 ++--
5 files changed, 131 insertions(+), 5 deletions(-)
create mode 100644 Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
create mode 100644 Examples/test-suite/ruby_manual_proxy.i
#diff --git a/CHANGES.current b/CHANGES.current
#index 6e347a7..f754e96 100644
#--- a/CHANGES.current
#+++ b/CHANGES.current
#@@ -5,6 +5,12 @@ See the RELEASENOTES file for a summary of changes in each release.
# Version 3.0.9 (in progress)
# ===========================
#
#+2016-05-23: wsfulton
#+ [Ruby] Fix #602 - Error handling regression of opaque pointers introduced
#+ in swig-3.0.8 when C functions explicitly reset a pointer using 'DATA_PTR(self) = 0'.
#+ An ObjectPreviouslyDeleted error was incorrectly thrown when the pointer was used
#+ as a parameter.
#+
# 2016-05-17: tamuratak
# [Ruby] Patch #651 - Correct overloaded function error message when function is
# using %newobject.
diff --git a/Examples/test-suite/ruby/Makefile.in b/Examples/test-suite/ruby/Makefile.in
index 847c959..d94ac70 100644
--- a/Examples/test-suite/ruby/Makefile.in
+++ b/Examples/test-suite/ruby/Makefile.in
@@ -32,7 +32,8 @@ CPP_TEST_CASES = \
C_TEST_CASES += \
li_cdata \
- li_cstring
+ li_cstring \
+ ruby_manual_proxy \
include $(srcdir)/../common.mk
diff --git a/Examples/test-suite/ruby/ruby_manual_proxy_runme.rb b/Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
new file mode 100644
index 0000000..c1cee2d
--- /dev/null
+++ b/Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
@@ -0,0 +1,49 @@
+#!/usr/bin/env ruby
+#
+# The Subversion bindings use this manually written proxy class approach
+# to the Ruby bindings. Note that in C the struct svn_fs_t is an
+# opaque pointer and the Ruby FileSystem proxy class is hand written around it.
+# This testcase tests this and the C close function and subsequent error
+# handling.
+
+require 'swig_assert'
+require 'ruby_manual_proxy'
+
+module Svn
+ module Fs
+ module_function
+ def create(path)
+ f = Ruby_manual_proxy::svn_fs_create(path)
+ return f
+ end
+
+ FileSystem = SWIG::TYPE_p_svn_fs_t
+ class FileSystem
+ class << self
+ def create(*args)
+ Fs.create(*args)
+ end
+ end
+ def path
+ Ruby_manual_proxy::svn_fs_path(self)
+ end
+ end
+ end
+end
+
+f = Svn::Fs::FileSystem.create("/tmp/myfile")
+path = f.path
+f.close
+begin
+ # regression in swig-3.0.8 meant ObjectPreviouslyDeleted error was thrown instead
+ path = f.path
+ raise RuntimeError.new("IOError (1) not thrown")
+rescue IOError
+end
+
+file = nil
+begin
+ path = Ruby_manual_proxy::svn_fs_path(file)
+ raise RuntimeError.new("IOError (2) not thrown")
+rescue IOError
+end
diff --git a/Examples/test-suite/ruby_manual_proxy.i b/Examples/test-suite/ruby_manual_proxy.i
new file mode 100644
index 0000000..2cb154e
--- /dev/null
+++ b/Examples/test-suite/ruby_manual_proxy.i
@@ -0,0 +1,66 @@
+%module ruby_manual_proxy
+
+
+%typemap(in, numinputs=0) SWIGTYPE ** ($*1_ltype temp) "$1 = &temp;";
+
+%typemap(argout) SWIGTYPE **OUTPARAM {
+ $result = SWIG_Ruby_AppendOutput($result, SWIG_NewPointerObj(*$1, $*1_descriptor, 0));
+}
+
+%apply SWIGTYPE **OUTPARAM {
+ svn_fs_t **
+};
+
+%typemap(check) svn_fs_t * {
+ if (!$1) {
+ svn_swig_rb_raise_svn_fs_already_close();
+ }
+}
+
+%{
+typedef struct svn_fs_t {
+ char path[256];
+} svn_fs_t;
+
+void svn_fs_create(svn_fs_t **fs_p, const char *path) {
+ svn_fs_t *fs = (svn_fs_t *)malloc(sizeof(svn_fs_t));
+ strncpy(fs->path, path, 256);
+ *fs_p = fs;
+}
+const char *svn_fs_path(svn_fs_t *fs) {
+ return fs->path;
+}
+%}
+
+typedef struct svn_fs_t svn_fs_t;
+void svn_fs_create(svn_fs_t **fs_p, const char *path);
+const char *svn_fs_path(svn_fs_t *fs);
+
+%{
+static void svn_swig_rb_raise_svn_fs_already_close(void) {
+ rb_raise(rb_eIOError, "already closed");
+}
+
+static VALUE svn_fs_swig_rb_close(VALUE self) {
+ if (!DATA_PTR(self)) {
+ svn_swig_rb_raise_svn_fs_already_close();
+ }
+
+ DATA_PTR(self) = NULL;
+
+ return Qnil;
+}
+
+static VALUE svn_fs_swig_rb_closed(VALUE self) {
+ return DATA_PTR(self) ? Qfalse : Qtrue;
+}
+%}
+
+%insert("init") %{
+ {
+ VALUE cSvnfs;
+ cSvnfs = rb_const_get(_mSWIG, rb_intern("TYPE_p_svn_fs_t"));
+ rb_define_method(cSvnfs, "close",
+ VALUEFUNC(svn_fs_swig_rb_close), 0);
+ }
+%}
diff --git a/Lib/ruby/rubyrun.swg b/Lib/ruby/rubyrun.swg
index e18208f..249494a 100644
--- a/Lib/ruby/rubyrun.swg
+++ b/Lib/ruby/rubyrun.swg
@@ -305,6 +305,14 @@ SWIG_Ruby_ConvertPtrAndOwn(VALUE obj, void **ptr, swig_type_info *ty, int flags,
/* Do type-checking if type info was provided */
if (ty) {
+ if (ty->clientdata) {
+ if (rb_obj_is_kind_of(obj, ((swig_class *) (ty->clientdata))->klass)) {
+ if (vptr == 0) {
+ /* The object has already been deleted */
+ return SWIG_ObjectPreviouslyDeletedError;
+ }
+ }
+ }
if ((c = SWIG_MangleStr(obj)) == NULL) {
return SWIG_ERROR;
}
@@ -312,10 +320,6 @@ SWIG_Ruby_ConvertPtrAndOwn(VALUE obj, void **ptr, swig_type_info *ty, int flags,
if (!tc) {
return SWIG_ERROR;
} else {
- if (vptr == 0) {
- /* The object has already been deleted */
- return SWIG_ObjectPreviouslyDeletedError;
- }
if (ptr) {
if (tc->type == ty) {
*ptr = vptr;
--
2.5.5