From 910fd1e3cf973888fb3e0e84646edff7410657ab Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Fri, 5 Aug 2022 19:36:54 +0100 Subject: [PATCH] [D] Fix occasional undefined behaviour with inheritance hierarchies Particularly when using virtual inheritance as the pointers weren't correctly upcast from derived class to base class when stored in the base's proxy class. Fixes commented out test code in cpp11_std_unique_ptr_runme and li_std_auto_ptr_runme D tests. --- CHANGES.current | 5 +++++ Examples/test-suite/cpp11_std_unique_ptr.i | 13 ++++++++++++- .../test-suite/d/cpp11_std_unique_ptr_runme.1.d | 12 ++++++++++-- .../test-suite/d/cpp11_std_unique_ptr_runme.2.d | 12 ++++++++++-- Examples/test-suite/d/li_std_auto_ptr_runme.1.d | 12 ++++++++++-- Examples/test-suite/d/li_std_auto_ptr_runme.2.d | 13 +++++++++++-- Examples/test-suite/li_std_auto_ptr.i | 16 ++++++++++++++-- Source/Modules/d.cxx | 2 +- 8 files changed, 73 insertions(+), 12 deletions(-) diff --git a/CHANGES.current b/CHANGES.current index 2fed37737..bb16fef2d 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,11 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.1.0 (in progress) =========================== +2022-08-05: wsfulton + [D] Fix occasional undefined behaviour with inheritance hierarchies, particularly + when using virtual inheritance as the pointers weren't correctly upcast from derived + class to base class when stored in the base's proxy class. + 2022-08-05: wsfulton [D] Add support for std::unique_ptr in std_unique_ptr.i. Add support for std::auto_ptr in std_auto_ptr.i. diff --git a/Examples/test-suite/cpp11_std_unique_ptr.i b/Examples/test-suite/cpp11_std_unique_ptr.i index 49c85d6aa..0549a7b45 100644 --- a/Examples/test-suite/cpp11_std_unique_ptr.i +++ b/Examples/test-suite/cpp11_std_unique_ptr.i @@ -10,6 +10,7 @@ %inline %{ #include #include +//#include #include "swig_examples_lock.h" class Klass { @@ -53,8 +54,18 @@ struct KlassInheritance : virtual Klass { } }; +std::string useKlassRawPtr(Klass* k) { +// std::cout << "useKlassRawPtr " << std::hex << (Klass*)k << std::endl; + std::string s(k->getLabel()); +// std::cout << "useKlassRawPtr string: " << s << std::endl; + return s; +} + std::string takeKlassUniquePtr(std::unique_ptr k) { - return std::string(k->getLabel()); +// std::cout << "takeKlassUniquePtr " << std::hex << (Klass*)k.get() << std::endl; + std::string s(k->getLabel()); +// std::cout << "takeKlassUniquePtr string: " << s << std::endl; + return s; } bool is_nullptr(Klass *p) { diff --git a/Examples/test-suite/d/cpp11_std_unique_ptr_runme.1.d b/Examples/test-suite/d/cpp11_std_unique_ptr_runme.1.d index 7730f255f..5aee10727 100644 --- a/Examples/test-suite/d/cpp11_std_unique_ptr_runme.1.d +++ b/Examples/test-suite/d/cpp11_std_unique_ptr_runme.1.d @@ -13,6 +13,16 @@ void checkCount(int expected_count) { } void main() { + // Test raw pointer handling involving virtual inheritance + { + scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); + checkCount(1); + string s = useKlassRawPtr(kini); + if (s != "KlassInheritanceInput") + throw new Exception("Incorrect string: " ~ s); + } + checkCount(0); + // unique_ptr as input { scope Klass kin = new Klass("KlassInput"); @@ -65,7 +75,6 @@ void main() { } checkCount(0); - /* { scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); checkCount(1); @@ -77,7 +86,6 @@ void main() { throw new Exception("is_nullptr failed"); } // dispose should not fail, even though already deleted checkCount(0); - */ // unique_ptr as output Klass k1 = makeKlassUniquePtr("first"); diff --git a/Examples/test-suite/d/cpp11_std_unique_ptr_runme.2.d b/Examples/test-suite/d/cpp11_std_unique_ptr_runme.2.d index 7730f255f..5aee10727 100644 --- a/Examples/test-suite/d/cpp11_std_unique_ptr_runme.2.d +++ b/Examples/test-suite/d/cpp11_std_unique_ptr_runme.2.d @@ -13,6 +13,16 @@ void checkCount(int expected_count) { } void main() { + // Test raw pointer handling involving virtual inheritance + { + scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); + checkCount(1); + string s = useKlassRawPtr(kini); + if (s != "KlassInheritanceInput") + throw new Exception("Incorrect string: " ~ s); + } + checkCount(0); + // unique_ptr as input { scope Klass kin = new Klass("KlassInput"); @@ -65,7 +75,6 @@ void main() { } checkCount(0); - /* { scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); checkCount(1); @@ -77,7 +86,6 @@ void main() { throw new Exception("is_nullptr failed"); } // dispose should not fail, even though already deleted checkCount(0); - */ // unique_ptr as output Klass k1 = makeKlassUniquePtr("first"); diff --git a/Examples/test-suite/d/li_std_auto_ptr_runme.1.d b/Examples/test-suite/d/li_std_auto_ptr_runme.1.d index 76c81ffc8..3b51ba3ce 100644 --- a/Examples/test-suite/d/li_std_auto_ptr_runme.1.d +++ b/Examples/test-suite/d/li_std_auto_ptr_runme.1.d @@ -13,6 +13,16 @@ void checkCount(int expected_count) { } void main() { + // Test raw pointer handling involving virtual inheritance + { + scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); + checkCount(1); + string s = useKlassRawPtr(kini); + if (s != "KlassInheritanceInput") + throw new Exception("Incorrect string: " ~ s); + } + checkCount(0); + // auto_ptr as input { scope Klass kin = new Klass("KlassInput"); @@ -65,7 +75,6 @@ void main() { } checkCount(0); - /* { scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); checkCount(1); @@ -77,7 +86,6 @@ void main() { throw new Exception("is_nullptr failed"); } // dispose should not fail, even though already deleted checkCount(0); - */ // auto_ptr as output Klass k1 = makeKlassAutoPtr("first"); diff --git a/Examples/test-suite/d/li_std_auto_ptr_runme.2.d b/Examples/test-suite/d/li_std_auto_ptr_runme.2.d index 76c81ffc8..38efa3d7d 100644 --- a/Examples/test-suite/d/li_std_auto_ptr_runme.2.d +++ b/Examples/test-suite/d/li_std_auto_ptr_runme.2.d @@ -13,6 +13,17 @@ void checkCount(int expected_count) { } void main() { + // Test raw pointer handling involving virtual inheritance + { + scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); + checkCount(1); + string s = useKlassRawPtr(kini); + checkCount(1); + if (s != "KlassInheritanceInput") + throw new Exception("Incorrect string: " ~ s); + } + checkCount(0); + // auto_ptr as input { scope Klass kin = new Klass("KlassInput"); @@ -65,7 +76,6 @@ void main() { } checkCount(0); - /* { scope KlassInheritance kini = new KlassInheritance("KlassInheritanceInput"); checkCount(1); @@ -77,7 +87,6 @@ void main() { throw new Exception("is_nullptr failed"); } // dispose should not fail, even though already deleted checkCount(0); - */ // auto_ptr as output Klass k1 = makeKlassAutoPtr("first"); diff --git a/Examples/test-suite/li_std_auto_ptr.i b/Examples/test-suite/li_std_auto_ptr.i index afa4782c3..5f3b7724f 100644 --- a/Examples/test-suite/li_std_auto_ptr.i +++ b/Examples/test-suite/li_std_auto_ptr.i @@ -15,6 +15,7 @@ #if defined(SWIGCSHARP) || defined(SWIGJAVA) || defined(SWIGPYTHON) || defined(SWIGRUBY) || defined(SWIGPERL) || defined(SWIGTCL) || defined(SWIGOCTAVE) || defined(SWIGJAVASCRIPT) || defined(SWIGD) %include "std_string.i" +//#include %include "std_auto_ptr.i" %auto_ptr(Klass) @@ -100,10 +101,21 @@ struct KlassInheritance : virtual Klass { } }; -std::string takeKlassAutoPtr(std::auto_ptr k) { - return std::string(k->getLabel()); +std::string useKlassRawPtr(Klass* k) { +// std::cout << "useKlassRawPtr " << std::hex << (Klass*)k << std::endl; + std::string s(k->getLabel()); +// std::cout << "useKlassRawPtr string: " << s << std::endl; + return s; } +std::string takeKlassAutoPtr(std::auto_ptr k) { +// std::cout << "takeKlassAutoPtr " << std::hex << (Klass*)k.get() << std::endl; + std::string s(k->getLabel()); +// std::cout << "takeKlassAutoPtr string: " << s << std::endl; + return s; +} + + bool is_nullptr(Klass *p) { return p == 0; } diff --git a/Source/Modules/d.cxx b/Source/Modules/d.cxx index ff32dbf0b..c1f68da07 100644 --- a/Source/Modules/d.cxx +++ b/Source/Modules/d.cxx @@ -3402,7 +3402,7 @@ private: } else { Printv(upcasts_code, "SWIGEXPORT ", baseclassname, " * ", upcast_wrapper_name, - "(", baseclassname, " *objectRef) {\n", + "(", classname, " *objectRef) {\n", " return (", baseclassname, " *)objectRef;\n" "}\n", "\n", NIL);