llvm dev - Jul 2016 - Should analyses be able to hold AssertingVH to IR? (related to PR28400)

On Mon, Jul 11, 2016 at 11:17 PM, Sanjoy Das <sanjoy at
playingwithpointers.com
> wrote:
> Hi Sean,
>
> Sean Silva wrote:
> >
> >     But asan won't catch problems (insofar I understand how it
works) if
> >     the free'ed BasicBlock is used as a key in a DenseMap or
something --
> >     if another BasicBlock gets allocated to the same location
we'll end
> up
> >     returning bad cached results.
> >
> >
> > ASan's allocator is specifically hardened against reusing memory,
which
> > mitigates this, but I'm not sure by how much (but I think by a
> > significant amount).
>
> Do you mean it will re-use memory less often?  Won't that just hide
> the bug above?  If anything, I don't want ASan to "mitigate"
bugs, I
> want it to make the bug trigger more often. :)
>
ASan's reuses it less often, but keeps it poisoned so that dangling
pointers get caught. This makes it less likely that re-use will cause
invalid analysis results. BUT it makes it more likely that when we access a
dangling pointer, it falls into a poisoned heap area. So the net result is
that it catches dangling pointers better.

Or to put it another way, the "heap slot reuse causes invalid analysis
results" situation is actually a subset of "we access a dangling
pointer"
which is what we really want to catch (I mean "dangling" in a sense
that a
pointer stays "dangling" even if its heap slot is reused). By avoiding
reuse of heap slots, the dangling pointer is more likely to be in a heap
slot that ASan is keeping poisoned and not reusing (hence it can detect the
error).

-- Sean Silva

>
> (Or did I just re-state what you were saying?)
>
> -- Sanjoy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160711/f89e07d8/attachment.html>

Hi Sean,

Sean Silva wrote:
 >
 > ASan's reuses it less often, but keeps it poisoned so that dangling
pointers get caught. This makes it less likely that
 > re-use will cause invalid analysis results. BUT it makes it more likely
that when we access a dangling pointer, it falls
 > into a poisoned heap area. So the net result is that it catches dangling
pointers better.
 >
 > Or to put it another way, the "heap slot reuse causes invalid
analysis results" situation is actually a subset of "we
 > access a dangling pointer"

I'm not sure about this -- is manipulating a pointer as an integer
(without loading or storing through it) included in "access"?  IOW,
will asan catch this bug at all:

DenseMap M;
M[BB] = 5;
delete BB;
BB = new BasicBlock()
return M[BB];

?

It seems to be fine with

int main(int, char **) {
      int *f = new int[5];
      delete[]f;
      return f == (int*) 100;
}


 > which is what we really want to catch (I mean "dangling" in a
sense that a pointer stays
 > "dangling" even if its heap slot is reused). By avoiding reuse
of heap slots, the dangling pointer is more likely to be
 > in a heap slot that ASan is keeping poisoned and not reusing (hence it can
detect the error).
 >
 > -- Sean Silva
 >
 >
 >     (Or did I just re-state what you were saying?)
 >
 >     -- Sanjoy
 >
 >

On Tue, Jul 12, 2016 at 12:48 AM Sanjoy Das <sanjoy at
playingwithpointers.com>
wrote:
> Hi Sean,
>
> Sean Silva wrote:
>  >
>  > ASan's reuses it less often, but keeps it poisoned so that
dangling
> pointers get caught. This makes it less likely that
>  > re-use will cause invalid analysis results. BUT it makes it more
likely
> that when we access a dangling pointer, it falls
>  > into a poisoned heap area. So the net result is that it catches
> dangling pointers better.
>  >
>  > Or to put it another way, the "heap slot reuse causes invalid
analysis
> results" situation is actually a subset of "we
>  > access a dangling pointer"
>
> I'm not sure about this -- is manipulating a pointer as an integer
> (without loading or storing through it) included in "access"?

No, sadly.

However, ASan provides tools that we could extend things like DenseMap
with. For example, we could potentially have DenseMap check whether any
pointer keys in the map are poisoned. We probably wouldn't want to *always*
do this, but we could have a particular mode that controls this inside of
DenseMap.

Then, when accessing the map (say, clearing, rebucketing, probing, with an
explicit "check" or a randomized walk of the entire table) we could
have it
directly call __asan_address_is_poisoned or we could have it directly read
(and discard) the first byte of underlying storage at that address. This is
guaranteed to be safe and correct for valid pointers but will synthesize a
nice error if a dangling pointer is left in the map.

We could probably handle most common idioms by allowing you to directly
*remove* a dangling pointer, but crashing if we probe past one and
periodically (on lookups only) check the entire map. I suspect we might
even be able to make that the default for non-"void*" keys and change
anyone using raw pointers that aren't to allocated objects to use
"void*"
as the key type.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160712/403266a0/attachment.html>

On Tue, Jul 12, 2016 at 12:48 AM, Sanjoy Das <sanjoy at
playingwithpointers.com
> wrote:
> Hi Sean,
>
> Sean Silva wrote:
> >
> > ASan's reuses it less often, but keeps it poisoned so that
dangling
> pointers get caught. This makes it less likely that
> > re-use will cause invalid analysis results. BUT it makes it more
likely
> that when we access a dangling pointer, it falls
> > into a poisoned heap area. So the net result is that it catches
dangling
> pointers better.
> >
> > Or to put it another way, the "heap slot reuse causes invalid
analysis
> results" situation is actually a subset of "we
> > access a dangling pointer"
>
> I'm not sure about this -- is manipulating a pointer as an integer
> (without loading or storing through it) included in "access"? 
IOW,
> will asan catch this bug at all:
>
> DenseMap M;
> M[BB] = 5;
> delete BB;
> BB = new BasicBlock()
> return M[BB];
>
> ?
>
> It seems to be fine with
>
> int main(int, char **) {
>      int *f = new int[5];
>      delete[]f;
>      return f == (int*) 100;
>
> }
>

Yeah, this is the reason that I qualified my ASan comment with "if we do
try to access through any of these dangling pointers".

-- Sean Silva

>
>
> > which is what we really want to catch (I mean "dangling" in
a sense that
> a pointer stays
> > "dangling" even if its heap slot is reused). By avoiding
reuse of heap
> slots, the dangling pointer is more likely to be
> > in a heap slot that ASan is keeping poisoned and not reusing (hence it
> can detect the error).
> >
> > -- Sean Silva
> >
> >
> >     (Or did I just re-state what you were saying?)
> >
> >     -- Sanjoy
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160712/44d3da65/attachment.html>