Bug 70904 – Fix compilation of DFG JIT on PIC targets

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Andy Wingo]

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.

[Andy Wingo]

Created attachment 112483 [details]
Patch

[WebKit Review Bot]

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.

[Darin Adler]

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?

[Andy Wingo]

Created attachment 112560 [details]
Patch

[Andy Wingo]

Thanks for the comments.  I attached an updated patch and re-submitted it to the checkers.

[Darin Adler]

Comment on attachment 112560 [details]
Patch

Unfortunate to have two copies of this macro. Later we should put it in a header.

[Andy Wingo]

Created attachment 112561 [details]
Patch

[Andy Wingo]

The newly attached patch updates the changelog message, as you suggested (and I forgot).

[Andy Wingo]

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

[Andy Wingo]

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.)

[WebKit Review Bot]

Comment on attachment 112561 [details]
Patch

Clearing flags on attachment: 112561

Committed r98891: <http://trac.webkit.org/changeset/98891>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.