On Wed, Nov 26, 2014 at 11:21:06AM -0700, ygribov wrote: > > Formatting. The {} should be indented like static > > and return 2 columns to the right of that. > > Right. > > > For base_addr computation, you don't really need g or ptr_checks, > > do you? So why not move the: > > auto_vec<gimple> *ptr_checks = &ctx->asan_check_map.get_or_insert (ptr); > > gimple g = maybe_get_dominating_check (*ptr_checks); > > lines below the if? > > I can do this. But then base_checks would be invalidated when I call > get_or_insert for ptr_checks so I'll still have to hash_map::get.
You're right. > > If asan (kernel-address) is > > recovering, I don't see a difference from not reporting two different > > invalid accesses to the same function and not reporting two integer > > overflows in the same function, at least if they have different > > location_t. > > Ok, agreed. BTW how about replacing '& SANITIZE_KERNEL_ADDRESS' with '& > SANITIZE_ADDRESS'? I know we do not support recovery for userspace but > having a general enum sounds more logical. Testing SANITIZE_ADDRESS bit in flag_sanitize_recover doesn't make sense, testing it in flag_sanitize of course does, but for recover you care whether the SANITIZE_{KERNEL,USER}_ADDRESS bit in flag_sanitize_recover is set depending on if SANITIZE_{KERNEL,USER}_ADDRESS is set in flag_sanitize_recover. So supposedly ((flag_sanitize & flag_sanitize_recover) & (SANITIZE_USER_ADDRESS | SANITIZE_KERNEL_ADDRESS)) != 0 is the right check for whether the current address sanitization wants to recover. Jakub