Question of TempsToInstrument - instrumentation granularity of ASan

[king19...@gmail.com]

Hello all,

I am a new to ASan and I need to modify the source code of ASan. When I trace the source code

of ASan, I have some question about the following code:

// Fill the set of memory operations to instrument.
  for (Function::iterator FI = F.begin(), FE = F.end();
       FI != FE; ++FI) {
    TempsToInstrument.clear(); // Is it possible to comment it?
    int NumInsnsPerBB = 0;
    for (BasicBlock::iterator BI = FI->begin(), BE = FI->end();
         BI != BE; ++BI) {
      if (LooksLikeCodeInBug11395(BI)) return false;
      if (Value *Addr = isInterestingMemoryAccess(BI, &IsWrite)) {
        if (ClOpt && ClOptSameTemp) {
          if (!TempsToInstrument.insert(Addr))
            continue;  // We've seen this temp in the current BB.
        }
      } else if (isa<MemIntrinsic>(BI) && ClMemIntrin) {
        // ok, take it.
      } else {
        if (CallInst *CI = dyn_cast<CallInst>(BI)) {
          // A call inside BB.
          TempsToInstrument.clear();
          if (CI->doesNotReturn()) {
            NoReturnCalls.push_back(CI);
          }
        }
        continue;
      }
      ToInstrument.push_back(BI);
      NumInsnsPerBB++;
      if (NumInsnsPerBB >= ClMaxInsnsToInstrumentPerBB)
        break;
    }
  }

The source code above indicate that a variable which is written/read in the same basic block

is instrumented once. However the same variable should be instrumented again on the other

basic block. Take the following test case as example:

==================== test.cpp =======================================

extern bool x;

int main() {

  int i;

  if (x) {

    // instrumented only once

    i = 1;

    i = 3;

  } else {

    i = 2; // instrumented

  }

  return i;

}

===================================================================

The address of i in the three assignment is all the same. The instrumentation of i = 1 & i = 3 can be 

merged, on the other hand, i = 2 is instrumented in the separated way. In my opinion, all assignment

of i may use the only one instrumentation, even they are in different basic blocks. So is it ok to

comment "TempsToInstrument.clear();", which reduce the redundancy instrumentation?

Thanks

[Dmitry Vyukov]

Hi,

If you remove TempsToInstrument.clear(), you will end up with the
following instrumentation:

if (x) {
__asan_read4(&i);

i = 1;
i = 3;
} else {
i = 2; // instrumented
}

Then, if x==0, asan won't verify access to i.

What you want to do is possible, but you must be more complex analysis
and potentially place asan instrumentation in different places. E.g.
for your example it's correct to emit either:

__asan_read4(&i);
if (x) {

i = 1;
i = 3;
} else {
i = 2;
}

or:

if (x) {

i = 1;
i = 3;
} else {
i = 2;

__asan_read4(&i);
}

However, if you have code like:

if (x) {

i = 1;
i = 3;
} else {

some_func();
i = 2;
}

The first variant is incorrect (won't catch if some_func() frees i),
while the second is still valid.

[Dmitry Vyukov]

it's meant to be: