Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Coverity: cannot ROTL_UV (U64) a U32 #13827

Closed
p5pRT opened this issue May 13, 2014 · 6 comments
Closed

[PATCH] Coverity: cannot ROTL_UV (U64) a U32 #13827

p5pRT opened this issue May 13, 2014 · 6 comments

Comments

@p5pRT
Copy link

p5pRT commented May 13, 2014

Migrated from rt.perl.org#121858 (status was 'resolved')

Searchable as RT121858$

@p5pRT
Copy link
Author

p5pRT commented May 13, 2014

From @jhi

The hash perturbation code tried to rotate U32 left by 64 bits, which
makes no sense. Well, one could devise *some* meaning for that
operation, but then the 'standard' C code for rotating a U64 no more
makes sense, and would need to be revised.

(patch confirmed okay by Yves)

Patch attached.

@p5pRT
Copy link
Author

p5pRT commented May 13, 2014

From @jhi

0001-Cannot-rotl-u32-hek_hash-by-64-bits.patch
From ff6534a20287888e07baa6a68da33d3819211b02 Mon Sep 17 00:00:00 2001
From: Jarkko Hietaniemi <jhi@iki.fi>
Date: Tue, 13 May 2014 08:18:05 -0400
Subject: [PATCH] Cannot rotl u32 (hek_hash) by 64 bits.

Fix for Coverity perl5 CID 28935:
Operands don't affect result (CONSTANT_EXPRESSION_RESULT)
result_independent_of_operands: (unsigned long)entry->hent_hek->hek_hash >> 47 /* 64 - 17 */ is 0 regardless of the values of its operands. This occurs as the bitwise second operand of '|'.
---
 hv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hv.c b/hv.c
index ef686ab..4577363 100644
--- a/hv.c
+++ b/hv.c
@@ -1259,7 +1259,7 @@ S_hsplit(pTHX_ HV *hv, STRLEN const oldsize, STRLEN newsize)
                  * and use the new low bit to decide if we insert at top,
                  * or next from top. IOW, we only rotate on a collision.*/
                 if (aep[j] && PL_HASH_RAND_BITS_ENABLED) {
-                    PL_hash_rand_bits+= ROTL_UV(HeHASH(entry), 17);
+                    PL_hash_rand_bits+= ROTL32(HeHASH(entry), 17);
                     PL_hash_rand_bits= ROTL_UV(PL_hash_rand_bits,1);
                     if (PL_hash_rand_bits & 1) {
                         HeNEXT(entry)= HeNEXT(aep[j]);
-- 
1.9.2

@p5pRT
Copy link
Author

p5pRT commented May 15, 2014

From @tonycoz

On Tue May 13 08​:11​:45 2014, jhi wrote​:

The hash perturbation code tried to rotate U32 left by 64 bits, which
makes no sense. Well, one could devise *some* meaning for that
operation, but then the 'standard' C code for rotating a U64 no more
makes sense, and would need to be revised.

(patch confirmed okay by Yves)

Patch attached.

Added to 5.21.1 blockers.

Tony

@p5pRT
Copy link
Author

p5pRT commented May 15, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

From @tsee

Thanks, applied as 3f49e76.

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

@tsee - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant