767847 - Wrong type use causes error during compile with Clang

Status: RESOLVED WORKSFORME

(Thunderbird :: Address Book, defect)

Description

[Fuck You For Making Me Make an Account]

In the file mailnews/addressbook/src/nsDirPrefs.cpp

Two constants are defined on lines 95 and 96 with values 0x80000000 and 0x80000001.

All uses of these two defined values is through line 369, PRInt32 position.

Clang throws an error on line 378 and stops there. For correctness, position should be defined as PRUint32 as both values used with it should be unsigned to remain non-negative.

Comment 1

[Ludovic Hirlimann [:Usul]]

(In reply to **** You For Making Me Make an Account from comment #0)
> In the file mailnews/addressbook/src/nsDirPrefs.cpp
> 
> Two constants are defined on lines 95 and 96 with values 0x80000000 and
> 0x80000001.
> 
> All uses of these two defined values is through line 369, PRInt32 position.
> 
> Clang throws an error on line 378 and stops there. For correctness, position
> should be defined as PRUint32 as both values used with it should be unsigned
> to remain non-negative.

Would you be willing to make a patch now that you have an account ?

Comment 2

[Fuck You For Making Me Make an Account]

A patch would be a trivial one-liner for that change, however, that is not all that is needed. Unfortunately, the exact same error pattern is repeated in other files, signed variable is used exclusively for dealing with unsigned constants.

When I submitted the bug, I had only hit the one instance and then ran into something unrelated. I came to find out the build system is horribly broken. If I rerun the build, it dies saying a rule is missing for some .py file. Googling reveals known problem, clean the build. Ok, make clean, oops, now when I try to build it says rule is missing for Makefile.am, argh **** autotools garbage!

So, I can't resume the build where it left off, the make can't get there. I can't clean the working directly, make clean is broken. I must restart from scratch EVERY build attempt. Yes, fo real, WHAT THE MOTHERFUCK?!? Untar, apply patches, configure, gmake EVERY **** TIME!

When I had that figured out, confirmed this change was good, and then got barely further before hitting ANOTHER INSTANT of the SAME THING I just said **** it. rm -rf thunderbird

I'm going to give kmail2 a shot. Thunderbird is flaky and it was getting real bad with stuff like enigmail never works after the rapid version number insanity.

I DON'T GIVE A FLYING **** WHAT GOOGLE DOES WITH CHROME! IF I WANTED CHROME, I'D INSTALL IT! STOP TRYING TO MIMIC CHROME WITH FIREFOX, STOP TRYING TO MIMIC GOOGLE WITH MOZILLA!

Comment 3

[Erik Hvatum]

For the record, I know this guy and he's a good guy, if perhaps not the most patient.

To fix the issue he is referring to and possibly others that may not necessarily cause things to break but that aren't strictly correct, build Firefox and Thunderbird with Clang and, if you really want to clean things up, use the -Weverything option.  You are very likely to catch subtle, lurking logic bugs and security holes with -Weverything.

Comment 4

[Wayne Mery (:wsmwk)]

(this does not block development process, unless something hasn't been mentioned in the comments)

Comment 5

[:aceman]

Could you at least paste the text of the error message?

Maybe gcc shows it too but only as a warning so that I could go from there.

Also, how do you compile Thunderbird with Clang?

Comment 6

[Magnus Melin [:mkmelin]]

(In reply to Fuck You For Making Me Make an Account from comment #2)
> A patch would be a trivial one-liner for that change, however, that is not
> all that is needed. Unfortunately, the exact same error pattern is repeated
> in other files, signed variable is used exclusively for dealing with
> unsigned constants.

-1 is "not found"

> When I submitted the bug, I had only hit the one instance and then ran into
> something unrelated. I came to find out the build system is horribly broken.
> If I rerun the build, it dies saying a rule is missing for some .py file.
> Googling reveals known problem, clean the build. Ok, make clean, oops, now
> when I try to build it says rule is missing for Makefile.am, argh ****
> autotools garbage!

Probably bug 729752? This should fix it
rm mozilla/xpcom/idl-parser/xpidllex.py
rm mozilla/xpcom/idl-parser/xpidlyacc.py
rm mozilla/xpcom/idl-parser/*.pyc

Comment 7

[Ludovic Hirlimann [:Usul]]

is this a duplicate of bug 747621 ?

Comment 8

[:aceman]

Maybe. We don't know as the reporter didn't provide the error messages. Even though the file name may match, the line numbers don't.

Comment 9

[:aceman]

Ok, this seem to be the instructions to compile with Clang to include into mozconfig:
# -O4 tells clang to do LTO
ac_add_options --disable-debug --enable-optimize='-O4'
# Exclude this line, and your will use over 10x the amount of memory.
ac_add_options --disable-debug-symbols
# Build with clang
export CC=/usr/local/bin/clang
export CXX=/usr/local/bin/clang++
# These flags are necessary to get elfhack to compile
export LDFLAGS='-Wl,-z,norelro -flto'
# This flag is necessary to link properly
export RANLIB='ar -s --plugin /usr/local/lib/LLVMgold.so'

I have llvm 3.0 package installed, I'll see if clang is there and usable.

Comment 10

[Magnus Melin [:mkmelin]]

I assume you meant http://mxr.mozilla.org/comm-central/source/mailnews/addrbook/src/nsDirPrefs.cpp#60 - that has changed since your report, and this looks WFM now.