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: regcomp.c: bit-or used when bit-and meant #13770

Closed
p5pRT opened this issue Apr 26, 2014 · 7 comments
Closed

[PATCH] Coverity: regcomp.c: bit-or used when bit-and meant #13770

p5pRT opened this issue Apr 26, 2014 · 7 comments
Labels

Comments

@p5pRT
Copy link

p5pRT commented Apr 26, 2014

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

Searchable as RT121738$

@p5pRT
Copy link
Author

p5pRT commented Apr 26, 2014

From @jhi

regcomp.c​:14693-ish​:

operator_confusion​: ret->flags | 0x10 is always 1/true regardless of
the values of its operand. This occurs as the logical first operand of
'&&'. Did you intend to use '&' rather than '|'?

- && ! ((ANYOF_FLAGS(ret) | ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))
+ && ! ((ANYOF_FLAGS(ret) & ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))

(confirmed by Karl Williamson)

Attached.

@p5pRT
Copy link
Author

p5pRT commented Apr 26, 2014

From @jhi

0001-Fix-for-Coverity-perl5-CID-28936.patch
From e57c6ffa8fb795aa1ac77f67ee99dbe96df8b5aa Mon Sep 17 00:00:00 2001
From: Jarkko Hietaniemi <jhi@iki.fi>
Date: Fri, 25 Apr 2014 08:25:52 -0400
Subject: [PATCH] Fix for Coverity perl5 CID 28936: Wrong operator used
 (CONSTANT_EXPRESSION_RESULT)

operator_confusion: ret->flags | 0x10 is always 1/true regardless of
the values of its operand. This occurs as the logical first operand of '&&'.
Did you intend to use '&' rather than '|'?

(confirmed by Karl Williamson)
---
 regcomp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/regcomp.c b/regcomp.c
index ca2ffb8..b18022a 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -14690,7 +14690,7 @@ parseit:
            /* We don't optimize if we are supposed to make sure all non-Unicode
             * code points raise a warning, as only ANYOF nodes have this check.
             * */
-        && ! ((ANYOF_FLAGS(ret) | ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))
+        && ! ((ANYOF_FLAGS(ret) & ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))
     {
         UV start, end;
         U8 op = END;  /* The optimzation node-type */
-- 
1.9.2

@p5pRT
Copy link
Author

p5pRT commented Apr 27, 2014

From @tonycoz

On Sat Apr 26 09​:41​:48 2014, jhi wrote​:

regcomp.c​:14693-ish​:

operator_confusion​: ret->flags | 0x10 is always 1/true regardless of
the values of its operand. This occurs as the logical first operand of
'&&'. Did you intend to use '&' rather than '|'?

- && ! ((ANYOF_FLAGS(ret) | ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))
+ && ! ((ANYOF_FLAGS(ret) & ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))

(confirmed by Karl Williamson)

Attached.

This appears (to me) to be a lost optimization opportunity rather than a critical bug.

Added as a 5.21.1 blocker so it will be applied after 5.20 is released.

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 27, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Apr 27, 2014

From @khwilliamson

On 04/27/2014 05​:43 PM, Tony Cook via RT wrote​:

On Sat Apr 26 09​:41​:48 2014, jhi wrote​:

regcomp.c​:14693-ish​:

operator_confusion​: ret->flags | 0x10 is always 1/true regardless of
the values of its operand. This occurs as the logical first operand of
'&&'. Did you intend to use '&' rather than '|'?

- && ! ((ANYOF_FLAGS(ret) | ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))
+ && ! ((ANYOF_FLAGS(ret) & ANYOF_WARN_SUPER) && ALWAYS_WARN_SUPER))

(confirmed by Karl Williamson)

Attached.

This appears (to me) to be a lost optimization opportunity rather than a critical bug.

Correct

Added as a 5.21.1 blocker so it will be applied after 5.20 is released.

Tony

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

From @tsee

Thanks, applied as a482c76.

@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
Projects
None yet
Development

No branches or pull requests

1 participant