bugGnash - The GNU Flash player - Bugs: bug #38217, Testsuite build fails with...

 
 

bug #38217: Testsuite build fails with undefined symbols with --enable-visibility

Submitted by:  Sandro Santilli <strk>
Submitted on:  Fri Feb 1 06:04:29 2013  
 
Category: buildSeverity: 3 - Normal
Release: NoneStatus: Fixed
Privacy: PublicAssigned to: Sandro Santilli <strk>
Open/Closed: Closed

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

(Jump to the original submission Jump to the original submission)

Sun Jul 14 19:00:54 2013, comment #16:

As the only author of all the extensions, I believe only the fileio one is actually used by anyone. Or at least it has been used in the past. There really isn't a need for an extensions API, as the API for each extension mimics the library the extension is supporting. Extension are merely a wrapper. Printing a warning is probably all that needs to be done.

Rob Savoye <rsavoye>
Project Administrator
Sun Jul 14 18:46:24 2013, comment #15:

Good question. I guess if we had a clear-defined API then extensions authors would stick to that, and that API would be made visible. If we don't then, well, we should declare a build with --enable-visibility as not-functional with extension and notify the case explicitly at runtime, with something like a WARNING when extensions enabling is requested (and the testsuite should avoid running tests for extensions in that case).

Sandro Santilli <strk>
Project MemberIn charge of this item.
Sun Jul 14 11:16:28 2013, comment #14:

Is --enable-visibility not fundamentally incompatible with --enable-extensions? Although we could make the unresolved symbols visible, the idea of the extension framework is that an extension author can access any symbol in the Gnash core, which would require them modifying Gnash core, or, as is more practicable, to build without visibility.

Bastiaan Jacques <bjacques>
Project Member
Sat Jul 13 16:21:19 2013, comment #13:

bug #36642 is one reason not to enable by default, if we fix that then I'm happy with it being a default.

Thanks for double checking :)

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri Jul 12 22:21:04 2013, comment #12:

Can you think of any reason why hidden visibility should not be enabled by default, now that even the testsuite builds with it? I can think of only reasons in favour[1], and most especially having it by default would prevent developers from breaking it by accident.

[1] http://gcc.gnu.org/wiki/Visibility

Bastiaan Jacques <bjacques>
Project Member
Thu Jul 11 17:36:49 2013, comment #11:

DSOTEXPORT macro added, will make more symbols visible when testsuite is enabled, for a start, but it will be easy to switch to a different behavior if anyone would want to build with low visibility while still running a (forceably smaller) testsuite.

http://git.savannah.gnu.org/cgit/gnash.git/commit/?id=cd54b5ab5acd6bd357750f142059089eebe4664c

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue Jul 9 11:12:15 2013, comment #10:

I added --disable-testsuite to speed up cross builds, it doesn't really have any other purpose for native builds other than dropping a few warnings from configure. I doubt anyone else uses it anyway, so I'm not sure this needs "fixing".

Rob Savoye <rsavoye>
Project Administrator
Tue Jul 9 09:36:44 2013, comment #9:

> To make things simple and chunkable, what about starting with the TESTDSOEXPORT macro, currently defaulting to be an alias for DSOEXPORT ? We can discuss configure switches later


Doing that would also allow us to measure whether my concern is significant enough to merit making the extra exports conditional.

Bastiaan Jacques <bjacques>
Project Member
Tue Jul 9 09:28:55 2013, comment #8:

People that want to save the most space output could as well add --disable-testsuite. Those how don't will be able to run the testsuite, and thus will need those symbols.

If you prefer testsuite to be enabled by default that's a different topic.

I'm against an --enable-visibility=testsuite because you'll be then be back to "testsuite broken when adding --enable-visibility".

In any case the ./configure script should tell you if your configuration is non-functional (ie: requested testsuite but can't run due to visibility).

To make things simple and chunkable, what about starting with the TESTDSOEXPORT macro, currently defaulting to be an alias for DSOEXPORT ? We can discuss configure switches later

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue Jul 9 08:52:30 2013, comment #7:

I suspect few people actually build with --disable-testsuite. But I suppose --enable-visility could be given an =testsuite argument

Bastiaan Jacques <bjacques>
Project Member
Tue Jul 9 08:30:26 2013, comment #6:

How about having a TESTDSOEXPORT macro to only export when --disable-testsuite isn't given ?

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon Jul 8 21:17:32 2013, comment #5:

Fixing this would require littering Gnash core with even more DSOEXPORTs than are presently in use--at a very casual count, 20 functions are to be marked. I'm inclined to say this should not be done, because it is reducing the effect of --enable-visibility, with the only benefit of letting developers run make check with this flag.

Having said that, it might be useful to add a warning to the output of configure that the testsuite won't work when this flag is used.

Bastiaan Jacques <bjacques>
Project Member
Fri Feb 1 08:58:50 2013, comment #4:

The only difference I see in dumpconfig output is that the broken build has '-fvisibility=hidden'.

It could really be that MovieTester lacks a DSOEXPORT...

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri Feb 1 07:39:16 2013, comment #3:

with --enable-visibility the build still fails as of commit:118600cd59d4ebfc6b557b162cfe25e9f510da53

make[2]: Entering directory `/home/src/gnash/build/head/vanilla/testsuite/samples'
CXX clip_as_button2_TestRunner-clip_as_button2-TestRunner.o
CXXLD clip_as_button2-TestRunner
../../testsuite/.libs/libtestsuite.a(libtestsuite_la-MovieTester.o): In function `gnash::MovieTester::findDisplayItemByName(gna
sh::MovieClip const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)':
/usr/src/gnash/build/head/vanilla/testsuite/../../../../gnash-head/testsuite/MovieTester.cpp:337: undefined reference to `gnash
::getStringTable(gnash::as_object const&)'

make dumpconfig extract:

CXXFLAGS: -g -O2 -W -Wall -Wcast-qual -Wpointer-arith -Wreturn-type -Wnon-virtual-dtor -Wunused -fvisibility-inlines-hidden -fvisibility=hidden -fPIE -fstack-protector --param ssp-buffer-size=4 -Wformat -Werror=format-security
LDFLAGS: -Wl,--as-needed -fPIE -pie -Wl,-z,relro -Wl,-z,now

Configure line:

configure --enable-visibility

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri Feb 1 06:38:40 2013, comment #2:

I confirm it's the --enable-visibility flag breaking this.
Now trying with latest commit

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri Feb 1 06:07:42 2013, comment #1:

I see commits of Rob about visibility and flags.
Will try again after pulling 118600cd59d4ebfc6b557b162cfe25e9f510da53

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri Feb 1 06:04:29 2013, original submission:

While the core builds all fine, running "make check" fails to build some MovieTester based things in some kind of conditions.

I'm still debugging the conditions.
What I know is that a vanilla ./configure works fine.

I'm not sure which ./configure switches or FLAGS overriding break it. Still debugging. See bug #36642 for previous talk about this.

Sandro Santilli <strk>
Project MemberIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bjacques (Posted a comment)
  • -unavailable- added by strk (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 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat Jul 13 16:21:19 2013strkOpen/ClosedOpen=>Closed
    Fri Jul 12 22:21:04 2013bjacquesStatusReady For Test=>Fixed
    Thu Jul 11 17:36:49 2013strkStatusWont Fix=>Ready For Test
      Assigned toNone=>strk
    Mon Jul 8 21:17:32 2013bjacquesStatusNone=>Wont Fix
    Fri Feb 1 07:39:16 2013strkSummaryTestsuite build fails with undefined symbols=>Testsuite build fails with undefined symbols with --enable-visibility

    Back to the top


    Powered by Savane 3.1-cleanup