1326289 - Assertion failure when building clang-plugin against debug clang

Status: RESOLVED FIXED

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

Description

[:Nika Layzell (Away until Aug. 26)]

bool llvm::APInt::ult(const llvm::APInt&) const: Assertion
> `BitWidth == RHS.BitWidth && "Bit widths must be same for comparison"

Probably related to this line:

    if (Type->getSize().ule(Literal->getValue())) {

Comment 1

[:Nika Layzell (Away until Aug. 26)]

Attachment: Fix BitWidth assertion failure in debug clang-plugin

MozReview-Commit-ID: JNPc4CqEgPd

Comment 2

[:Ehsan Akhgari]

Comment on attachment 8828140 [details] [diff] [review]
Fix BitWidth assertion failure in debug clang-plugin

Review of attachment 8828140 [details] [diff] [review]:
-----------------------------------------------------------------

Sweet, thanks a lot!

Do these fix all of the assertions we hit?  Do you know what the overhead of having assertions on is?  I wonder if it's worth turning them on in our infrastructure?

Comment 3

[:Nika Layzell (Away until Aug. 26)]

Yes, I successfully ran an assertions enabled build on linux64 with clang 5.0 (trunk).

With regard to the cost of assertions is - I ran the build twice - once with a local release build running the clang plugin and once with a release build with assertions. The computer _was_ in use when both of these runs were done so it's not a perfect (or even "ok") comparison by any means.

With assertions disabled, a build of clang against my local tree took 21:46, while with assertions enabled it took 27:06 - so a pretty substantial hit. 

I would also be interested in potentially building clang-tidy with assertions enabled, and running that on try, as that could avoid a lot of the overhead from assertions during codegen which the clang-plugin would run into, but still makes the majority of the same checks. What do you think?

Comment 4

[:Ehsan Akhgari]

That would make the clang-tidy binaries that developers will use (post bug 1328454) slower, which isn't great...  I suppose we'll have to test this manually every once in a while then.

Comment 5

[:Nika Layzell (Away until Aug. 26)]

(In reply to :Ehsan Akhgari from comment #4)
> That would make the clang-tidy binaries that developers will use (post bug
> 1328454) slower, which isn't great...  I suppose we'll have to test this
> manually every once in a while then.

Potentially we could build both? As in a debug one which is used on infra, and a release one which is downloaded by mach static-analysis?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31b797ff2742
Fix BitWidth assertion failure in debug clang-plugin, r=ehsan

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/31b797ff2742