search

yibaini / thread-sanitizer

Automatically exported from code.google.com/p/thread-sanitizer
0 stars 0 forks source link

Bit logic issue in UnalignedMemoryAccess #86

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
In file tsan_rtl.cc line 611:

if (size >= 8 && (addr & ~7) == ((addr + 8) & ~7))

will always be false. addr+8 changes the 4th bit, so the masked compare will 
always fail.

Also the further comparisons don't meet the intention:
e.g., addr:=0x4, size:=4

(addr & ~7) = 0x0
((addr + 4) & ~7) = 0x10

But touching {0x4,0x5,0x6,0x7} fits into the 8 byte block starting at 0x0.

I guess the ifs should read like:

    if (size >= 8 && (addr & ~7) == ((addr + 7) & ~7)) {
...
    } else if (size >= 4 && (addr & ~7) == ((addr + 3) & ~7)) {
...
    } else if (size >= 2 && (addr & ~7) == ((addr + 1) & ~7)) {

Original issue reported on code.google.com by protze.j...@gmail.com on 16 Dec 2014 at 4:54

GoogleCodeExporter commented 9 years ago 
What revision are you looking at?
At current llvm tip (revision 224009), the code looks as:

void UnalignedMemoryAccess(ThreadState *thr, uptr pc, uptr addr,
    int size, bool kAccessIsWrite, bool kIsAtomic) {
  while (size) {
    int size1 = 1;
    int kAccessSizeLog = kSizeLog1;
    if (size >= 8 && (addr & ~7) == ((addr + 7) & ~7)) {
      size1 = 8;
      kAccessSizeLog = kSizeLog8;
    } else if (size >= 4 && (addr & ~7) == ((addr + 3) & ~7)) {
      size1 = 4;
      kAccessSizeLog = kSizeLog4;
    } else if (size >= 2 && (addr & ~7) == ((addr + 1) & ~7)) {
      size1 = 2;
      kAccessSizeLog = kSizeLog2;
    }
    MemoryAccess(thr, pc, addr, kAccessSizeLog, kAccessIsWrite, kIsAtomic);
    addr += size1;
    size -= size1;
  }
}

Original comment by dvyu...@google.com on 17 Dec 2014 at 7:55

GoogleCodeExporter commented 9 years ago 
Sorry for the confusion, I was not aware of the fact that I looked at the 3.5 
release branch.

Original comment by protze.j...@gmail.com on 18 Dec 2014 at 9:41

GoogleCodeExporter commented 9 years ago 
no problem
thanks for raising the potential issue

Original comment by dvyu...@google.com on 18 Dec 2014 at 9:53

GoogleCodeExporter commented 9 years ago 
Adding Project:ThreadSanitizer as part of GitHub migration.

Original comment by gli...@google.com on 30 Jul 2015 at 9:21