[MSan] Tweak CopyOrigin

[stephan.yichao.zhao]

Summary

There could be some mis-alignments when copying origins not aligned.

I believe unaligned memcpy is rare so the cases do not matter too much
in practice.

1) About the change at line 50

Let dst be (void*)5,
then d=5, beg=4
so we need to write 3 (4+4-5) bytes from 5 to 7.

2) About the change around line 77.

Let dst be (void*)5,
because of lines 50-55, the bytes from 5-7 were already writen.
So the aligned copy is from 8.

[eugenis]

Hmm this code is a lot more approximate than I remember. The case where src % 4 != dst % 4 is quite broken, but you are right that it is not very common, and also ambiguous - when two poisoned 4-byte chunks with different origins contribute to the same 4-byte chunk in the output, the result can not be represented.

Your change makes it better, so LGTM but please check formatting.

compiler-rt/lib/msan/msan_poisoning.cpp

94

did this go through clang-format? formatting looks strange.

[stephan.yichao.zhao]

reformated

[stephan.yichao.zhao]

compiler-rt/lib/msan/msan_poisoning.cpp

94

tried again. The formatter made the code like this for some reason...

[eugenis]

compiler-rt/lib/msan/msan_poisoning.cpp

94

You are right, the same happens for me. OK then. clang-format is always right.

compiler-rt/lib/msan/msan_poisoning.cpp

 void CopyOrigin(const void *dst, const void *src, uptr size,
                 StackTrace *stack) {
   if (!MEM_IS_APP(dst) || !MEM_IS_APP(src)) return;
 
   uptr d = (uptr)dst;
   uptr beg = d & ~3UL;
   // Copy left unaligned origin if that memory is poisoned.
   if (beg < d) {
-    u32 o = GetOriginIfPoisoned((uptr)src, d - beg);
+    u32 o = GetOriginIfPoisoned((uptr)src, beg + 4 - d);
     if (o) {
       if (__msan_get_track_origins() > 1) o = ChainOrigin(o, stack);
       *(u32 *)MEM_TO_ORIGIN(beg) = o;
     }
     beg += 4;
   }
 
   uptr end = (d + size) & ~3UL;
   // If both ends fall into the same 4-byte slot, we are done.
   if (end < beg) return;
 
   // Copy right unaligned origin if that memory is poisoned.
   if (end < d + size) {
     u32 o = GetOriginIfPoisoned((uptr)src + (end - d), (d + size) - end);
     if (o) {
       if (__msan_get_track_origins() > 1) o = ChainOrigin(o, stack);
       *(u32 *)MEM_TO_ORIGIN(end) = o;
     }
   }
 
   if (beg < end) {
     // Align src up.
     uptr s = ((uptr)src + 3) & ~3UL;
+    uptr aligned_beg = ((uptr)dst + 3) & ~3UL;
     // FIXME: factor out to msan_copy_origin_aligned
     if (__msan_get_track_origins() > 1) {
       u32 *src = (u32 *)MEM_TO_ORIGIN(s);
       u32 *src_s = (u32 *)MEM_TO_SHADOW(s);
-      u32 *src_end = (u32 *)MEM_TO_ORIGIN(s + (end - beg));
-      u32 *dst = (u32 *)MEM_TO_ORIGIN(beg);
+      u32 *src_end = (u32 *)MEM_TO_ORIGIN(s + (end - aligned_beg));
+      u32 *dst = (u32 *)MEM_TO_ORIGIN(aligned_beg);
       u32 src_o = 0;
       u32 dst_o = 0;
       for (; src < src_end; ++src, ++src_s, ++dst) {
         if (!*src_s) continue;
         if (*src != src_o) {
           src_o = *src;
           dst_o = ChainOrigin(src_o, stack);
         }
         *dst = dst_o;
       }
     } else {
-      REAL(memcpy)((void *)MEM_TO_ORIGIN(beg), (void *)MEM_TO_ORIGIN(s),
-                   end - beg);
+      REAL(memcpy)
+      ((void *)MEM_TO_ORIGIN(aligned_beg), (void *)MEM_TO_ORIGIN(s),
+       end - aligned_beg);
     }
   }
 }
 
 void MoveShadowAndOrigin(const void *dst, const void *src, uptr size,
                          StackTrace *stack) {
   if (!MEM_IS_APP(dst)) return;
   if (!MEM_IS_APP(src)) return;