Bug 389065 - valgrind meets gcc flag -Wlogical-op
Summary: valgrind meets gcc flag -Wlogical-op
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: 3.13 SVN
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Ivo Raisr
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-16 17:55 UTC by dcb314
Modified: 2018-01-23 09:44 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
patch (2.69 KB, patch)
2018-01-18 16:23 UTC, Ivo Raisr
Details

Note You need to log in before you can comment on or make changes to this bug.
Description dcb314 2018-01-16 17:55:22 UTC
1.


priv/guest_arm_toIR.c: In function ‘decode_V6MEDIA_instruction’:
priv/guest_arm_toIR.c:9149:31: warning: logical ‘or’ of collectively exhaustive tests is always true [-Wlogical-op]
             (INSNA(6,6)  == 0 || INSNA(6,6) == 1) ) {
                               ^~

I am guessing that INSNA(6,6) is only one bit.

2.

m_syswrap/syswrap-xen.c: In function ‘vgSysWrap_xen_domctl_before’:
m_syswrap/syswrap-xen.c:1281:84: warning: logical ‘or’ of equal expressions [-Wlogical-op]

          if (domctl->u.monitor_op_0000000b.op == VKI_XEN_DOMCTL_MONITOR_OP_ENABLE ||
              domctl->u.monitor_op_0000000b.op == VKI_XEN_DOMCTL_MONITOR_OP_ENABLE) {

Maybe the second one should be DISABLE ?

I suggest also add gcc flag -Wlogical-op to future builds
to prevent this kind of bug happening again.
Comment 1 Ivo Raisr 2018-01-16 19:49:20 UTC
Thank you for the bug report.
Indeed, the second one is a bug in Valgrind code.
The first one is not a bug but merely a mental note for maintainer that this piece of code handles both pkhbt (tb == 0) and pkhbt (tb == 1) variants.

Would you like to supply a patch for both the occurrences?
Would you like to supply a patch for extending Valgrind build with -Wlogical-op (have a look at configure.ac, similar flags are enabled there).
Comment 2 dcb314 2018-01-16 19:58:43 UTC
>Would you like to supply a patch for both the occurrences?
>Would you like to supply a patch for extending Valgrind build

No thanks. I just find them. Folks who know the code are 
in a better state to fix them.
Comment 3 Ivo Raisr 2018-01-18 16:23:24 UTC
Created attachment 109970 [details]
patch
Comment 4 Ivo Raisr 2018-01-18 16:26:11 UTC
The attached patch enables -Wlogical-op (if supported) and fixes an obvious typo.
I used https://github.com/mirage/xen/blob/master/tools/libxc/xc_monitor.c for an inspiration.

My gcc 7.2.0 on Ubuntu 17.10 did not report a problem in priv/guest_arm_toIR.c with -Wlogical-op. I guess the first reported problem must have been caused by some other factor.
Comment 5 Ivo Raisr 2018-01-23 09:44:50 UTC
Fixed in commit f19a956e0a96b4f6b37c50b00ae24ecf09a7a3f5.

https://sourceware.org/git/?p=valgrind.git;a=commitdiff;h=f19a956e0a96b4f6b37c50b00ae24ecf09a7a3f5