Turn on -Werror=int-to-pointer-cast globally

RESOLVED FIXED in mozilla29

Status

defect
RESOLVED FIXED
7 years ago
Last year

People

(Reporter: bjacob, Assigned: sylvestre)

Tracking

unspecified
mozilla29
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

This is a really good warning. It catches code such as

  int i;
  foo((void*)i);

on platforms where sizeof(int) != sizeof(void*). There is no valid use case for wanting to do that, but sometimes we have to because of bad APIs that want void* arguments; in which case we can still get it to compile by doing

  int i;
  foo((void*)(intptr_t)i);

So, I don't see any downside to turning this on globally (as opposed to just in WARNINGS_AS_ERRORS=1 directories).

See bug 781545 for the original motivation/example.
I do: we don't have tinderbox builds that support that warning yet.
Yeah, that's a blocker.  It seems like it's first in gcc 4.6.
It's not in clang?
I've looked for a list of warnings supported by Clang, but annoyingly, I didn't find any.  "man gcc" has a list for gcc (although I don't strictly know if it's complete, and it doesn't give first introduced version).  If you can find a list somewhere, please share!
(In reply to :Aryeh Gregor from comment #4)
> I've looked for a list of warnings supported by Clang, but annoyingly, I
> didn't find any.  "man gcc" has a list for gcc (although I don't strictly
> know if it's complete, and it doesn't give first introduced version).  If
> you can find a list somewhere, please share!

$ diagtool list-warnings
$ diagtool tree
Posted patch fix-bug-781552.diff (obsolete) — Splinter Review
The attached patch should fix the bug.

Clang supports this option at from version 3.2 (the 3.4 rc1 is just out).
Attachment #8339321 - Flags: review?
Assignee: nobody → sylvestre
Comment on attachment 8339321 [details] [diff] [review]
fix-bug-781552.diff

Thanks for the patch! This looks totally sensible, but it could stand a run by the try server. Also, we probably want to put this same thing in js/src/configure.in.

Do you have access to push patches to Try?
Attachment #8339321 - Flags: review? → review+
Sure, here it is.

but I don't have any permissions :)
Attachment #8339321 - Attachment is obsolete: true
Attachment #8339338 - Flags: review?
Comment on attachment 8339338 [details] [diff] [review]
fix-bug-781552.diff

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

r=ted
Attachment #8339338 - Flags: review?
Keywords: checkin-needed
Thanks for launching the build.

Some debug builds are broken with this patch:

../../../tools/trace-malloc/tmreader.c: In function 'tmreader_eventloop':
../../../tools/trace-malloc/tmreader.c:527:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
../../../tools/trace-malloc/tmreader.c:548:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

I reported the bug 944892
Depends on: 944892
https://hg.mozilla.org/mozilla-central/rev/bbebc7b81f0a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Depends on: 962821
Whiteboard: [qa-]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.