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 01 Feb 2013 06:04:29 AM UTC  
 
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 14 Jul 2013 07:00:54 PM UTC, 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 14 Jul 2013 06:46:24 PM UTC, 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 14 Jul 2013 11:16:28 AM UTC, 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 13 Jul 2013 04:21:19 PM UTC, 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 12 Jul 2013 10:21:04 PM UTC, 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 11 Jul 2013 05:36:49 PM UTC, 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 09 Jul 2013 11:12:15 AM UTC, 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 09 Jul 2013 09:36:44 AM UTC, 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 09 Jul 2013 09:28:55 AM UTC, 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 09 Jul 2013 08:52:30 AM UTC, 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 09 Jul 2013 08:30:26 AM UTC, 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 08 Jul 2013 09:17:32 PM UTC, 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 01 Feb 2013 08:58:50 AM UTC, 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 01 Feb 2013 07:39:16 AM UTC, 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 01 Feb 2013 06:38:40 AM UTC, 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 01 Feb 2013 06:07:42 AM UTC, 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 01 Feb 2013 06:04:29 AM UTC, 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 13 Jul 2013 04:21:19 PM UTCstrkOpen/ClosedOpen=>Closed
    Fri 12 Jul 2013 10:21:04 PM UTCbjacquesStatusReady For Test=>Fixed
    Thu 11 Jul 2013 05:36:49 PM UTCstrkStatusWont Fix=>Ready For Test
      Assigned toNone=>strk
    Mon 08 Jul 2013 09:17:32 PM UTCbjacquesStatusNone=>Wont Fix
    Fri 01 Feb 2013 07:39:16 AM UTCstrkSummaryTestsuite build fails with undefined symbols=>Testsuite build fails with undefined symbols with --enable-visibility

    Back to the top


    Powered by Savane 3.1-cleanup