I updated mozilla-central to d480b3542cc2 today and can no longer build. I get this error: js/src/jsmath.cpp:740:63: error: ignoring return value of ‘ssize_t read(int, void*, size_t)’, declared with attribute warn_unused_result [-Werror=unused-result] I am using this compiler: gcc (Ubuntu/Linaro 4.7.3-2ubuntu1~12.04) 4.7.3 From what I understand tinderbox uses an old version of glibc which prevents it from catch unused-result errors. This is pretty terrible for end developers, though, since the build keeps breaking.

Guessing this is from bug 1118149.

Line 740 is: > 740 (void)read(fd, seed.u8, mozilla::ArrayLength(seed.u8)); https://mxr.mozilla.org/mozilla-central/source/js/src/jsmath.cpp?rev=65e82280a4be#740 I'd expect the "(void)" there should already be "using" the value. That was added a few months ago in bug 1061664, presumably to fix this exact build warning. Strange that it's not working in your case. Ben, does the warning go away if you replace the "(void)" with "mozilla::unused <<"? (You'll need to add #include "mozilla/unused.h" to this file, too.)

Yep, mozilla::unused fixes it for me. Really weird.

bkelly: I will post a patch later (unless someone beats me to it :). You should be able to build by adding "ac_add_options --disable-warnings-as-errors" to your mozconfig.

Part 1: Use read()'s return value to fix -Wunused-result warning in jsmath.cpp's PRNG. Luke: I'm sending this review to you because you reviewed this block of code for me last time. :) gcc is complaining that read()'s return value, marked with the warn_unused_result attribute in *some* versions of glibc, is unused. gcc takes its warn_unused_result attribute very seriously and won't allow a (void) cast suppress the warning. This patch "uses" the return value by mixing it into the random seed bits. 99+% of the time the return value will 8. If we are unable to read /dev/urandom or get a short read, then the return value will be some other value -1 <= x < 8, providing some desperate scrap of entropy. :)

Part 2: Use rand_s()'s error return value in jsmath.cpp's PRNG. If we're overengineering random_generateSeed() with an error check for read(), we might as well add an error check to the Windows code path. Microsoft's rand_s() function returns 0 on success or EINVAL or an undocumented error code on failure. Also, PRMJ_Now() returns 64-bit time in microseconds. There's no need to throw away the high bits by stuffing the 64-bit time into seed.u32[1] instead of seed.u64.

Comment on attachment 8546899 [details] [diff] [review] part-1-fix-gcc-warn_unused_result.patch Review of attachment 8546899 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsmath.cpp @@ +738,5 @@ > MOZ_ASSERT(fd >= 0, "Can't open /dev/urandom"); > if (fd >= 0) { > + ssize_t nread = read(fd, seed.u8, mozilla::ArrayLength(seed.u8)); > + MOZ_ASSERT(nread == 8, "Can't read /dev/urandom"); > + seed.u32[1] ^= nread; I think 'unused << read(...);' would be the more standard Mozilla way to do this. (#include "mozilla/unused.h")

Comment on attachment 8546907 [details] [diff] [review] part-2-use-rand_s-error.patch Review of attachment 8546907 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsmath.cpp @@ +719,5 @@ > +{ > + uint32_t u32; > + errno_t error = rand_s(&u32); > + MOZ_ASSERT(error == 0, "rand_s() error"); > + return u32 ^ error; Again, it seems a bit strange to mix in the return value of the random number generator.

(In reply to Luke Wagner [:luke] from comment #8) > Again, it seems a bit strange to mix in the return value of the random > number generator. The idea was that if read() or rand_s() fail to return random bits, mixing in the error return value provides a small amount of randomness. But I can suppress the warning with 'unused << read()' if you prefer.

patch v2: * add 'unused << read(...)' * add assertions for "can't happen" read() and rand_s() errors * call rand_s() twice to fill entire 64-bit buffer * remove mixing of error code bits from open(), read(), and rand_s()

Thanks. https://hg.mozilla.org/integration/mozilla-inbound/rev/422367d84ec6

sorry had to back this out for bustage like: https://treeherder.mozilla.org/logviewer.html#?job_id=5388488&repo=mozilla-inbound

Landing again with build fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/4c63c749f2ca