develooper Front page | perl.perl5.porters | Postings from May 2013

Re: [perl #117835] Pointer cast to integer of different size inhv_func.h

Thread Previous | Thread Next
From:
Nicholas Clark
Date:
May 2, 2013 13:05
Subject:
Re: [perl #117835] Pointer cast to integer of different size inhv_func.h
Message ID:
20130502130518.GS3729@plum.flirble.org
On Thu, May 02, 2013 at 10:33:12PM +1000, sisyphus1@optusnet.com.au wrote:
> 
> 
> -----Original Message----- 
> From: Nicholas Clark
> Sent: Thursday, May 02, 2013 9:07 PM
> To: demerphq
> Cc: Perl5 Porteros ; bugs-bitbucket@rt.perl.org
> Subject: Re: [perl #117835] Pointer cast to integer of different size in 
> hv_func.h
> 
> > On Thu, May 02, 2013 at 12:46:30PM +0200, demerphq wrote:
> > > On 2 May 2013 12:34, Nicholas Clark <nick@ccl4.org> wrote:
> > > > On Thu, May 02, 2013 at 12:23:18PM +0200, demerphq wrote:
> > > >> Sorry about that, probably my bad porting skills at fault. I will
> > > >> apply your patch as soon as I can.
> > > >
> > > > It needs testing on Win64 first.
> > >
> > > Any reason Win64 is special in this regard?
> >
> > Because the warning is not from your code. It's from a change made to your
> > code to fix a build failure on 64 bit Windows:
> >
> > https://rt.perl.org/rt3/Ticket/Display.html?id=117687
> 
> It was I that created that ticket.
> 
> I've tried Robin's patch on 5.17.11 - it's fine for me on Win64. (I haven't 
> tested Win32.)

Cool, thanks. I hadn't yet asked you, because I was going to verify that the
automated smokers were happy with it first, before troubling a human, and
the branch was still in the queue.

> > So changing it has the potential to break their build.
> > [64 bit Windows is special because sizeof(void *) > sizeof(long)]
> >
> > I'd spotted this warning a few days ago and had pushed a branch recently
> > (smoke-me/nicholas/rt-117687) with my second go at a fix. And I'm now
> > particularly wary of that line because the first thing that I thought 
> > would
> > work didn't. (Squashed the warning on 32 bit platforms, caused a warning 
> > on
> > 64 bit platforms)
> >
> > Also, I don't understand why it was a build *failure* on Win64, and not 
> > just
> > a warning. This whole thing is behaving strangely.
> 
> It *is* strange.
> I would think (without actually knowing) that kmx has checked to see that 
> 5.17.10 and 5.17.11 build ok for x64 Strawberry Perl. If that's the case, 
> then he's obviously found no problem - as we've heard no complaint from him.
> Strawberry Perl uses an older version of gcc (version 4.6.3) whereas I'm 
> using 4.7.0. I was therefore wondering whether the problem I faced was 
> specific to 4.7.0 (and maybe even later versions, too.)
> 
> I also use 4.7.0 for my 32-bit builds. There was no problem there ... but, 
> of course, 'IV' and 'long' are the same thing on 32-bit builds.

Well, they are by default. The warning is cropping up on *nix when one
runs Configure with -Duse64bitint on a 32 bit platform, so that IVs are
long longs.


Also, Robin's attached *patch* has the 1 line commit message
"avoid cast from pointer to integer of different size"

which doesn't reference the RT number (it can't - RT allocated the ticket
number after submission), and doesn't include the extra discussion in the
patch. I think the commit message should be something more like

commit 275227021431a7dbfb1c18e88a68a35ce0851161
Author: Robin Barker <rmbarker@cpan.org>
Date:   Wed May 1 18:39:26 2013 +0100

    Use PTR2IV() instead of casting a pointer to IV, to avoid warnings.
    
    The cast to IV was added to avoid problems on platforms where pointers are
    larger than longs. However, the change instead generates warnings on
    platforms where IVs are larger than pointers. Instead, use the PTR2IV()
    macro provided by perl.h to make everyone happy.
    
    Also change the type of the variable on that line, because it is cast to
    STRLEN the first time it is used, and then passed to a macro that assigns it
    to an int regardless.

diff --git a/hv_func.h b/hv_func.h
index 0a66356..9dfe158 100644
--- a/hv_func.h
+++ b/hv_func.h
@@ -392,8 +392,8 @@ S_perl_hash_murmur3(const unsigned char * const seed, const 
     /* This CPU does not handle unaligned word access */
 
     /* Consume enough so that the next data byte is word aligned */
-    int i = -(IV)ptr & 3;
-    if(i && (STRLEN)i <= len) {
+    STRLEN i = -PTR2IV(ptr) & 3;
+    if(i && i <= len) {
       MURMUR_DOBYTES(i, h1, carry, bytes_in_carry, ptr, len);
     }
 
(which I've just pushed to smoke-me/nicholas/rt-117687
In case George's Win32 smoker is able to test this sooner than Steve on Win32)

Nicholas Clark

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About