Closed Bug 1593703 Opened 1 year ago Closed 1 year ago

MOZ_IMPLICIT seems to be busted with clang 5

Categories

(Firefox Build System :: Source Code Analysis, defect, P1)

defect

Tracking

(firefox72 fixed)

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: froydnj, Assigned: andi)

References

Details

Attachments

(1 file)

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?

Flags: needinfo?(bpostelnicu)

P1 as blocking some compiler upgrade.

Priority: -- → P1
Assignee: nobody → bpostelnicu
Flags: needinfo?(bpostelnicu)

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

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.

Attachment #9106930 - Attachment description: Bug 1593703 - for some unsupported toolchains (clang 5, clang 6) disable clang based static-analysis. → Bug 1593703 - disable clang based static-analysis for build-linux64-base-toolchains-clang/*.
See Also: → 1594473
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5a944fda1615
disable clang based static-analysis for build-linux64-base-toolchains-clang/*. r=froydnj
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

== 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

You need to log in before you can comment on or make changes to this bug.