577813 - Missing __attribute__ ((used)) on PrepareAndDispatch functions

Status: RESOLVED DUPLICATE of bug 826481

(Core :: XPConnect, defect)

Description

[Jan Hubicka]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100506 SUSE/3.5.10-0.1.1 Firefox/3.5.10 GTB7.1
Build Identifier: mainline

PrepareAndDispatch functions are used from asm statements and as such they should
be annotated with  __attribute__ ((used)) as follows:
*** xptcstubs_x86_64_linux.cpp.old      Wed Jul  7 21:10:54 2010                                                                                                       
--- xptcstubs_x86_64_linux.cpp  Wed Jul  7 21:10:56 2010                                                                                                               
*************** const PRUint32 FPR_COUNT            = 8;                                                                                                               
*** 62,68 ****                                                                                                                                                         
  // The parameters are mapped into an array of type 'nsXPTCMiniVariant'                                                                                               
  // and then the method gets called.                                                                                                                                  
                                                                                                                                                                       
! extern "C" nsresult                                                                                                                                                  
  PrepareAndDispatch(nsXPTCStubBase * self, PRUint32 methodIndex,                                                                                                      
                     PRUint64 * args, PRUint64 * gpregs, double *fpregs)                                                                                               
  {                                                                                                                                                                    
--- 62,68 ----                                                                                                                                                         
  // The parameters are mapped into an array of type 'nsXPTCMiniVariant'                                                                                               
  // and then the method gets called.                                                                                                                                  
                                                                                                                                                                       
! extern "C" nsresult __attribute__ ((used))                                                                                                                           
  PrepareAndDispatch(nsXPTCStubBase * self, PRUint32 methodIndex,                                                                                                      
                     PRUint64 * args, PRUint64 * gpregs, double *fpregs)                                                                                               
  {    
this is however just one of many copies of the problem.  This prevents Mozilla to build with GCC LTO.

Reproducible: Always

Steps to Reproduce:
1. Get GCC mainline and configure with checking disabled
2. Build Mozilla with -fwhopr
3. See undefined references to PrepareAndDispatch.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this a gcc-specific requirement?  Why do other compilers (and older gcc) not need this annotation?

Comment 2

[Benjamin Smedberg]

How is this symbol different from any other extern symbol which might end up as part of a shared library interface? Or does GCC think (in LTO mode) that this symbol is unused because it is hidden-visibility and therefore supposedly unreachable?

It seems strange also that it can't figure out automatically that the symbol is referenced from the assembly.

Is attribute(used) supported on GCC back to GCC 4.1 (and other GCC-like compilers which use this code, such as clang), or do we need to add some macrology for this case?

Comment 3

[(dormant account)]

(In reply to comment #2)
> How is this symbol different from any other extern symbol which might end up as
> part of a shared library interface? Or does GCC think (in LTO mode) that this
> symbol is unused because it is hidden-visibility and therefore supposedly
> unreachable?

The benefit of LTO is that it can treat hidden symbols similar to static functions in compilation units.

http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00580.html

Comment 4

[(dormant account)]

Attachment: fix

Jan, it's best to attach fixes as -u patches.

I'm guessing GCC isn't flagging the function as used because it treats assembly as text and the assembler is running in a separate process.

Comment 5

[Benjamin Smedberg]

At this point, Linux PGO is not a blocker although we'll take it. And this patch probably needs a reviewer set.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 456860 [details] [diff] [review]
fix

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> At this point, Linux PGO is not a blocker although we'll take it. And this
> patch probably needs a reviewer set.

I'll just read this as volunteering

Comment 7

[Benjamin Smedberg]

Comment on attachment 456860 [details] [diff] [review]
fix

nit, extra whitespace at top of file, please fix before landing

Comment 8

[(dormant account)]

Rafael, can you please get rid of the extra newline and land this?

Comment 9

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

I consider this a bug in gcc, as its LTO mode ignores references that are visible by the linker. This was fixed in LLVM:

http://blog.mozilla.com/respindola/2011/03/04/lto-on-os-x/

But unfortunately I don't see gcc being able to parse assembly any time soon, so I guess we have to take this.

I am checking it in.

Comment 10

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c7bba86f5e02

Comment 11

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Sorry, had to back it out

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ab2bc0593355

It was failing to build. The log is at

https://tbpl.mozilla.org/php/getParsedLog.php?id=7752478&tree=Mozilla-Inbound&full=1

Has this macro changed since the patch was written?

Comment 12

[Martin Stránský [:stransky] (ni? me)]

This is already fixed:

https://dxr.mozilla.org/mozilla-central/rev/e923330d5bd3aef1f687eddf96a06ad5ec3860ed/xpcom/reflect/xptcall/md/unix/xptcstubs_ppc64_linux.cpp#35

by Bug 826481