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
