False aliasing between shadow, origin and app memory?

[ramosian-glider]

Originally reported on Google Code with ID 7

The following code,

void f(int* p, int x) {
  *p = x;
}

with -O2 -mllvm -msan-store-clean-origin=0 -mllvm -msan-track-origins=1 -mllvm -msan-check-access-address=0
is compiled into

_Z1fPii():
   0:   48 b8 ff ff ff ff ff    mov    $0xffffbfffffffffff,%rax
   7:   bf ff ff 
   a:   48 21 f8                and    %rdi,%rax
   d:   48 8b 0d 00 00 00 00    mov    0x0(%rip),%rcx        # 14 <_Z1fPii+0x14>
            10: R_X86_64_GOTTPOFF   __msan_param_origin_tls-0x4
  14:   64 8b 49 08             mov    %fs:0x8(%rcx),%ecx
  18:   48 8b 15 00 00 00 00    mov    0x0(%rip),%rdx        # 1f <_Z1fPii+0x1f>
            1b: R_X86_64_GOTTPOFF   __msan_param_tls-0x4
  1f:   64 8b 52 08             mov    %fs:0x8(%rdx),%edx
  23:   89 10                   mov    %edx,(%rax)
  25:   85 d2                   test   %edx,%edx
  27:   74 13                   je     3c <_Z1fPii+0x3c>
  29:   48 ba 00 00 00 00 00    mov    $0x200000000000,%rdx
  30:   20 00 00 
  33:   48 01 d0                add    %rdx,%rax
  36:   48 83 e0 fc             and    $0xfffffffffffffffc,%rax
  3a:   89 08                   mov    %ecx,(%rax)
  3c:   89 37                   mov    %esi,(%rdi)
  3e:   c3                      retq   


Looks like the load from origin tls could be moved below the branch at offset 27, but
it's not, because it may be aliasing the store of the shadow value at offset 23.

Does it sound right? Could we use TBAA to tell LLVM that these are different memory
locations?

Reported by eugenis@google.com on 2012-11-19 12:43:34

[ramosian-glider]

+1 for using TBAA. I can handle it later, once the rest of the code is in. 

Reported by kcc@chromium.org on 2012-11-19 12:48:13

[ramosian-glider]

Humm... what if you just explicitly move the load below the branch? It looks like the
way to go. Then the compiler does not need to figure out anything.

Reported by dvyukov@google.com on 2012-11-19 12:56:29

[ramosian-glider]

tbaa is the general mechanism, if we can reply on it and save a bit of code in our phase
-- we should

Reported by konstantin.s.serebryany on 2012-11-19 13:04:04

[ramosian-glider]

Yes, there must be a lot of other optimization possibilities suppressed by this issue.
Asan may benefit from tbaa, too.

Reported by eugenis@google.com on 2012-11-19 13:14:36

[ramosian-glider]

It seems like llvm (and gcc, too) simply does not do this optimization.
See an example:

int f(bool z, long long* y, long long* y1, int* p, int* p1) {
  long long x = *y;
  *p1 = 0;
  if (__builtin_expect(z, 0)) {
    *y1 = x;
  }
  return 2;
}

I could not make any of the compilers move the load of (*y) under if().

Reported by eugenis@google.com on 2012-11-22 11:12:52

[ramosian-glider]

Adding Project:MemorySanitizer as part of GitHub migration.

Reported by glider@google.com on 2015-07-30 09:22:25

• Labels added: ProjectMemorySanitizer

[morehouse]

@eugenis, @dvyukov, @kcc: Bug is 5 years old. What is the status of this?