1289366 - [Static Analysis] Control may reach end of non-void function in CharToByte from CTTestUtils.cpp

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Andi [:andi]]

As we are trying to integrate 1283395, i'm having this error when building:

security/certverifier/tests/gtest/CTTestUtils.cpp:128:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]

This is because function CharToByte doesn't return a value on all it's return paths.

>>static uint8_t
>>CharToByte(char c)
>>{
>>  if (c >= '0' && c <= '9') {
>>    return c - '0';
>>  } else if (c >= 'a' && c <= 'f') {
>>    return c - 'a' + 10;
>>  } else if (c >= 'A' && c <= 'F') {
>>    return c - 'A' + 10;
>>  }
>>  MOZ_RELEASE_ASSERT(false);
}

I think this worked now since the branch prediction for MOZ_RELEASE_ASSERT(false) was consistent thus in this case function would have always abort.
As we have added a signature function on static analysis builds - clang based analysis, this is not the case anymore so i think branching cannot be predicted so well and in this case a control flow problem is signalled by the compiler.

Comment 1

[Andi [:andi]]

Attachment: Bug 1289366 - added return statement for CharToByte when assertion fails.

Review commit: https://reviewboard.mozilla.org/r/67130/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67130/

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8774671 [details]
Bug 1289366 - added return statement for CharToByte when assertion fails.

https://reviewboard.mozilla.org/r/67130/#review64126

LGTM. Thanks for taking care of this.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/232175372fa0
added return statement for CharToByte when assertion fails. r=keeler

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/232175372fa0