1580260 - TestMustReturnFromCaller.cpp broken with clang trunk

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect, P1)

Description

[(Away)]

error: 'error' diagnostics seen but not expected:
TestMustReturnFromCaller.cpp Line 62: You must immediately return after calling this function
TestMustReturnFromCaller.cpp Line 128: You must immediately return after calling this function

Bisection points to LLVM r370247. I can't say whether this is a bug upstream or a change that our static analysis needs to adapt to. (Note that in that Phabricator, there is mention of a followup change 370406; it doesn't help for this issue.)

Comment 1

[Andi [:andi]]

There is a bug in the CFG generator since we don't have the ReturnStmt into the CFGBlock but instead we have an IntegerLiteral followed by the ReturnStmt which is wrong, since IntegerLiteral should have parent the ReturnStmt.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:Sylvestre, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Andi; what do you think the priority should be?

Comment 4

[(Away)]

If I remove that test file from the build, then it fails at https://searchfox.org/mozilla-central/rev/23f836a71cfe961373c8bd0d0219ec60a64b3c8f/dom/bindings/BindingUtils.h#2480

[task 2019-09-30T22:58:09.281Z] 22:58:09    ERROR -  /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/BindingUtils.h:2480:5: error: You must immediately return after calling this function
[task 2019-09-30T22:58:09.281Z] 22:58:09     INFO -      aRv.Throw(NS_ERROR_FAILURE);
[task 2019-09-30T22:58:09.281Z] 22:58:09     INFO -      ^

I guess the destructor for the nsCOMPtr is in the way?

Anyway, since blocks building the primary codebase (not "just tests") I think it should be relatively high priority.

Comment 5

[Andi [:andi]]

Indeed i concur to what :dmajor is saying, this should be P1, even though it's related with the trunk version of clang.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:Sylvestre, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Actually, it is a P2 because clang 10 isn't going to be released in before one or two firefox releases.

Comment 8

[(Away)]

(In reply to Sylvestre Ledru [:Sylvestre] from comment #7)

Actually, it is a P2 because clang 10 isn't going to be released in before one or two firefox releases.

Hello from three months in the future. :-) This is preventing from testing the clang 10 release candidates, can we reprioritize?

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Sure! Time flies!

Comment 10

[(Away)]

Andi, if I'm reading your comment 1 correctly, this sounds like a bug in the clang source, is that right? If so, would you be able to file a bug against LLVM and explain the details there? We would need to do this fairly soon as we are running out of runway for the 10.0 release.

Comment 11

[Andi [:andi]]

Yes, it's a bug in the CFG generator but maybe in the meantime we can have a workaround for this in order to have this unblocked. I'm assigning this to myself to work on it.
Also will open a bug on llvm side and upstream a potential fix.

Comment 12

[Andi [:andi]]

Attachment: Bug 1580260 - rework the logic for `MustReturnFromCaller` checker.

Starting with Clang-10, and in particular with https://reviews.llvm.org/D66404,
the CFG has been reflown in order to reduce discrepancies in destructor calls with
the actual codegen. This has also impacted the order of CFGStmt for ReturnStmt,
whereas now the AST children of the ReturnStmt have precedence other their parent
node.
The purpose of this patch is to fix this and add new supported cases for the Expr
that is generated from ReturnStmt.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b476d3ea3fc9
rework the logic for `MustReturnFromCaller` checker. r=froydnj

Comment 14

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/b476d3ea3fc9