Dear Steve,
Thank you for your analysis and suggestion for moving to gcc-8.
I would change my local GCC compiler to gcc-8, and g++-8.
Also, I would modify the offending code so that
the evaluation order of the arguments does not get in the way.
I still yet to do a few more tasks, but I think the mystery of the
difference between the clang version and GCC version, which got me
wondering for the last several days,
has been solved by your and Jan's analysis.
I *think* I was assuming left-to-right argument ordering somehow.
I probably should use clang occasionally to put my implicit assumptions
gleaned from years of GCC usage to torture test, so to speak.
Thank you very much !!!
Chiaki
PS: Well, for whatever the reason, I also found a problem with cflow-8
(maybe my particular setting of C compilation time macros, etc.).
That would be an unexpected bonus. :-)
On 2019/07/31 0:15, Steve Fink wrote:
On 7/30/19 6:20 AM, Jan de Mooij wrote:
On Tue, Jul 30, 2019 at 1:51 PM ISHIKAWA,chiaki <ishik...@yk.rim.or.jp>
wrote:
nsresult rv2
=
NS_NewBufferedOutputStream(getter_AddRefs(mCopyState->m_fileStream),
mCopyState->m_fileStream.forget(), <=== It seems this can be
nullptr in
clang-8 version???
64 * 1024 );
This looks like it could be caused by Clang evaluating your arguments
in a
different order from GCC (see also [0]). If Clang evaluates
left-to-right,
that getter_AddRefs might null out your m_fileStream before we
evaluate the
m_fileStream.forget().
Note that, if I'm reading the spec correctly, it's not just the order
of evaluation between arguments. Before c++17, the evaluation of
different arguments could be interleaved arbitrarily. It looks like
c++17 changes argument evaluation to be "indeterminately sequenced"
instead of the previous "unsequenced". Indeterminately sequenced means
they can happen in any order, but *cannot* be interleaved as they are
now. I don't think that comes up in your specific example, but it's
closely related.
https://en.cppreference.com/w/cpp/language/eval_order
I know the interleaving happens in practice because the GC rooting
hazard analysis ran into this: a Rooted value passed to a function was
extracted into a temporary, another argument's evaluation triggered a
GC that moved the value, and then the now-invalid temporary was passed
into the function. It looks like c++17 will prevent this from happening.
I guess that gives me motivation to finish updating the rooting
analysis to gcc8, which has c++17 support. Which is relevant to you,
since you're compiling with gcc: the minimum gcc version is going to
be bumped to gcc7 soon, and it looks like we'll only be testing with
gcc8 in the continuous integration system, so it would be safest to
use a minimum of gcc 8. (gcc 7.4.0 has a bug that breaks the rooting
analysis, so it will go straight to gcc 8.)
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
