1102586 - TraceLoggingGraph.cpp:107:61: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘size_t {aka long unsigned int}’ [-Wformat=]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

Recently-introduced build warning, with up-to-date mozilla-inbound and gcc 4.8, on 64-bit Ubuntu 14.10:
{
js/src/vm/TraceLoggingGraph.cpp: In member function ‘uint32_t TraceLoggerGraphState::nextLoggerId()’:
js/src/vm/TraceLoggingGraph.cpp:107:61: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
                            numLoggers, numLoggers, numLoggers);
                                                              ^
}

Looks like this file (TraceLoggingGraph.cpp) was only just created, in bug 1072903; hence, marking as blocking that bug.

Comment 1

[Daniel Holbert [:dholbert]]

I think we should probably be using "%z" here -- that's the format string for size_t.

(I recall that this wasn't standard C++ at some point, but according to a comment on [1], it's standard as of C++1x, and it's possible/likely that our C++1x requirements mean we can rely on %z being available. Plus, a MXR search shows that we already use it elsewhere [2] so we can probably rely on it.)

[1] https://stackoverflow.com/questions/2524611/how-to-print-size-t-variable-portably
[2] https://mxr.mozilla.org/mozilla-central/search?string=%25z&find=.cpp%24

Comment 2

[Emanuel Hoogeveen [:ehoogeveen]]

As far as I can tell from the Microsoft documentation and Visual Studio blog posts, Visual Studio 2013 still doesn't support %z (and I don't see it mentioned for the Visual Studio 2015 preview either). MSVC has '%I' instead: http://msdn.microsoft.com/en-us/library/tcxf1dw6.aspx

Comment 3

[Daniel Holbert [:dholbert]]

(Sorry, I guess I meant "%zu" - u for unsigned, and z to indicate number-of-bytes-in-an-unsigned)

Well, we do seem to use %zu here at least:
>141     fprintf(stderr, "  file %s line %u offset %zu\n",
>142             script()->filename(), (unsigned) script()->lineno(),
>143             script()->pcToOffset(pc()));
https://mxr.mozilla.org/mozilla-central/source/js/src/jit/RematerializedFrame.cpp#141

...and pcToOffset there does indeed return size_t:
> 1066     size_t pcToOffset(const jsbytecode *pc) const {
https://mxr.mozilla.org/mozilla-central/source/js/src/jsscript.h#1066

RematerializedFrame.cpp doesn't seem to be excluded from our MSVC build, so I *think* MSVC is seeing & correctly-handling this format string there.

If we're worried though, we can always just use "%u" and static_cast<unsigned>(mySizeTValue).

Comment 4

[Hannes Verschore [:h4writer]]

Fixed in the patch queue. Thanks for reporting!

Comment 5

[Chris Peterson [:cpeterson]]

btw, MSVC because does not support %zu, the current approach for printf-ing size_t is to use MFBT's (nonstandard) PRIuSIZE format macro (from bug 1114724) in cross-platform code.