Bug 139384 - Fix Build Warning in JavaScriptCore ControlFlowProfiler::dumpData() api
Summary: Fix Build Warning in JavaScriptCore ControlFlowProfiler::dumpData() api
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: 2014-12-07 21:27 PST by Shivakumar J M
Modified: 2014-12-08 21:24 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.40 KB, patch)
2014-12-07 22:33 PST, Shivakumar J M
mark.lam: review-
Details | Formatted Diff | Diff
Patch-Updated-Review1 (1.40 KB, patch)
2014-12-08 00:13 PST, Shivakumar J M
mark.lam: review-
Details | Formatted Diff | Diff
Patch-Updated-Review1 (1.41 KB, patch)
2014-12-08 19:36 PST, Shivakumar J M
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shivakumar J M 2014-12-07 21:27:16 PST
Fix Build Warning in JavaScriptCore ControlFlowProfiler::dumpData() api:

Wpointer-arith -Wundef -Wwrite-strings -MMD -MT Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/runtime/ControlFlowProfiler.cpp.o -MF "Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/runtime/ControlFlowProfiler.cpp.o.d" -o Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/runtime/ControlFlowProfiler.cpp.o -c ../../Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp
../../Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp: In member function ‘void JSC::ControlFlowProfiler::dumpData() const’:
../../Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp:58:46: error: format ‘%ld’ expects argument of type ‘long int’, but argument 2 has type ‘int’ [-Werror=format=]
         dataLogF("SourceID: %ld\n", iter->key);
                                              ^
cc1plus: all warnings being treated as errors
ninja: build stopped: subcommand failed.
Comment 1 Shivakumar J M 2014-12-07 22:33:36 PST
Created attachment 242785 [details]
Patch

 Fix Build Warning by using proper format argument for dataLogF() function.
Comment 2 Shivakumar J M 2014-12-07 22:39:00 PST
might have casued by bug: https://bugs.webkit.org/show_bug.cgi?id=137785
Comment 3 Mark Lam 2014-12-07 22:53:32 PST
Comment on attachment 242785 [details]
Patch

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

> Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp:58
> +        dataLogF("SourceID: %d\n", iter->key);

Looks like the EFL port disagrees with this fix.  Out of curiosity, what port are you seeing this build warning on?

FYI, instead, I think the better fix is to do:
    dataLog("SourceID: “, iter->key, “\n”);
Comment 4 Shivakumar J M 2014-12-07 22:59:54 PST
(In reply to comment #3)
> Comment on attachment 242785 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242785&action=review
> 
> > Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp:58
> > +        dataLogF("SourceID: %d\n", iter->key);
> 
> Looks like the EFL port disagrees with this fix.  Out of curiosity, what
> port are you seeing this build warning on?
> 
> FYI, instead, I think the better fix is to do:
>     dataLog("SourceID: “, iter->key, “\n”);


Dear Mark,

     I am using EFL port, in my local build machine i got these error, but i am usng g++-4.8, std=c++11, is it some thing do with setup?

Thanks
Comment 5 Mark Lam 2014-12-07 23:01:57 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 242785 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=242785&action=review
> > 
> > > Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp:58
> > > +        dataLogF("SourceID: %d\n", iter->key);
> > 
> > Looks like the EFL port disagrees with this fix.  Out of curiosity, what
> > port are you seeing this build warning on?
> > 
> > FYI, instead, I think the better fix is to do:
> >     dataLog("SourceID: “, iter->key, “\n”);
> 
> 
> Dear Mark,
> 
>      I am using EFL port, in my local build machine i got these error, but i
> am usng g++-4.8, std=c++11, is it some thing do with setup?
> 
> Thanks

Probably something to do with your compiler/toolchain version or perhaps custom compiler flags that you are using (if any).  Anyway, try the dataLog() version I suggested on the EWS bots.  Every port should be happy with that.
Comment 6 Mark Lam 2014-12-07 23:03:40 PST
(In reply to comment #5)
> (In reply to comment #4)
> >      I am using EFL port, in my local build machine i got these error, but i
> > am usng g++-4.8, std=c++11, is it some thing do with setup?
> > 
> > Thanks
> 
> Probably something to do with your compiler/toolchain version or perhaps
> custom compiler flags that you are using (if any).

Or ... it’s probably a 32-bit vs 64-bit issue.  But using dataLog() should take care of that.
Comment 7 Shivakumar J M 2014-12-08 00:13:46 PST
Created attachment 242787 [details]
Patch-Updated-Review1

Fix Build Warning by using dataLog() function instead of dataLogF() function.
Comment 8 Mark Lam 2014-12-08 08:19:03 PST
Comment on attachment 242787 [details]
Patch-Updated-Review1

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

> Source/JavaScriptCore/runtime/ControlFlowProfiler.cpp:58
> +        dataLog("SourceID: %ld\n", iter->key);

This is wrong.  dataLog() doesn’t take a format string.  Instead, it should take a comma separated list or items to be printed.  The correct way to do this is:
    dataLog(“SourceID: “, iter->key, “\n”);
Comment 9 Shivakumar J M 2014-12-08 19:36:26 PST
Created attachment 242871 [details]
Patch-Updated-Review1

Updated the patch to properly use dataLog() function.
Comment 10 Mark Lam 2014-12-08 20:41:19 PST
Comment on attachment 242871 [details]
Patch-Updated-Review1

r=me
Comment 11 WebKit Commit Bot 2014-12-08 21:24:05 PST
Comment on attachment 242871 [details]
Patch-Updated-Review1

Clearing flags on attachment: 242871

Committed r177009: <http://trac.webkit.org/changeset/177009>
Comment 12 WebKit Commit Bot 2014-12-08 21:24:09 PST
All reviewed patches have been landed.  Closing bug.