bugGNU Octave - Bugs: bug #60567, oct-file has an undefined symbol

 
 

bug #60567: oct-file has an undefined symbol

Submitter:  Robert Jenssen <morgawr>
Submitted:  Sun 09 May 2021 11:01:25 PM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Robert Jenssen Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 12 May 2021 04:11:13 PM UTC, comment #11: 
John W. Eaton <jwe>
Group administrator
Tue 11 May 2021 04:42:32 PM UTC, comment #10: 

Sounds good to me.

Closing again.

Markus Mützel <mmuetzel>
Group administrator
Tue 11 May 2021 02:08:25 PM UTC, comment #9: 

We can keep the patch for now.  If the problem reported here is fixed we can also close this report.  I'd just like to avoid adding and trying to maintain tags for every individual function, so before we spend a lot of time tagging more functions let's discuss the best way forward on discourse.

John W. Eaton <jwe>
Group administrator
Tue 11 May 2021 01:58:21 PM UTC, comment #8: 

Tbh, I don't understand completely how visibility is implemented by different compilers and vendors on different platforms. It's mostly trial and error for me.
I saw that adding those flags didn't break anything with gcc and clang on Ubuntu, and cross-compiling with gcc for Windows still worked. They also seem to have fixed the motivating undefined symbol issue. So, I went ahead and made those changes. (It might have been better to post the patch here and wait for feedback.)
The changes also seem to have allowed proceeding a little bit further in the build process with Apple clang on macOS (see bug #59820).

But I agree that it might be preferable to try to apply those flags more minimalisticly.

This change in CRowVector.h might have sufficed to resolve the motivating error:

-  friend ComplexRowVector operator * (const ComplexRowVector& a,
-                                      const ComplexMatrix& b);
+  friend OCTAVE_API ComplexRowVector
+  operator * (const ComplexRowVector& a, const ComplexMatrix& b);

-  friend ComplexRowVector operator * (const RowVector& a,
-                                      const ComplexMatrix& b);
+  friend OCTAVE_API ComplexRowVector
+  operator * (const RowVector& a, const ComplexMatrix& b);


It involves friend operator functions like you wrote. Should we add the flags only to friend functions?

I don't mind at all reverting 3e8bc8bee8e9.

Opening a discourse thread might be a good idea. It sounds like you already know what to do. But we can still ask for further input if you prefer.

Markus Mützel <mmuetzel>
Group administrator
Tue 11 May 2021 01:28:35 PM UTC, comment #7: 

Prior to 3e8bc8bee8e9, the function ComplexRowVector::extract was also not tagged individually but I was able to modify the source for the .oct file as follows and executing the resulting .oct file works for me:


--- complex_matrix.cc.orig        2021-05-11 09:01:33.130956833 -0400
+++ complex_matrix.cc        2021-05-11 09:00:52.691481265 -0400
@@ -59,8 +59,14 @@
       argval(0)=zIminusA;
       Rval=octave::feval("inv",argval,1);
       ComplexMatrix R=Rval(0).complex_matrix_value();
-
-      ComplexRowVector cR(c*R);
+
+      ComplexRowVector cR (R.columns ());
+      for (octave_idx_type ii = 0; ii < cR.numel (); ii++)
+        cR(ii) = Complex (ii, ii+10);
+      ComplexRowVector tmp = cR.extract (0, 1);
+      octave_stdout << "cR.numel (): " << cR.numel () << std::endl;
+      octave_stdout << "tmp(0): " << tmp(0) << std::endl;
+      octave_stdout << "tmp(1): " << tmp(1) << --- complex_matrix.cc.orig        2021-05-11 09:01:33.130956833 -0400
+++ complex_matrix.cc        2021-05-11 09:00:52.691481265 -0400
@@ -59,8 +59,14 @@
       argval(0)=zIminusA;
       Rval=octave::feval("inv",argval,1);
       ComplexMatrix R=Rval(0).complex_matrix_value();
-
-      ComplexRowVector cR(c*R);
+
+      ComplexRowVector cR (R.columns ());
+      for (octave_idx_type ii = 0; ii < cR.numel (); ii++)
+        cR(ii) = Complex (ii, ii+10);
+      ComplexRowVector tmp = cR.extract (0, 1);
+      octave_stdout << "cR.numel (): " << cR.numel () << std::endl;
+      octave_stdout << "tmp(0): " << tmp(0) << std::endl;
+      octave_stdout << "tmp(1): " << tmp(1) << std::endl;
       Complex cRb(cR*b);
       H(l)=cRb+d;
     }
std::endl;
       Complex cRb(cR*b);
       H(l)=cRb+d;
     }


Is the real problem with the friend function declaration?  If I understand correctly, that just declares that the function is a friend of the class (and can access private class data) but there should also be a declaration outside the class definition.  And there, we would need an API tag.  But member functions should be visible because of the API tag on the class itself.

I suspect that most of these friend operator functions don't really need to be declared as friends (they probably don't need access to private class data or functions) and we could just have extern declarations outside the class scope.  And if we had them all in namespaces, then I think we could just apply the visibility tag to the namespace in the header file and not have to repeat the visibility tag on each declaration.

If we should discuss further I can open a discourse topic.

John W. Eaton <jwe>
Group administrator
Tue 11 May 2021 12:03:47 PM UTC, comment #6: 

The classes were already tagged as visible.
But that doesn't seem to apply to each member function (as I had expected).

Markus Mützel <mmuetzel>
Group administrator
Tue 11 May 2021 11:37:40 AM UTC, comment #5: 

It seems to me that only in rare cases should we be limiting visibility of class methods and functions that are declared in public header files.  So instead of tagging individual functions in public header files, I think it would be better to tag the classes (or namespaces) as public in those files.

John W. Eaton <jwe>
Group administrator
Tue 11 May 2021 11:25:59 AM UTC, comment #4: 

Thanks for testing.

The buildbot builders and GitHub runners also seem to be ok with this change.

Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Tue 11 May 2021 10:42:51 AM UTC, comment #3: 

Works for me.

Robert Jenssen <morgawr>
Tue 11 May 2021 09:17:28 AM UTC, comment #2: 

Thanks for the report and the example to reproduce.

I can confirm this issue with a build from a recent default branch on Linux and Windows.

The following change adds visibility flags to more function declarations in liboctave/array headers:
https://hg.savannah.gnu.org/hgweb/octave/rev/3e8bc8bee8e9

With those changes, your original example compiles and runs for me.

I wonder if this will also reduce the number of undefined symbols when using visibility flags with Apple clang (see bug #59820).

Markus Mützel <mmuetzel>
Group administrator
Sun 09 May 2021 11:06:20 PM UTC, comment #1: 

I missed setting the path for the m-file version in complex_matrix_test.sh but that makes no difference to the problem.

(file #51412)

Robert Jenssen <morgawr>
Sun 09 May 2021 11:01:25 PM UTC, original submission:  

Running complex_matrix_test.sh with octave-6.2.0 works as expected.

Running complex_matrix_test.sh with octave-7.0.0:
$ sh complex_matrix_test.sh > out.7.0.0  2>&1
$ cat out.7.0.0
+ OCTAVE_VER=7.0.0
+ ls
complex_matrix.cc
complex_matrix.m
complex_matrix_test.m
complex_matrix_test.sh
out.7.0.0
+ /usr/local/octave-7.0.0/bin/octave --no-gui --eval ver
----------------------------------------------------------------------
GNU Octave Version: 7.0.0 (hg id: 1b945016d837)
GNU Octave License: GNU General Public License
Operating System: Linux 5.11.18-300.fc34.x86_64 #1 SMP Mon May 3 15:10:32 UTC 2021 x86_64
----------------------------------------------------------------------
Package Name  | Version | Installation directory
--------------+---------+-----------------------
     control  |   3.2.0 | .../octave-7.0.0/share/octave/packages/control-3.2.0
          io  |   2.6.3 | /usr/local/octave-7.0.0/share/octave/packages/io-2.6.3
       optim  |   1.6.1 | .../octave-7.0.0/share/octave/packages/optim-1.6.1
      signal  |   1.4.1 | .../octave-7.0.0/share/octave/packages/signal-1.4.1
  statistics  |   1.4.2 | .../share/octave/packages/statistics-1.4.2
      struct  |  1.0.17 | .../octave-7.0.0/share/octave/packages/struct-1.0.17
    symbolic  |   2.9.0 | .../octave-7.0.0/share/octave/packages/symbolic-2.9.0
+ rm complex_matrix.oct
rm: cannot remove 'complex_matrix.oct': No such file or directory
+ octave --no-gui complex_matrix_test.m
ans = 0
warning: Using Octave m-file version of function complex_matrix()!
warning: called from
    complex_matrix at line 4 column 3
    complex_matrix_test at line 11 column 3

H = -3.9650e-05 - 7.0719e-01i
+ /usr/local/octave-7.0.0/bin/mkoctfile -v -Wall -Werror complex_matrix.cc
g++ -c  -fPIC -I/usr/local/octave-7.0.0/include/octave-7.0.0/octave/.. -I/usr/local/octave-7.0.0/include/octave-7.0.0/octave -I/usr/local/octave-7.0.0/include  -pthread -fopenmp -m64 -march=nehalem -O2 -std=c++11 -I/usr/local/octave-7.0.0/include  -Wall -Werror   complex_matrix.cc -o /tmp/oct-dxexzT.o
g++ -I/usr/local/octave-7.0.0/include/octave-7.0.0/octave/.. -I/usr/local/octave-7.0.0/include/octave-7.0.0/octave -I/usr/local/octave-7.0.0/include  -pthread -fopenmp -m64 -march=nehalem -O2 -std=c++11 -I/usr/local/octave-7.0.0/include  -Wall -Werror -o complex_matrix.oct  /tmp/oct-dxexzT.o  -shared -Wl,-Bsymbolic  -L/usr/local/octave-7.0.0/lib -shared -Wl,-Bsymbolic   -L/usr/local/octave-7.0.0/lib -shared -Wl,-Bsymbolic  -L/usr/local/octave-7.0.0/lib
+ ls
complex_matrix.cc
complex_matrix.m
complex_matrix.oct
complex_matrix_test.m
complex_matrix_test.sh
out.7.0.0
+ /usr/local/octave-7.0.0/bin/octave --no-gui complex_matrix_test.m
ans = 0
error: /home/robj/TMP/complex_matrix_test/complex_matrix.oct: failed to load: /home/robj/TMP/complex_matrix_test/complex_matrix.oct: undefined symbol: _ZmlRK16ComplexRowVectorRK13ComplexMatrix
error: called from
    complex_matrix_test at line 11 column 3
$

Robert Jenssen <morgawr>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #51412:  complex_matrix_test.sh added by morgawr (352B - application/x-shellscript)
file #51408:  complex_matrix.cc added by morgawr (2KiB - text/x-c++src - Test files showing undefined symbol)
file #51409:  complex_matrix.m added by morgawr (709B - text/x-objcsrc - Test files showing undefined symbol)
file #51410:  complex_matrix_test.m added by morgawr (223B - text/x-objcsrc - Test files showing undefined symbol)
file #51411:  complex_matrix_test.sh added by morgawr (318B - application/x-shellscript - Test files showing undefined symbol)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by morgawr (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-05-11 mmuetzel StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2021-05-11 mmuetzel StatusFixed In Progress
        Open/ClosedClosed Open
    2021-05-11 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-05-11 mmuetzel StatusNone Ready For Test
    2021-05-11 mmuetzel Operating SystemGNU/Linux Any
    2021-05-09 morgawr Attached File- Added complex_matrix_test.sh, #51412
    2021-05-09 morgawr Attached File- Added complex_matrix.cc, #51408
        Attached File- Added complex_matrix.m, #51409
        Attached File- Added complex_matrix_test.m, #51410
        Attached File- Added complex_matrix_test.sh, #51411

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code