MOZ_AUTOMATION is supposed to be set on all builds, but currently isn't. When set, --enable-warnings-as-errors is enabled by default. When that happens, SM arm64 debug builds are busted with: [task 2017-04-14T22:43:47.839283Z] In file included from /home/worker/workspace/build/src/obj-spider/js/src/Unified_cpp_js_src22.cpp:20:0: [task 2017-04-14T22:43:47.839363Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Debugger-vixl.cpp: In static member function 'static vixl::DebugCommand* vixl::DebugCommand::Parse(char*)': [task 2017-04-14T22:43:47.839436Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Debugger-vixl.cpp:1123:44: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::Token*; T = vixl::Token*; long unsigned int MinInlineCapacity = 0ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.839457Z] args.append(Token::Tokenize(chunk)); [task 2017-04-14T22:43:47.839474Z] ^ [task 2017-04-14T22:43:47.839543Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Debugger-vixl.cpp:1124:28: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::Token*&; T = vixl::Token*; long unsigned int MinInlineCapacity = 0ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.839563Z] args.append(format); [task 2017-04-14T22:43:47.839575Z] ^ [task 2017-04-14T22:43:47.839644Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Debugger-vixl.cpp:1128:44: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::Token*; T = vixl::Token*; long unsigned int MinInlineCapacity = 0ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.839665Z] args.append(Token::Tokenize(chunk)); [task 2017-04-14T22:43:47.839683Z] ^ [task 2017-04-14T22:43:47.839751Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Debugger-vixl.cpp:1131:42: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::Token*; T = vixl::Token*; long unsigned int MinInlineCapacity = 0ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.839771Z] args.append(Token::Tokenize(chunk)); [task 2017-04-14T22:43:47.839786Z] ^ [task 2017-04-14T22:43:47.847611Z] In file included from /home/worker/workspace/build/src/obj-spider/js/src/Unified_cpp_js_src22.cpp:29:0: [task 2017-04-14T22:43:47.847691Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp: In member function 'void vixl::Decoder::AppendVisitor(vixl::DecoderVisitor*)': [task 2017-04-14T22:43:47.847773Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp:115:32: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::DecoderVisitor*&; T = vixl::DecoderVisitor*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.847798Z] visitors_.append(new_visitor); [task 2017-04-14T22:43:47.847827Z] ^ [task 2017-04-14T22:43:47.848905Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp: In member function 'void vixl::Decoder::PrependVisitor(vixl::DecoderVisitor*)': [task 2017-04-14T22:43:47.848992Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp:120:51: error: ignoring return value of 'T* mozilla::Vector<T, N, AllocPolicy>::insert(T*, U&&) [with U = vixl::DecoderVisitor*&; T = vixl::DecoderVisitor*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.849017Z] visitors_.insert(visitors_.begin(), new_visitor); [task 2017-04-14T22:43:47.849037Z] ^ [task 2017-04-14T22:43:47.849760Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp: In member function 'void vixl::Decoder::InsertVisitorBefore(vixl::DecoderVisitor*, vixl::DecoderVisitor*)': [task 2017-04-14T22:43:47.849848Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp:128:40: error: ignoring return value of 'T* mozilla::Vector<T, N, AllocPolicy>::insert(T*, U&&) [with U = vixl::DecoderVisitor*&; T = vixl::DecoderVisitor*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.849871Z] visitors_.insert(it, new_visitor); [task 2017-04-14T22:43:47.849888Z] ^ [task 2017-04-14T22:43:47.849974Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp:133:32: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::DecoderVisitor*&; T = vixl::DecoderVisitor*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.850004Z] visitors_.append(new_visitor); [task 2017-04-14T22:43:47.850022Z] ^ [task 2017-04-14T22:43:47.850061Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp: In member function 'void vixl::Decoder::InsertVisitorAfter(vixl::DecoderVisitor*, vixl::DecoderVisitor*)': [task 2017-04-14T22:43:47.850136Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp:142:40: error: ignoring return value of 'T* mozilla::Vector<T, N, AllocPolicy>::insert(T*, U&&) [with U = vixl::DecoderVisitor*&; T = vixl::DecoderVisitor*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.850166Z] visitors_.insert(it, new_visitor); [task 2017-04-14T22:43:47.850192Z] ^ [task 2017-04-14T22:43:47.850279Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Decoder-vixl.cpp:147:32: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::DecoderVisitor*&; T = vixl::DecoderVisitor*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:47.850300Z] visitors_.append(new_visitor); [task 2017-04-14T22:43:47.850314Z] ^ [task 2017-04-14T22:43:48.588512Z] /home/worker/workspace/gcc/bin/g++ -std=gnu++11 -o Unified_cpp_js_src26.o -c -I/home/worker/workspace/build/src/obj-spider/dist/system_wrappers -include /home/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DENABLE_BINARYDATA -DENABLE_SIMD -DENABLE_SHARED_ARRAY_BUFFER -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/home/worker/workspace/build/src/js/src -I/home/worker/workspace/build/src/obj-spider/js/src -I/home/worker/workspace/build/src/obj-spider/dist/include -I/home/worker/workspace/build/src/obj-spider/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /home/worker/workspace/build/src/obj-spider/js/src/js-confdefs.h -MD -MP -MF .deps/Unified_cpp_js_src26.o.pp -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O3 -fno-omit-frame-pointer -Werror -Wno-shadow -Werror=format /home/worker/workspace/build/src/obj-spider/js/src/Unified_cpp_js_src26.cpp [task 2017-04-14T22:43:49.143378Z] In file included from /home/worker/workspace/build/src/obj-spider/js/src/Unified_cpp_js_src23.cpp:2:0: [task 2017-04-14T22:43:49.143495Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Instrument-vixl.cpp: In constructor 'vixl::Instrument::Instrument(const char*, uint64_t)': [task 2017-04-14T22:43:49.143639Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/Instrument-vixl.cpp:142:32: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = vixl::Counter*&; T = vixl::Counter*; long unsigned int MinInlineCapacity = 8ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:49.143678Z] counters_.append(counter); [task 2017-04-14T22:43:49.143710Z] ^ [task 2017-04-14T22:43:49.226021Z] In file included from /home/worker/workspace/build/src/obj-spider/js/src/Unified_cpp_js_src23.cpp:47:0: [task 2017-04-14T22:43:49.226144Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp: In member function 'virtual void vixl::Simulator::VisitException(const vixl::Instruction*)': [task 2017-04-14T22:43:49.226286Z] /home/worker/workspace/build/src/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:412:57: error: ignoring return value of 'bool mozilla::Vector<T, N, AllocPolicy>::append(U&&) [with U = long int; T = long int; long unsigned int MinInlineCapacity = 0ul; AllocPolicy = js::SystemAllocPolicy]', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:49.226333Z] spStack_.append(xreg(31, Reg31IsStackPointer)); [task 2017-04-14T22:43:49.226375Z] ^ [task 2017-04-14T22:43:50.346434Z] In file included from /home/worker/workspace/build/src/obj-spider/js/src/Unified_cpp_js_src21.cpp:20:0: [task 2017-04-14T22:43:50.346549Z] /home/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp: In member function 'void js::jit::MacroAssembler::callWithABIPre(uint32_t*, bool)': [task 2017-04-14T22:43:50.346648Z] /home/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:685:32: error: ignoring return value of 'bool js::jit::MoveResolver::resolve()', declared with attribute warn_unused_result [-Werror=unused-result] [task 2017-04-14T22:43:50.346695Z] moveResolver_.resolve(); [task 2017-04-14T22:43:50.346727Z] ^ [task 2017-04-14T22:43:54.250775Z] cc1plus: all warnings being treated as errors [task 2017-04-14T22:43:54.337146Z] /home/worker/workspace/build/src/config/rules.mk:1022: recipe for target 'Unified_cpp_js_src21.o' failed [task 2017-04-14T22:43:54.337234Z] make[3]: *** [Unified_cpp_js_src21.o] Error 1 [task 2017-04-14T22:43:54.337281Z] make[3]: *** Waiting for unfinished jobs.... [task 2017-04-14T22:43:54.341304Z] make[3]: Entering directory '/home/worker/workspace/build/src/obj-spider/config/external/icu' [task 2017-04-14T22:43:54.341352Z] libicu.a.desc [task 2017-04-14T22:43:54.341385Z] rm -f libicu.a [task 2017-04-14T22:43:54.342127Z] /home/worker/workspace/build/src/obj-spider/_virtualenv/bin/python /home/worker/workspace/build/src/config/expandlibs_gen.py -o libicu.a.desc ../../../config/external/icu/common/libicuuc.a ../../../config/external/icu/i18n/libicui18n.a ../../../config/external/icu/data/libicudata.a [task 2017-04-14T22:43:54.434997Z] make[3]: Leaving directory '/home/worker/workspace/build/src/obj-spider/config/external/icu' [task 2017-04-14T22:43:55.591125Z] cc1plus: all warnings being treated as errors [task 2017-04-14T22:43:55.742281Z] /home/worker/workspace/build/src/config/rules.mk:1022: recipe for target 'Unified_cpp_js_src22.o' failed [task 2017-04-14T22:43:55.742369Z] make[3]: *** [Unified_cpp_js_src22.o] Error 1 [task 2017-04-14T22:43:56.122109Z] cc1plus: all warnings being treated as errors [task 2017-04-14T22:43:56.225761Z] /home/worker/workspace/build/src/config/rules.mk:1022: recipe for target 'Unified_cpp_js_src23.o' failed [task 2017-04-14T22:43:56.225844Z] make[3]: *** [Unified_cpp_js_src23.o] Error 1 [task 2017-04-14T22:44:13.504324Z] make[3]: Leaving directory '/home/worker/workspace/build/src/obj-spider/js/src' [task 2017-04-14T22:44:13.504551Z] /home/worker/workspace/build/src/config/recurse.mk:73: recipe for target 'js/src/target' failed [task 2017-04-14T22:44:13.504591Z] make[2]: *** [js/src/target] Error 2 [task 2017-04-14T22:44:13.504618Z] make[2]: Leaving directory '/home/worker/workspace/build/src/obj-spider' [task 2017-04-14T22:44:13.504761Z] /home/worker/workspace/build/src/config/recurse.mk:32: recipe for target 'compile' failed [task 2017-04-14T22:44:13.504782Z] make[1]: *** [compile] Error 2 [task 2017-04-14T22:44:13.504808Z] make[1]: Leaving directory '/home/worker/workspace/build/src/obj-spider' [task 2017-04-14T22:44:13.505012Z] /home/worker/workspace/build/src/config/rules.mk:519: recipe for target 'default' failed [task 2017-04-14T22:44:13.505033Z] make: *** [default] Error 2

Sean - I barely glanced at the code to decide how to handle these.

Comment on attachment 8859008 [details] [diff] [review] Fix uses of uninitialized values in arm64 code Review of attachment 8859008 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/arm64/vixl/MozSimulator-vixl.cpp @@ +408,5 @@ > case kCallRtRedirected: > VisitCallRedirection(instr); > return; > case kMarkStackPointer: > + AutoEnterOOMUnsafeRegion oomUnsafe; /home/worker/workspace/build/src/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:431:11: error: 'AutoEnterOOMUnsafeRegion' was not declared in this scope @@ +410,5 @@ > return; > case kMarkStackPointer: > + AutoEnterOOMUnsafeRegion oomUnsafe; > + if (!spStack_.append(xreg(31, Reg31IsStackPointer))) > + oomUnsafe.crash("tracking stack for ARM64 simulator"); /home/worker/workspace/build/src/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:433:13: error: 'oomUnsafe' was not declared in this scope

Comment on attachment 8859008 [details] [diff] [review] Fix uses of uninitialized values in arm64 code Review of attachment 8859008 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/arm64/vixl/Decoder-vixl.cpp @@ +30,5 @@ > > #include "jit/arm64/vixl/Globals-vixl.h" > #include "jit/arm64/vixl/Utils-vixl.h" > > +#include "mozilla/Unused.h" From check_spidermonkey_style.py: js/src/jit/arm64/vixl/Decoder-vixl.cpp:32:34: error: "jit/arm64/vixl/Utils-vixl.h" should be included after "mozilla/Unused.h"

Yeah, I just slapped that together without testing last night, since it didn't really affect the review. This version compiles and passes check-style.

Comment on attachment 8859300 [details] [diff] [review] Fix uses of uninitialized values in arm64 code Review of attachment 8859300 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/arm64/vixl/MozSimulator-vixl.cpp @@ +33,2 @@ > #include "threading/LockGuard.h" > #include "vm/Runtime.h" FWIW, context changed here in bug 1353763

Comment on attachment 8859300 [details] [diff] [review] Fix uses of uninitialized values in arm64 code Review of attachment 8859300 [details] [diff] [review]: ----------------------------------------------------------------- r+ with minor comment. ::: js/src/jit/arm64/MacroAssembler-arm64.cpp @@ +683,5 @@ > reserveStack(*stackAdjust); > { > + AutoEnterOOMUnsafeRegion oomUnsafe; > + if (!moveResolver_.resolve()) > + oomUnsafe.crash("MacroAssembler::callWithABIPre"); ARM64 has the same enoughMemory_ functionality as x64. Therefore, this can mirror that code as follows: enoughMemory_ &= moveResolver_.resolve(); if (!enoughMemory_) return; The code paths in the Simulator and the Debugger are OK to crash on OOM since those are just for testing in debug builds, but the MacroAssembler should be more crash-averse.

Comment on attachment 8859300 [details] [diff] [review] Fix uses of uninitialized values in arm64 code Review of attachment 8859300 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/arm64/vixl/Decoder-vixl.cpp @@ +113,5 @@ > } > } > > void Decoder::AppendVisitor(DecoderVisitor* new_visitor) { > + mozilla::Unused << visitors_.append(new_visitor); AFAIK, we use the instruction decoder to emulate JS behaviour in case of asm.js failures. In such case we are running under the signal handler. 2 questions: - Is it safe to allocate? - Why don't we check the result of the allocation?

(In reply to Nicolas B. Pierron [:nbp] from comment #7) > ::: js/src/jit/arm64/vixl/Decoder-vixl.cpp > @@ +113,5 @@ > > } > > } > > > > void Decoder::AppendVisitor(DecoderVisitor* new_visitor) { > > + mozilla::Unused << visitors_.append(new_visitor); > > AFAIK, we use the instruction decoder to emulate JS behaviour in case of > asm.js failures. In such case we are running under the signal handler. > > 2 questions: > - Is it safe to allocate? > - Why don't we check the result of the allocation? That sounds like a question very much outside of my area. If this is running in a signal handler, then I would certainly expect it to not be safe to allocate in the first place. I don't really know what any of this code does. But from skimming it, my guess is that we only add these visitors during setup type things. Hopefully that doesn't run under a signal handler? But even if it does, there are hardly any calls to these functions: - the Debugger adds a single visitor: printer_->AppendVisitor(disasm_) - Simulator-vixl.cpp adds two: decoder_->AppendVisitor(instrumentation_) in set_instruction_stats and decoder_->InsertVisitorBefore(print_disasm_, this) in set_trace_parameters - MozSimulator-vixl.cpp adds one: decoder_->AppendVisitor(this) in Simulator::init All these *sound* like singletons, and the visitors_ vector is initialized with inline space for 8 visitors. By that logic, these should all be MOZ_ALWAYS_TRUE. But as I said, I don't really know what's going on here. I'll resubmit for review with that change.

Pushed by sfink@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3b545af8c568 Fix uses of uninitialized values in arm64 code, r=sstangl