Re: clang version on tryserver?
(sorry for top posting) Dear Jan and Steve, Problems solved. After a few trials and errors, I figured there are TWO similar constructs that, depending on the order of parameter evaluation, may not pass what is desired due to unexpected modification during parameter evaluation. I changed my local patches as follows and now the errors during test run on the tryserver (using clang) match the ones I see locally (using GCC). Now I can home in the few remaining errors possibly introduced my patches. (They may be timing-dependent issues uncovered by my patch, but who knows.) Thank you again! Chiaki PS: I have found that gcc-8 has a more feature-rich format string checking: it warns if a complete output would be longer than the buffer's length passed to snprintf(). Amazing. However, since I turn some warnings including this one into compilation errors (to avoid the unexpected failure on the tryserver while I can build successfully), I had to tweak the webrtc code in a few places. Modified parts: comm/mailnews/local/src/nsPop3Sink.cpp // // clang and GCC has a different order of argument evaluation. // We have to save m_outFileSream because // geter_AddRefs(m_outFileStream) may trash it depending on the // order of evaluation of passed arguments! // nsCOMPtr tmp_p = m_outFileStream; rv1 = NS_NewBufferedOutputStream(getter_AddRefs(m_outFileStream), tmp_p.forget(), 64 * 1024 ); A bit overkill.: /comm/mailnews/local/src/nsLocalMailFolder.cpp: nsCOMPtr tmp_in = mCopyState->m_fileStream; nsCOMPtr tmp_out; nsresult rv2 = NS_NewBufferedOutputStream(getter_AddRefs(tmp_out), tmp_in.forget(), 64 * 1024 ); mCopyState->m_fileStream = tmp_out; On 2019/07/31 5:45, ISHIKAWA,chiaki wrote: 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 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 ___ dev-platform mailin
Re: clang version on tryserver?
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 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
Re: clang version on tryserver?
Dear Jan, I am glad that I included the snippet of code here. I should have known. I would try to work around this, well I would try, and see if I can avoid the runtime error on tryserver (!). Thanks a million! Chiaki On 2019/07/30 22:20, Jan de Mooij wrote: On Tue, Jul 30, 2019 at 1:51 PM ISHIKAWA,chiaki 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(). Hope this helps, Jan [0] https://stackoverflow.com/questions/15440833/g-vs-intel-clang-argument-passing-order Chiaki ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: clang version on tryserver?
On 2019/07/30 21:13, Jörg Knobloch wrote: On 30 Jul 2019 13:50, ISHIKAWA,chiaki wrote: What is the version of clang used on tryserver? The same as on the local machine? For Windows I get: $ ../.mozbuild/clang/bin/clang --version clang version 8.0.0 (tags/RELEASE_800/final 356365) Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: c:\mozilla-source\.mozbuild\clang\bin I'm running debug builds and the compiler crashes every once in a while, but very rarely lately. Restarting the build fixes the problem. Reporting the problem at LLVM didn't help in this case, see: https://bugs.llvm.org/show_bug.cgi?id=39848 Jörg. I see. Your problem was a random crash, that is hard to crack. The same 8.0.0 version, but my local clang-8 is run under linux. The difference between your problem and mine is that my problem is a constant crash due to an internal error caught with stack dump (hooray!). Hopefullly the stack dump and the detailed message from the compiler itself will help the developer developer to home in the bugs. At the end, I am attaching the short text that I included to the mail that I sent to request to have an account on llvm.org's bugzilla. Like I said, this happens all the time and so is a logical defect somewhere. But then you use clang-8.0.0. The same version which I think I have under linux. I am clueless then what to do to compile my code using clang on my local PC. Hmm... Ok there was a mention of increasing the main thread stack size or whatever. Maybe I should try it... Chiaki Stack dump: 0. Program arguments: /home/ishikawa/.mozbuild/clang/bin/clang-8 -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -disable-free -disable-llvm-verifier -discard-value-names -main-file-name line64.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -relaxed-aliasing -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -enable-split-dwarf -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -ggnu-pubnames -split-dwarf-file line64.dwo -momit-leaf-frame-pointer -v -ffunction-sections -fdata-sections -coverage-notes-file /KERNEL-SRC/moz-obj-dir/objdir-tb3/comm/ldap/c-sdk/libraries/libldif/line64.gcno -resource-dir /home/ishikawa/.mozbuild/clang/lib/clang/8.0.0 -include /KERNEL-SRC/moz-obj-dir/objdir-tb3/mozilla-config.h -D DEBUG=1 -D LINUX=1 -D LINUX2_0 -D linux=1 -D _PR_PTHREADS -D NET_SSL -D NS_DOMESTIC -D LDAP_DEBUG -D USE_WAITPID -D NEEDPROTOS -I /NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldif -I /KERNEL-SRC/moz-obj-dir/objdir-tb3/comm/ldap/c-sdk/libraries/libldif -I /NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/include -I /KERNEL-SRC/moz-obj-dir/objdir-tb3/dist/include -I /KERNEL-SRC/moz-obj-dir/objdir-tb3/dist/include/nspr -I /KERNEL-SRC/moz-obj-dir/objdir-tb3/dist/include/nss -D MOZILLA_CLIENT -U _FORTIFY_SOURCE -D _FORTIFY_SOURCE=2 -D fdatasync=fdatasync -D DEBUG_4GB_CHECK -D USEHELGRIND=1 -D DEBUG -U _FORTIFY_SOURCE -D _FORTIFY_SOURCE=2 -internal-isystem /usr/local/include -internal-isystem /home/ishikawa/.mozbuild/clang/lib/clang/8.0.0/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -Og -Werror=sign-compare -Werror=unused-result -Werror=unused-variable -Werror=format -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -std=gnu99 -fdebug-compilation-dir /KERNEL-SRC/moz-obj-dir/objdir-tb3/comm/ldap/c-sdk/libraries/libldif -ferror-limit 19 -fmessage-length 0 -stack-protector 2 -fno-builtin-strlen -fobjc-runtime=gcc -fdiagnostics-show-option -fcolor-diagnostics -o line64.o -x c /NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldif/line64.c -faddrsig 1. parser at end of file 2. Per-module optimization passes 3. Running pass 'Function Pass Manager' on module '/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldif/line64.c'. 4. Running pass 'Combine redundant instructions' on function '@ldif_base64_decode' #0 0x7f9ac1292fe4 PrintStackTraceSignalHandler(void*) (/home/ishikawa/.mozbuild/clang/bin/../lib/libLLVM-8.so+0x62cfe4) #1 0x7f9ac1290f0e llvm::sys::RunSignalHandlers() (/home/ishikawa/.mozbuild/clang/bin/../lib/libLLVM-8.so+0x62af0e) #2 0x7f9ac1293298 SignalHandler(int) (/home/ishikawa/.mozbuild/clang/bin/../lib/libLLVM-8.so+0x62d298)
Re: clang version on tryserver?
On 7/30/19 6:20 AM, Jan de Mooij wrote: On Tue, Jul 30, 2019 at 1:51 PM ISHIKAWA,chiaki 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
Re: clang version on tryserver?
On Tue, Jul 30, 2019 at 1:51 PM ISHIKAWA,chiaki 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(). Hope this helps, Jan [0] https://stackoverflow.com/questions/15440833/g-vs-intel-clang-argument-passing-order Chiaki > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: clang version on tryserver?
On 30 Jul 2019 13:50, ISHIKAWA,chiaki wrote: What is the version of clang used on tryserver? The same as on the local machine? For Windows I get: $ ../.mozbuild/clang/bin/clang --version clang version 8.0.0 (tags/RELEASE_800/final 356365) Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: c:\mozilla-source\.mozbuild\clang\bin I'm running debug builds and the compiler crashes every once in a while, but very rarely lately. Restarting the build fixes the problem. Reporting the problem at LLVM didn't help in this case, see: https://bugs.llvm.org/show_bug.cgi?id=39848 Jörg. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
