Bug 389065

Summary: valgrind meets gcc flag -Wlogical-op
Product: [Developer tools] valgrind Reporter: dcb314
Component: generalAssignee: Ivo Raisr <ivosh>
Status: RESOLVED FIXED    
Severity: normal CC: ivosh
Priority: NOR    
Version First Reported In: 3.13 SVN   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: patch

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