Missing __attribute__ ((used)) on PrepareAndDispatch functions

ASSIGNED
Unassigned

Status

()

Core
XPConnect
ASSIGNED
7 years ago
2 months ago

People

(Reporter: Jan Hubicka, Unassigned)

Tracking

(Blocks: 1 bug, {perf})

unspecified
x86_64
Linux
Points:
---

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(1 attachment)

fix
919 bytes, patch
Benjamin Smedberg
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
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.
Is this a gcc-specific requirement?  Why do other compilers (and older gcc) not need this annotation?
blocking2.0: --- → ?
Component: General → XPConnect
Keywords: perf
QA Contact: general → xpconnect

Comment 2

7 years ago
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

7 years ago
(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

7 years ago
Created attachment 456860 [details] [diff] [review]
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.
Assignee: nobody → tglek
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #456860 - Flags: review?

Comment 5

7 years ago
At this point, Linux PGO is not a blocker although we'll take it. And this patch probably needs a reviewer set.
blocking2.0: ? → -

Updated

7 years ago
Depends on: 521435

Updated

7 years ago
Blocks: 521435
No longer depends on: 521435
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
Attachment #456860 - Flags: review? → review?(benjamin)

Comment 7

6 years ago
Comment on attachment 456860 [details] [diff] [review]
fix

nit, extra whitespace at top of file, please fix before landing
Attachment #456860 - Flags: review?(benjamin) → review+

Comment 8

6 years ago
Rafael, can you please get rid of the extra newline and land this?
Assignee: tglek → respindola
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.
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c7bba86f5e02
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?
Assignee: respindola → nobody
You need to log in before you can comment on or make changes to this bug.