<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<blockquote
cite="mid:CAAJHn49jN4FyPd_Ex=Lems4ST-mKr26Bb9+2w3aQkO9pk87DoA@mail.gmail.com"
type="cite">
<div dir="ltr">Thanks Anton for looking at the issue, modelling
"memcpy" seems like the right solution, are you saying that code
written to model "assignment" could be reused to model "memcpy"
?</div>
</blockquote>
Just an idea, the memcpy action looks similar to assignment at first
glance.<br>
Here is a comment from CString checker relevant to the topic:<br>
// Invalidate the destination (regular invalidation without
pointer-escaping<br>
// the address of the top-level region).<br>
// FIXME: Even if we can't perfectly model the copy, we should
see if we<br>
// can use LazyCompoundVals to copy the source values into the
destination.<br>
// This would probably remove any existing bindings past the end
of the<br>
// copied region, but that's still an improvement over blank
invalidation.<br>
<br>
<br>
<blockquote
cite="mid:CAAJHn49jN4FyPd_Ex=Lems4ST-mKr26Bb9+2w3aQkO9pk87DoA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
<div>The patch you provided definitely fixes the issue thanks,
on the other hand if I understand correctly, it seems to me it
can lead to new false positives, as the analyzer would wrongly
assume that 'a.data''s content remains unchanged after the
memcpy call ?</div>
<div><br>
</div>
<div>I am thinking it might be possible to modify the region
store invalidation worker to restrict the invalidation in the
case of arrays by adding a new flag like you did, for example
TK_InvalidateArrayOnly ?</div>
</div>
</blockquote>
Here an issue is related to the struct, not array. However arrays
are also in the risk group.<br>
I thought of something like TK_DoNotInvalidateSuperRegion. Worker's
AddToWorkList() automatically adds superregion to worklist:<br>
bool AddToWorkList(const MemRegion *R) {<br>
const MemRegion *BaseR = R->getBaseRegion();<br>
return AddToWorkList(WorkListElement(BaseR), getCluster(BaseR));<br>
}<br>
With TK_DoNotInvalidateSuperRegion we could selectively localize
invalidation to a particular subregion, not the whole superregion.
This could work both for classes and arrays. Perhaps additional
coding in invalidateRegionsWorker::VisitCluster() is required.<br>
What do you think?<br>
<br>
<br>
<blockquote
cite="mid:CAAJHn49jN4FyPd_Ex=Lems4ST-mKr26Bb9+2w3aQkO9pk87DoA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Thanks for your help,</div>
<div><br>
</div>
<div>Pierre Gousseau</div>
<div>SN Systems - Sony Computer Entertainment Group</div>
<div>
<div>
<div><br>
</div>
</div>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On 16 July 2015 at 20:59, Anton Yartsev
<span dir="ltr"><<a moz-do-not-send="true" href="mailto:anton.yartsev@gmail.com" target="_blank">anton.yartsev@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>Perhaps the ideal solution is to model "memcpy"
properly (similar to assignment?). As you correctly
noticed currently we just invalidate the destination
buffer in the CString checker and the invalidation
worker invalidates the whole struct (do not exactly know
the reason for this).<br>
While experimented wrote a patch (attached) that fixes
the issue by adding a trait for the struct region that
prevents the region from being invalidated. The solution
does not cause regressions in the clang testsuite. Do
not know if the solution is acceptable as a short term
fix. Please review!<br>
<br>
</div>
<div>
<div class="h5">
<blockquote type="cite">
<div dir="ltr">Ping !
<div>Adding analyzer experts to cc.</div>
<div><br>
</div>
<div>Regards,</div>
<div><br>
</div>
<div>
<div style="font-size:12.8000001907349px">Pierre
Gousseau</div>
<div style="font-size:12.8000001907349px">SN
Systems - Sony Computer Entertainment</div>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On 2 July 2015 at 09:06,
Pierre Gousseau <span dir="ltr"><<a moz-do-not-send="true" href="mailto:pierregousseau14@gmail.com" target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:pierregousseau14@gmail.com">pierregousseau14@gmail.com</a></a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div dir="ltr">
<div>Dear All,</div>
<div><br>
</div>
<div>I have been looking into PR22954 which
has been kindly raised by krzystof at <a moz-do-not-send="true" href="https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D22954&d=AwMDaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=7uflCt79iL-ecQRK6131Rm86ntE89BlzH5o_4W-AR6Y&s=h7QrSYXR7kaCxfn1t5ZNKbPzt0d9OQiKzOV3wgW-knI&e=" target="_blank"><a class="moz-txt-link-freetext" href="https://llvm.org/bugs/show_bug.cgi?id=22954">https://llvm.org/bugs/show_bug.cgi?id=22954</a></a>
and being new to this area of Clang I
would appreciate any tips on how to fix
it.</div>
<div><br>
</div>
<div>To me the root of the issue seems to
originate from the CString checker as it
performs invalidation of the destination
buffer.</div>
<div>Given the snippet below:</div>
<div>-----------------</div>
<div>struct aa { char *s; char data[32];};</div>
<div>...</div>
<div>a.s = malloc(nbytes);</div>
<div>memcpy(a.data, source, len);</div>
<div>...</div>
<div>-----------------</div>
<div>As the CString checker handles the
memcpy call, it requests the invalidation
of the 'a.data' region. But the
invalidation worker seems to consider that
the whole memory region of 'a' has to be
invalidated. The Malloc checker is not
made aware of this causing the false
positive.</div>
<div><br>
</div>
<div>It seems a short term fix could be to
detect this specific case and have the
CString checker notify the Malloc checker
that it should stop tracking 'a.s'.</div>
<div>However this solution would reduce the
number of genuine defects detected.</div>
<div><br>
</div>
<div>So I would be grateful if someone could
give some hints on how to provide the
right solution.</div>
<div><br>
</div>
<div>Regards,</div>
<div><br>
</div>
<div>Pierre Gousseau</div>
<div>SN Systems - Sony Computer
Entertainment</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
<br>
</div>
</div>
<span class="HOEnZb"><font color="#888888">
<pre cols="72">--
Anton</pre>
</font></span></div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Anton</pre>
</body>
</html>