Summary: | Linking fails with "relocation R_X86_64_PC32 against symbol `cti_vm_throw'" | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Priit Laes (IRC: plaes) <plaes> | ||||||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | barraclough, commit-queue, darin, eric, jmalonzo, oliver, yusukes | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | PC | ||||||||||||||||||
OS: | Linux | ||||||||||||||||||
Attachments: |
|
Description
Priit Laes (IRC: plaes)
2009-08-18 00:27:34 PDT
Created attachment 35026 [details]
webkit-bug-28422-fix-debug-symbol-visibility.patch
Not sure whether this is the right way to fix it, but at least it WORKSFORME :)
Why? Have you tried recompiling with -fPIC? Comment on attachment 35026 [details]
webkit-bug-28422-fix-debug-symbol-visibility.patch
Hiding/filtering the symbols is not the right fix here. You might as well do a release build. Have you tried forcing -fPIC in CFLAGS? If that works then we need to fix our build scripts to do that automatically.
(In reply to comment #3) > (From update of attachment 35026 [details]) > Hiding/filtering the symbols is not the right fix here. You might as well do a > release build. Have you tried forcing -fPIC in CFLAGS? If that works then we > need to fix our build scripts to do that automatically. Tried with CXXFLAGS="-fPIC" CFLAGS="-fPIC" but it didn't work. I have an alternative patch that seems to fix this issue (at least on my machine): http://plaes.org/files/2009-Q3/webkit-bug-28422-use-plt-segment-for-cti_vm_throw.patch It seems to be the right approach ( See section 1.5.5 in http://people.redhat.com/drepper/dsohowto.pdf paper..) Created attachment 38479 [details]
webkit-bug-28422-use-plt-segment-for-cti_vm_throw-v2.patch
Added #if PLATFORM(LINUX) guards suggested by bdash.
The SYMBOL_NAME macro is intended to abstract the variance in symbol names between platforms. #if'ing it at the call site defeats that purpose. Created attachment 38607 [details]
webkit-bug-28422-use-plt-segment-for-cti_vm_throw-v3.patch
I had to introduce another macro SYMBOL_STRING_INTERNAL in order to properly change symbol name..
Tested on Linux x86-64 (built with and without debugging).
*** Bug 28798 has been marked as a duplicate of this bug. *** Oliver would know if this looks sane. I am deferring to Gavin on this one -- conceivably hoisting to vm_throw should just use a safer mechanism to enter the trampoline Not a clue whether we should be doing this on Mac or not. :o) Lemme give this a test in the morning. G. Comment on attachment 38607 [details]
webkit-bug-28422-use-plt-segment-for-cti_vm_throw-v3.patch
This patch seems sensible to me, seems reasonable to use plt-indirect calls on linux. Doesn't seem to be a relevant issue to Mac.
My one concern would be the name, SYMBOL_STRING_INTERNAL, which doesn't seem to be accurately descriptive to me. The strings formed by this macro seem specific to the use case – I don't believe foo@plt syntax is commonly used outside of use in a call? For symbols internal to a compilation module, it is just a question of omitting the .globl directive? I'd suggest renaming the macro to something like SYMBOL_FOR_CALL would probably be clearer, but I don't know linux x86-64 asm that well, so perhaps I'm wrong.
I chose the SYMBOL_STRING_INTERNAL because it is supposedly used only inside the function. How about SYMBOL_STRING_CALL or SYMBOL_STRING_FOR_CALL? (In reply to comment #13) > I chose the SYMBOL_STRING_INTERNAL because it is supposedly used only inside > the function. > How about SYMBOL_STRING_CALL or SYMBOL_STRING_FOR_CALL? To my mind either of those is nice and clear, I'd happily r+ either. Your choice. Created attachment 39330 [details]
webkit-bug-28422-use-plt-segment-for-cti_vm_throw-final.patch
I decided to go with SYMBOL_STRING_RELOCATION which seemed more appropriate (only PC platforms seem to have call opcode, arms have branch)..
I also took the liberty to fill the Reviewed by slot :P
Created attachment 40285 [details]
webkit-bug-28422-use-plt-segment-for-cti_vm_throw-final.patch
Resynced the patch.
Comment on attachment 40285 [details]
webkit-bug-28422-use-plt-segment-for-cti_vm_throw-final.patch
Please don't change NOBODY (OOPS!) or svn-apply (and thus the commit-queue) won't be able to handle the patch. cq- for this patch, you could post another which would be commit-queue compatible, or someone else can land this by hand for you. Thanks for the patch. :)
Created attachment 40732 [details]
webkit-plt.patch
Updated the patch (again.. and already getting tired...)
Comment on attachment 40732 [details]
webkit-plt.patch
This patch has been reviewed -- can you update the changelog so it can be marked commit+ ?
Created attachment 40764 [details]
webkit-plt-reviewed.patch
Added "Reviewed by Gavin Barraclough." there...
Comment on attachment 40764 [details]
webkit-plt-reviewed.patch
I'm not sure I fully understand Oliver's request. But OK. To use the commit-queue you need both an r+ and a cq+. I'll add the r+. Since you already edited the ChangeLog to state that Gavin reviewed this (which is fine, but not normally done) the commit-queue will not set me as the reviewer, as it would have normally done having seen my r+.
Comment on attachment 40764 [details] webkit-plt-reviewed.patch Clearing flags on attachment: 40764 Committed r49224: <http://trac.webkit.org/changeset/49224> All reviewed patches have been landed. Closing bug. |