Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Description • 10 years ago

Coverity analysis of source code in js/src has found a Missing return statement issue, that probably happened from Feb to Mar 2014.


________________________________________________________________________________________________________
*** CID 1123000:  Missing return statement  (MISSING_RETURN)
/js/src/jit/CodeGenerator.cpp: 6587 in js::jit::CodeGenerator::addGetPropertyCache(js::jit::LInstruction *, js::jit::RegisterSet, js::jit::Register, js::PropertyName *, js::jit::TypedOrValueRegister, bool)()
6581             GetPropertyParIC cache(objReg, name, output);
6582             return addCache(ins, allocateCache(cache));
6583           }
6584           default:
6585             MOZ_ASSUME_UNREACHABLE("Bad execution mode");
6586         }
>>>     CID 1123000:  Missing return statement  (MISSING_RETURN)
>>>     Arriving at the end of a function without returning a value.
6587     }
6588     
6589     bool
6590     CodeGenerator::addSetPropertyCache(LInstruction *ins, RegisterSet liveRegs, Register objReg,
6591                                        PropertyName *name, ConstantOrRegister value, bool strict,
6592                                        bool needsTypeBarrier)

Eric, any idea what should be added?

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 1 • 10 years ago

> statement issue, that probably happened from Feb to Mar 2014.

On further thoughts, let's discount this range for now.

	Luke Wagner [:luke]
	Comment 2 • 10 years ago

It would appear that Coverity doesn't understand MOZ_ASSUME_UNREACHABLE.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 3 • 10 years ago

So I guess this should be regarded as INVALID?

	Sean Stangl [:sstangl]
	Comment 4 • 10 years ago

Yes.

	Luke Wagner [:luke]
	Comment 5 • 10 years ago

Also, it you might want to see if Coverity has their own version of "assume not reached" that you can plug into MOZ_ASSUME_UNREACHABLE.  (This pattern shows up in tons of other places.)

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 6 • 10 years ago

OK, thanks all!

	Christian Holler (:decoder)
	Comment 7 • 10 years ago

Isn't one problem here that MOZ_ASSUME_UNREACHABLE is a no-op in release builds?

	Christian Holler (:decoder)
	Comment 8 • 10 years ago

(In reply to Christian Holler (:decoder) from comment #7)
> Isn't one problem here that MOZ_ASSUME_UNREACHABLE is a no-op in release
> builds?

Correcting myself: Not a no-op but just calling __builtin_unreachable and that won't abort anything.