Bug 382041 - False uninitialized on bit packing when the compiler chooses XOR to implement masking and shifting (x86_64)
Summary: False uninitialized on bit packing when the compiler chooses XOR to implement...
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: 3.14 SVN
Platform: Other Other
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-06 08:02 UTC by Alex
Modified: 2017-11-13 12:11 UTC (History)
0 users

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


Attachments
Full output (20.46 KB, text/plain)
2017-07-06 08:02 UTC, Alex
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex 2017-07-06 08:02:07 UTC
Created attachment 106457 [details]
Full output

While troubleshooting an image processing program, I've received a lot of warnings regarding uninitialized values after running a custom bit packing routine. Managed to narrow down and write a minimal example:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

void __attribute__((noinline))
set(uint16_t * x, uint16_t val, uint16_t mask)
{
    *x = (*x & ~mask) | ((val << 4) & mask);
}

int main()
{
    /* allocate 2 bytes of uninitialized memory */
    uint16_t * buf = malloc(2);
    /* write 0x2340 in 2 steps, using 2 bit masks and a shift*/
    /* this way, some compilers are "tempted" to use xor */
    /* exact mask value doesn't matter - it's complemented */
    set(buf, 0x1234, 0x003F);
    set(buf, 0x1234, ~0x003F);
    /* print the result => 0x2340 */
    printf("%x\n", *buf);
    free(buf);
    return 0;
}

Compiled with e.g.:
gcc bit.c -O2

Results:

gcc version 5.2.1 20151010 (Ubuntu 5.2.1-22ubuntu2) and
gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.4):

...
==22509== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
...
==22509== Use of uninitialised value of size 8
==22509==    at 0x4E81711: _itoa_word (_itoa.c:180)
==22509==    by 0x4E85A78: vfprintf (vfprintf.c:1641)
==22509==    by 0x4F4E955: __printf_chk (printf_chk.c:35)
==22509==    by 0x400545: main (in /home/alex/vg/a.out)
...
==22509== ERROR SUMMARY: 20 errors from 10 contexts (suppressed: 0 from 0)

objdump a.out -d -M intel | grep "<set>:" -A 8
0000000000400670 <set>:
  400670:	0f b7 07             	movzx  eax,WORD PTR [rdi]
  400673:	c1 e6 04             	shl    esi,0x4
  400676:	31 c6                	xor    esi,eax
  400678:	21 d6                	and    esi,edx
  40067a:	31 f0                	xor    eax,esi
  40067c:	66 89 07             	mov    WORD PTR [rdi],ax
  40067f:	c3                   	ret    

gcc version 4.8.5 (Ubuntu 4.8.5-1ubuntu1) 
==1105== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

0000000000400660 <set>:
  400660:	89 d0                	mov    eax,edx
  400662:	c1 e6 04             	shl    esi,0x4
  400665:	f7 d0                	not    eax
  400667:	66 23 07             	and    ax,WORD PTR [rdi]
  40066a:	21 d6                	and    esi,edx
  40066c:	09 f0                	or     eax,esi
  40066e:	66 89 07             	mov    WORD PTR [rdi],ax
  400671:	c3                   	ret    

Ubuntu clang version 3.6.2-1 (tags/RELEASE_362/final) (based on LLVM 3.6.2)
==1302== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

0000000000400590 <set>:
  400590:	89 d0                	mov    eax,edx
  400592:	f7 d0                	not    eax
  400594:	66 23 07             	and    ax,WORD PTR [rdi]
  400597:	c1 e6 04             	shl    esi,0x4
  40059a:	21 d6                	and    esi,edx
  40059c:	09 c6                	or     esi,eax
  40059e:	66 89 37             	mov    WORD PTR [rdi],si
  4005a1:	c3                   	ret    

My conclusion: the warning only appears when the compiler implements these bitfield-like operations with XOR.




Another example, this time without shift. However, it only triggers the warning on gcc 5.2.1, not on 5.4.0.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

int main()
{
    /* allocate 2 bytes of uninitialized memory */
    uint16_t * buf = malloc(2);

    /* write 0x1234 in 2 steps, using 2 bit masks */
    /* exact mask value doesn't matter */
    const uint16_t mask1 = 0x3F;
    *buf = (*buf & ~mask1) | (0x1234 & mask1);
    /* complement the mask and write the other "half" of the number */
    uint16_t mask2 = ~mask1;
    *buf = (*buf & ~mask2) | (0x1234 & mask2);
    /* print the result => 0x1234 */
    printf("%x\n", *buf);
    free(buf);
    return 0;
}

gcc bit.c -O2

This is clean, as the compiler optimizes it out (because the masks are constant).

0000000000400470 <main>:
  400470:	48 83 ec 08          	sub    rsp,0x8
  400474:	ba 34 12 00 00       	mov    edx,0x1234
  400479:	be 24 06 40 00       	mov    esi,0x400624
  40047e:	bf 01 00 00 00       	mov    edi,0x1
  400483:	31 c0                	xor    eax,eax
  400485:	e8 d6 ff ff ff       	call   400460 <__printf_chk@plt>

Remove one of the const's (say the second):

==25688== ERROR SUMMARY: 9 errors from 5 contexts (suppressed: 0 from 0)
gcc version 5.2.1 20151010 (Ubuntu 5.2.1-22ubuntu2) 
0000000000400500 <main>:
  400500:	53                   	push   rbx
  400501:	bf 02 00 00 00       	mov    edi,0x2
  400506:	e8 d5 ff ff ff       	call   4004e0 <malloc@plt>
  40050b:	48 89 c3             	mov    rbx,rax
  40050e:	0f b7 00             	movzx  eax,WORD PTR [rax]
  400511:	be c4 06 40 00       	mov    esi,0x4006c4
  400516:	bf 01 00 00 00       	mov    edi,0x1
  40051b:	83 e0 c0             	and    eax,0xffffffc0
  40051e:	89 c2                	mov    edx,eax
  400520:	80 f4 12             	xor    ah,0x12
  400523:	83 ca 34             	or     edx,0x34
  400526:	31 d0                	xor    eax,edx
  400528:	0f b7 d0             	movzx  edx,ax
  40052b:	31 c0                	xor    eax,eax
  40052d:	e8 be ff ff ff       	call   4004f0 <__printf_chk@plt>

On gcc 5.4, removing "const" doesn't change the disassembly (therefore it doesn't trigger the warnings); the compiler apparently figures out the second mask is constant.

Valgrind versions tested:
valgrind-3.11.0 (prepackaged with Ubuntu)
valgrind-3.13.0 (compiled from source)
valgrind-3.14.0.SVN rev 16458 (compiled from source)

Attached full output for both examples, using valgrind from SVN and gcc 5.2.1.

Thanks for reading.
Comment 1 Julian Seward 2017-11-13 08:37:56 UTC
I'm not sure if it exactly the same, but Valgrind does already have
machinery which transforms the xor-based scheme back into the
and-or-not scheme, in order to avoid this problem.  It is disabled
by default though.  Find this at the bottom of ir_opt.c and enable
it, then see if that helps.

      ///////////////////////////////////////////////////////////
      // BEGIN MSVC optimised code transformation hacks
      if (0)
         bb = do_MSVC_HACKS(bb);
      // END   MSVC optimised code transformation hacks
      ///////////////////////////////////////////////////////////
Comment 2 Julian Seward 2017-11-13 08:41:54 UTC
Possibly-related-to bug 344382.
Comment 3 Alex 2017-11-13 12:11:46 UTC
Found, tried on the first example, but the valgrind output did not change at all.

Double-checked by adding a while(1) in do_MSVC_HACKS to make sure it gets called.