Cherrypick clang and gcc warning fixes from upstream jemalloc

RESOLVED FIXED in Firefox 32

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

(Blocks 1 bug)

unspecified
mozilla32
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox31 wontfix, firefox32 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch jemalloc-warnings.patch (obsolete) — Splinter Review
This patch fixes the following warnings I see when building Firefox for Android (gcc) and OS X (clang):

src/prof.c:1070:11 [-Wsign-compare] comparison of integers of different signs: 'int64_t' (aka 'long long') and 'unsigned long long'
src/util.c:179:55 [-Wsign-compare] comparison of integers of different signs: 'uintmax_t' (aka 'unsigned long') and 'int'
src/util.c:180:60 [-Wsign-compare] comparison of integers of different signs: 'uintmax_t' (aka 'unsigned long') and 'int'
src/util.c:181:60 [-Wsign-compare] comparison of integers of different signs: 'uintmax_t' (aka 'unsigned long') and 'int'
src/util.c:551:35 [-Wsign-compare] signed and unsigned type in conditional expression

src/util.c:502:5 [-Wsometimes-uninitialized] variable 'val' is used uninitialized whenever switch default is taken
src/util.c:512:5 [-Wsometimes-uninitialized] variable 'val' is used uninitialized whenever switch default is taken
src/util.c:521:5 [-Wsometimes-uninitialized] variable 'val' is used uninitialized whenever switch default is taken
src/util.c:530:5 [-Wsometimes-uninitialized] variable 'val' is used uninitialized whenever switch default is taken

I fixed these warnings in upstream jemalloc here:
https://github.com/jemalloc/jemalloc/pull/86
Attachment #8433048 - Flags: review?(mh+mozilla)
Comment on attachment 8433048 [details] [diff] [review]
jemalloc-warnings.patch

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

You need to add a patch in memory/jemalloc, and reference it in memory/jemalloc/update.sh.
Attachment #8433048 - Flags: review?(mh+mozilla) → feedback+
(BTW, if you feel like looking at the warnings building on windows emits, be my guest ;) )
(In reply to Mike Hommey [:glandium] from comment #1)
> You need to add a patch in memory/jemalloc, and reference it in
> memory/jemalloc/update.sh.

Do we need an in-tree patch even though these fixes are already in jemalloc upstream? The next time we update our in-tree copy of jemalloc, these fixes won't need to be restored with a local patch.
Flags: needinfo?(mh+mozilla)
(In reply to Chris Peterson (:cpeterson) from comment #3)
> (In reply to Mike Hommey [:glandium] from comment #1)
> > You need to add a patch in memory/jemalloc, and reference it in
> > memory/jemalloc/update.sh.
> 
> Do we need an in-tree patch even though these fixes are already in jemalloc
> upstream?

All the in-tree patches are already in jemalloc upstream. Still, we want that running update.sh without updating upstream.info yields the same tree.
Flags: needinfo?(mh+mozilla)
Patch v2 adds in-tree patch 0004-Fix-clang-warnings.patch and references it in update.sh.
Attachment #8433048 - Attachment is obsolete: true
Attachment #8433907 - Flags: review?(mh+mozilla)
Attachment #8433907 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/257269554ad9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.