1593703 - MOZ_IMPLICIT seems to be busted with clang 5

Status: RESOLVED FIXED

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

Description

[Nathan Froyd [:froydnj]]

See e.g.

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274389511&repo=try&lineNumber=4724

where we are bumping the minimum clang version to 5: the base toolchains build for clang (which uses clang 5 in this case), complains about CheckedInt constructors not being marked as explicit when they are in fact marked as MOZ_EXPLICIT.

Andi, can you take a look at this?

Comment 1

[Sylvestre Ledru [:Sylvestre]]

P1 as blocking some compiler upgrade.

Comment 2

[Andi [:andi]]

Since this is a P1, I'm working on this, will come up with an update shortly.

Comment 3

[Andi [:andi]]

Clang 5 has a serious problem when it generates the AnnotateAttr for a Decl it only supports a single annotation and the rest are dropped. In the case for CheckedInt we have present two annotations MOZ_IMPLICIT and MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT but only the later one is included into the AST node.
Let's take as an example js/src/builtin/RegExp.cpp we have 8 AST matches for CheckedInt(U) but only for the later one we have generated the sane specific_attrs. Also this behavior ranges from clang 5.0 - clang 6.0 and was fixed in 7.0.
So this leads me to the concussion that right now we are dealing with a corrupt AST for a compilation unit, at least the one that is exposed to clang-tooling.

Comment 4

[Andi [:andi]]

Attachment: Bug 1593703 - disable clang based static-analysis for build-linux64-base-toolchains-clang/*.

Comment 5

[Andi [:andi]]

Let make sure we don't regress: https://treeherder.mozilla.org/#/jobs?repo=try&revision=29fa0f6841f7a7049cc82ef5e7fbbec8834db7c1&selectedJob=274859842

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a944fda1615
disable clang based static-analysis for build-linux64-base-toolchains-clang/*. r=froydnj

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5a944fda1615

Comment 8

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #23731 (as of Wed, 06 Nov 2019 22:09:41 GMT) ==

Improvements:

11% build times linux64 debug base-toolchains-clang taskcluster-m5.4xlarge 2,016.28 -> 1,795.81
9% build times linux64 opt base-toolchains-clang taskcluster-c5d.4xlarge 1,789.33 -> 1,625.03
8% build times linux64 opt base-toolchains-clang taskcluster-m5.4xlarge 1,949.11 -> 1,795.07

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=23731