Bug 133546 – [JavaScriptCore] FTL buildfix for EFL platform

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[László Langó]

Fix compiler warnings that breaks the FTL build on EFL

[László Langó]

Created attachment 232547 [details]
Patch

[Filip Pizlo]

Comment on attachment 232547 [details]
Patch

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

> Source/JavaScriptCore/ftl/FTLLocation.cpp:64
> +    return loc;

It would be far better to just say "return Location();"

> Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66
> +#pragma GCC diagnostic ignored "-Wmissing-format-attribute"

Why?

[László Langó]

(In reply to comment #2)
> (From update of attachment 232547 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232547&action=review
> 
> > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66
> > +#pragma GCC diagnostic ignored "-Wmissing-format-attribute"
> 
> Why?

Because I don't know any better. This looks like false positive warning to me, but it breaks the build, so we have to ignore it. Do you have any better idea how to handle this warning?

[László Langó]

Created attachment 232856 [details]
Patch

[Filip Pizlo]

Comment on attachment 232856 [details]
Patch

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

> Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66
> +#pragma GCC diagnostic ignored "-Wmissing-format-attribute"

I think you should add the format attribute that it is asking for. Have you tried it?  Also, it would be useful to post the compiler error you're getting so that others could suggest a better fix. In general using pragmas to disable warnings is not an OK way of fixing compiler warnings.

[László Langó]

(In reply to comment #5)
> (From update of attachment 232856 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232856&action=review
> 
> > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66
> > +#pragma GCC diagnostic ignored "-Wmissing-format-attribute"
> 
> I think you should add the format attribute that it is asking for. Have you tried it? Also, it would be useful to post the compiler error you're getting so that others could suggest a better fix.

Ok the problem is the following. We have this logger function in WTF:
void WTFLogAlwaysAndCrash(const char* format, ...) __attribute__((__format__(printf, 1, 2)))

And we have this function in LLVMExports.cpp#56 that has a function pointer parameter:
extern "C" WTF_EXPORT_PRIVATE JSC::LLVMAPI* initializeAndGetJSCLLVMAPI(void (*)(const char*, ...));

Then we call the function with a logger function:
initializeAndGetJSCLLVMAPI(WTFLogAlwaysAndCrash);

And we got the folloing waring message:
/mnt/work/Webkit/Webkit/Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66:44: error: parameter might be a candidate for a format attribute [-Werror=suggest-attribute=format]
/mnt/work/Webkit/Webkit/Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66:44: error: argument of function call might be a candidate for a format attribute [-Werror=suggest-attribute=format]

This is a bit crazy because it points to the call of 'initializeAndGetJSCLLVMAPI' and not to the call of 'WTFLogAlwaysAndCrash'. We call the callback here:
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/llvm/library/LLVMExports.cpp#L70
You can see that is has format argument (and doesn't have any additional args). So it still looks like a false positive warning message for me. But if someone know a better way to resolve this warning, then please let me know.

[László Langó]

Created attachment 233163 [details]
Patch

[Darin Adler]

Comment on attachment 233163 [details]
Patch

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

> Source/JavaScriptCore/ftl/FTLAbstractHeap.cpp:67
> +        if ((static_cast<unsigned>(1) << i) == m_elementSize) {

It’s much better to write this as 1U rather than static_cast<unsigned>(1).

> Source/JavaScriptCore/ftl/FTLStackMaps.cpp:53
> +    out.printf("0x%016llx", static_cast<long long unsigned>(integer));

We normally call the type "unsigned long long" rather than "long long unsigned".

[László Langó]

Created attachment 233968 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 233968 [details]
patch for landing

Clearing flags on attachment: 233968

Committed r170525: <http://trac.webkit.org/changeset/170525>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.