Bug 312980

Summary:	Building on Mountain Lion generates some compiler warnings
Product:	[Developer tools] valgrind	Reporter:	Guy Harris <gharris>
Component:	general	Assignee:	Julian Seward <jseward>
Status:	RESOLVED FIXED
Severity:	normal	CC:	flo2030
Priority:	NOR
Version First Reported In:	3.9.0.SVN
Target Milestone:	---
Platform:	Compiled Sources
OS:	macOS
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	patch to squelch the warnings patch to squelch the warnings in the VEX subtree

Description Guy Harris 2013-01-09 23:16:55 UTC

Building valgrind on Mountain Lion with Xcode 4.5.whatever (from the command line, of course :-)) generated some const-qualifier-removed warnings and some data-type warnings.

Reproducible: Always

Steps to Reproduce:
./autogen.sh; ./configure; make

Comment 1 Guy Harris 2013-01-09 23:18:26 UTC

Created attachment 76346 [details]
patch to squelch the warnings

Comment 2 Guy Harris 2013-01-09 23:20:23 UTC

There are also some signedness warnings; I haven't looked to see whether those can be cleanly and correctly fixed by changing some types.  (DEC has a lot to answer for with the sign-extending byte instructions on the PDP-11.... :-))

Comment 3 Guy Harris 2013-01-10 02:52:42 UTC

Created attachment 76355 [details]
patch to squelch the warnings in the VEX subtree

(So what's the deal with VEX?  Should patches for that be submitted here or to the external source whence it comes?)

This change turns

    vassert(imm8 >= 0 && imm8 <= 255);

into

    vassert(imm8 <= 255);

and does similar things for imm4 because imm8 and imm4 are unsigned and are thus always >= 0.  If it's considered a bit more "self-documenting" to put in a check against 0, feel free to leave that check in (or leave it in, commented out).

Comment 4 Guy Harris 2013-01-10 03:01:06 UTC

(In reply to comment #3)
> (So what's the deal with VEX?  Should patches for that be submitted here or
> to the external source whence it comes?)

I guess the answer is "the vex component"; should I file a separate bug for it, or is this sufficient?

Comment 5 Florian Krohm 2013-01-15 03:21:39 UTC

(In reply to comment #1)
> Created attachment 76346 [details]
> patch to squelch the warnings

Thanks for the patch. I've applied it as r13231.

Comment 6 Florian Krohm 2013-01-15 03:29:11 UTC

(In reply to comment #3)
> Created attachment 76355 [details]
> patch to squelch the warnings in the VEX subtree
> 
> (So what's the deal with VEX?  Should patches for that be submitted here or
> to the external source whence it comes?)

Yes, just select the VEX component. There is no separate bugzilla for VEX.

> 
> This change turns
> 
>     vassert(imm8 >= 0 && imm8 <= 255);
> 
> into
> 
>     vassert(imm8 <= 255);
> 
> and does similar things for imm4 because imm8 and imm4 are unsigned and are
> thus always >= 0.  If it's considered a bit more "self-documenting" to put
> in a check against 0, feel free to leave that check in (or leave it in,
> commented out).

Yes, this style is intentional. You'll find it all over valgrind.  You will have to silence clang somehow.
I've committed the change to guest_amd_toIR.c as VEX r2634.

Thanks for the patches.