885538 - Clang compile failure in nsMsgSearchTerm.cpp

Status: RESOLVED FIXED

(MailNews Core :: Search, defect)

Description

[Frank Wein [:mcsmurf]]

Got this error report via IRC:
nsMsgSearchTerm.cpp fails to compile with a recent clang version:
mailnews/base/search/src/nsMsgSearchTerm.cpp:200:13: error: 
      assigning to 'const char *' from incompatible type 'char'
     *string = '\0'; // don't leave the string uninitialized
           ^ ~~~~

Comment 1

[:aceman]

The containing function is defined as:
nsresult NS_MsgGetStringForAttribute(int16_t attrib, const char **string)

And the caller sends a &(const char *) argument. This would need thought what is actually const in that variable when we do change it. It is the output variable.

There is also the NS_MsgGetStringForOperator() function where this sanitizing setting of \0 may be missing.

Comment 2

[:aceman]

mcsmurf, can you get the clang version from the user? Is it a stable release or developement branch?

Comment 3

[Frank Wein [:mcsmurf]]

The user joined on IRC, pasted the error message and left again iirc.

Comment 4

[:aceman]

Attachment: patch

So not sure what the reporter had, but with clang 3.2 and 3.3 the expression only produces this warning:
mailnews/base/search/src/nsMsgSearchTerm.cpp:200:15: warning: expression which evaluates to zero treated as a null pointer constant of type 'const char *' [-Wnon-literal-nul
    *string = '\0'; // don't leave the string uninitialized

But I think even the warning is useful and shows the logic problem. We surely probably do not want a null pointer there according to the comment.

Comment 5

[:aceman]

I built whole TB with clang 3.2 successfully and this was the only place producing this type of warning.

Comment 6

[Frank Wein [:mcsmurf]]

I checked again, the reporter said on IRC only "reporting a compilation error with a fairly recent version of Clang:" :)

Comment 7

[:aceman]

Attachment: sample program

This is a sample C++ program that I hope simulates the code in question well enough to see the problem.

Clang produces the same warning on it and the last printf produces "(null)".
GCC 4.8.1 does no warning and the program crashes on the last printf.

Comment 8

[Kent James (:rkent)]

My mac uses clang to compile. It reports:

Apple LLVM version 4.2 (clang-425.0.24) (based on LLVM 3.2svn)
Target: x86_64-apple-darwin11.4.2

Do you expect that version to exhibit the symptoms here?

Comment 9

[:aceman]

Yes, LLVM 3.2 should at least show the warning we mentioned. Can you paste what the sample program outputs?

Comment 10

[Kent James (:rkent)]

Comment on attachment 767420 [details] [diff] [review]
patch

Yes this get rid of the warning on my Mac Clang compile.

Comment 11

[:aceman]

Thanks.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/89bbcd5a2293

Comment 13

[Walter Meinl]

Just stumbled over this bug when building the brandnew source of TB-24.3.0 on Gentoo-Linux with clang-3.4. In contrast to clang-3.3 that does only warn, clang-3.4 throws the error described in comment #0.
Any chance that this fix could be checked-in also for TB-24.4.0 (waiting for the next major esr release will take more than half a year)?

Comment 14

[:aceman]

Comment on attachment 767420 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): unknown
User impact if declined: build failure in llvm/clang 3.4+ (that is now stable and going into distros)
Testing completed (on c-c, etc.): TB25, clang 3.2
Risk to taking this patch (and alternatives if risky):

Yes, we can try to get it into 24.x . Can you confirm the patch (when applied manually) does fix the building on your clang 3.4?

Comment 15

[Walter Meinl]

(In reply to :aceman from comment #14)
 
> Can you confirm the patch (when applied manually) does fix the building on your clang 3.4?

 * Applying user patches from /etc/portage/patches//mail-client/thunderbird-24.3.0 ...
 *   nsSearchTerm-clang.patch ...
 [ ok ]
>---snip---<
clang++ -o nsMsgSearchTerm.o -c -I../../../../mozilla/dist/stl_wrappers -I../../
../../mozilla/dist/system_wrappers -include /var/tmp/portage/mail-client/thunder
bird-24.3.0/work/comm-esr24/mozilla/config/gcc_hidden.h -DMOZ_GLUE_IN_PROGRAM -D
MOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_N
S_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DMOZ_THUNDER
BIRD=1 -DOSTYPE=\"Linux3.13\" -DOSARCH=Linux -DNO_NSPR_10_SUPPORT  -I/var/tmp/po
rtage/mail-client/thunderbird-24.3.0/work/comm-esr24/mailnews/base/search/src -I
. -I../../../../mozilla/dist/include -I../../../../mozilla/dist/include/nsprpub 
 -I/usr/include/nspr -I/usr/include/nss      -fPIC -Qunused-arguments  -Qunused-
arguments -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-
limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wno-c++0x-extensions -
Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -march=btver1 -pipe -Wno-return-type -w -mno-avx -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -pipe  -DNDEBUG -DTRIMMED -Os -freorder-blocks  -fomit-frame-pointer  -Qunused-arguments  -DMOZILLA_CLIENT -include ../../../../mozilla/mozilla-config.h -MD -MP -MF .deps/nsMsgSearchTerm.o.pp /var/tmp/portage/mail-client/thunderbird-24.3.0/work/comm-esr24/mailnews/base/search/src/nsMsgSearchTerm.cpp
>---snip---<
>>> Source compiled.
>>> Install thunderbird-24.3.0 into /var/tmp/portage/mail-client/thunderbird-24.3.0/image/ category mail-client
...
Works great patch applies as is and compilation ends successfully.

Comment 16

[Toni Ballesta]

I'm on the same problem. I will check the patch now.

Comment 17

[Toni Ballesta]

Yes, removing '\0' with the path solves the problem. Thanks.

Comment 18

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr24/rev/f98c23e31b02