991340 - CID 1123000: Missing return statement in jit/CodeGenerator.cpp as found by Coverity

Status: RESOLVED INVALID

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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?

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

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

Comment 2

[Luke Wagner [:luke]]

It would appear that Coverity doesn't understand MOZ_ASSUME_UNREACHABLE.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

So I guess this should be regarded as INVALID?

Comment 4

[Sean Stangl [:sstangl]]

Yes.

Comment 5

[Luke Wagner [:luke]]

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.)

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

OK, thanks all!

Comment 7

[Christian Holler (:decoder)]

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

Comment 8

[Christian Holler (:decoder)]

(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.