Bug 70904 - Fix compilation of DFG JIT on PIC targets
Summary: Fix compilation of DFG JIT on PIC targets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-26 05:04 PDT by Andy Wingo
Modified: 2011-10-31 15:38 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.22 KB, patch)
2011-10-26 05:11 PDT, Andy Wingo
no flags Details | Formatted Diff | Diff
Patch (4.00 KB, patch)
2011-10-26 10:27 PDT, Andy Wingo
no flags Details | Formatted Diff | Diff
Patch (4.12 KB, patch)
2011-10-26 10:34 PDT, Andy Wingo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Wingo 2011-10-26 05:04:13 PDT
The DFG JIT currently does not compile on PIC targets due to missing PLT relocations in DFGOperations.cpp.  The patch to be attached fixes this issue.
Comment 1 Andy Wingo 2011-10-26 05:11:29 PDT
Created attachment 112483 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-26 05:13:54 PDT
Attachment 112483 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
Source/JavaScriptCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Source/JavaScriptCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Source/JavaScriptCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/JavaScriptCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Source/JavaScriptCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2011-10-26 10:15:14 PDT
Comment on attachment 112483 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112483&action=review

Contents of patch looks fine, but needs a new change log without tab characters and preferably with an explanation of the change too.

> Source/JavaScriptCore/dfg/DFGOperations.cpp:53
> +#if (OS(LINUX) || OS(FREEBSD)) && CPU(X86_64)
> +#define SYMBOL_STRING_RELOCATION(name) #name "@plt"
> +#elif OS(DARWIN) || (CPU(X86_64) && COMPILER(MINGW) && !GCC_VERSION_AT_LEAST(4, 5, 0))
> +#define SYMBOL_STRING_RELOCATION(name) "_" #name
> +#elif CPU(X86) && COMPILER(MINGW)
> +#define SYMBOL_STRING_RELOCATION(name) "@" #name "@4"
> +#else
> +#define SYMBOL_STRING_RELOCATION(name) #name
> +#endif

Can keep the size of this macro definition smaller by having it call SYMBOL_STRING in the #else case and removing one of the if cases?
Comment 4 Andy Wingo 2011-10-26 10:27:31 PDT
Created attachment 112560 [details]
Patch
Comment 5 Andy Wingo 2011-10-26 10:29:43 PDT
Thanks for the comments.  I attached an updated patch and re-submitted it to the checkers.
Comment 6 Darin Adler 2011-10-26 10:31:17 PDT
Comment on attachment 112560 [details]
Patch

Unfortunate to have two copies of this macro. Later we should put it in a header.
Comment 7 Andy Wingo 2011-10-26 10:34:02 PDT
Created attachment 112561 [details]
Patch
Comment 8 Andy Wingo 2011-10-26 10:35:10 PDT
The newly attached patch updates the changelog message, as you suggested (and I forgot).
Comment 9 Andy Wingo 2011-10-27 07:32:44 PDT
Please let me know if there is something I need to do to this patch.  Otherwise if it is acceptable, I am not a committer yet, so the ball would be in your court.

Thanks :)

Andy
Comment 10 Andy Wingo 2011-10-28 06:33:31 PDT
Darin, I'm setting cq?.  Would you mind taking a look at this again?  Thanks :)

(I'm still new at this webkit bugzilla protocol, please let me know if I'm getting it wrong.)
Comment 11 WebKit Review Bot 2011-10-31 15:38:20 PDT
Comment on attachment 112561 [details]
Patch

Clearing flags on attachment: 112561

Committed r98891: <http://trac.webkit.org/changeset/98891>
Comment 12 WebKit Review Bot 2011-10-31 15:38:25 PDT
All reviewed patches have been landed.  Closing bug.