patchGNU Octave - Patches: patch #8919, Start of patch to enable...

 
 

patch #8919: Start of patch to enable visibility attributes for GCC in build system

Submitted by:  Rik <rik5>
Submitted on:  Fri 26 Feb 2016 05:32:15 PM UTC  
 
Category: Core : otherPriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Mon 04 Apr 2016 11:41:25 AM UTC, comment #2:

The problem with index_exception::message (as described - but interpreted wrongly - in bug #47587) still puzzles me.

Making it pure virtual and adding definitions to every class that inherits from it (as done in file #36813) does not seem the right way to go.

When I revert that part of the patch and re-compile, I get the following additional linker errors:

The last two led to my assumption that index_exception::message was not defined. But it is in lo-array-errwarn.cc.

The first error indicates problems with linking objects that have been compiled with different ABIs. But I do not know where (and if really) this is a problem.

Those three link errors disappear if the definition for index_exception::message is inlined (see attached patch).
(However, I do not know what inlining a virtual function means for the compiler and whether this could lead to changed behavior where "message" is actually overloaded by a child class. AFAICS "message" is not overloaded anywhere in core Octave.)

A similar behavior shows for the destructor of octave_chunk_buffer. To avoid linker errors, the destructor has to be inlined (also included in the new patch).

Maybe someone more experienced can comment on this...

I should already have mentioned in comment #1 that these are "minimal patches": I do not see any more link errors than the ones already mentioned. But most probably there are more functions that should be exposed to get all packages (and any .oct file) to compile...

After updating to the newest Mercurial repo, I see a lot of new warnings (probably from changeset f3f8e1d3e399: avoid mulitple definitions of static function-scope vars (bug #47372)). So I wonder whether that change is working as expected... I'll attach the complete output from make, too.

I cannot test any further since Octave still does not compile because of the functions that are not exported by gnulib when it is compiled with CFLAGS="-fvisibility=hidden" CXXFLAGS="-fvisibility=hidden".

(file #36831, file #36832)

Markus Mützel <mmuetzel>
Project Member
Thu 31 Mar 2016 07:16:13 PM UTC, comment #1:

I hope it is to OK if I jump in with some results from trial and error.

I applied the patch from comment #0 and build on a freshly cloned default branch after bootstrap, configure with:
make CFLAGS="-fvisibility=hidden" CXXFLAGS="-fvisibility=hidden"
I decorated the functions which threw errors with OCTAVE_API. In this way, the linker errors are down to the following:

These are functions (not) exported from gnulib. I do not know how to configure to use the visibility flags for compilation but not for the gnulib part.

With the attached patch applied, a lot of warnings along these lines occur while compiling:

Hope this helps.

(file #36813)

Markus Mützel <mmuetzel>
Project Member
Fri 26 Feb 2016 05:32:15 PM UTC, original submission:

By using visibility attributes a programmer can control which functions are exported into the symbol table of a shared object. By reducing the number of symbols the linker can run faster and the generated shared object is smaller.

For an explanation of the benefits and the approach see: https://gcc.gnu.org/wiki/Visibility.

Octave already has support mostly in place for this.

Steps:

1) make maintainer-clean
2) Apply visibility.patch from this issue report
3) change CFLAGS and CXXFLAGS to include "-fvisibility=hidden"
4) bootstrap
5) configure
6) make

The build currently fails because liboctinterp is looking for symbols that have not been exported from liboctave. This requires finding the necessary functions and decorating them with OCTAVE_API so they are exported into the symbol table of liboctave. This is where things stand today (2/26/2016).

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #36831:  visibility3.patch added by mmuetzel (28KiB - text/x-diff)
file #36832:  output_make.log added by mmuetzel (164KiB - application/octet-stream)
file #36813:  visibility2.patch added by mmuetzel (25KiB - text/x-diff)
file #36475:  visibility.patch added by rik5 (1KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by lachlan (Updated the item)
  • -unavailable- added by mmuetzel (Updated the item)
  • -unavailable- added by rik5 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 07 Jul 2016 02:05:40 AM UTClachlanCategoryNone=>Core : other
    Mon 04 Apr 2016 11:41:25 AM UTCmmuetzelAttached File-=>Added visibility3.patch, #36831
      Attached File-=>Added output_make.log, #36832
    Thu 31 Mar 2016 07:16:13 PM UTCmmuetzelAttached File-=>Added visibility2.patch, #36813
    Fri 26 Feb 2016 05:32:16 PM UTCrik5Attached File-=>Added visibility.patch, #36475

    Back to the top


    Powered by Savane 3.1-cleanup1