<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 25, 2018 at 11:51 AM George Karpenkov <<a href="mailto:ekarpenkov@apple.com">ekarpenkov@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><br><blockquote type="cite"><div>On Jun 25, 2018, at 9:38 AM, David Blaikie via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="m_4029422686001650388Apple-interchange-newline"><div><div dir="ltr">So, yes, the reading of the Clang AST is a bit of a special case & hasn't met this interpretation of "do not crash on arbitrary inputs" - but the intent is that it does not crash on ASTs produced by the same version of Clang (if you model the "system" as the totality of a single clang writing something to disk then reading it back again - that is not expected to crash). So any assertions that are firing in that situation should be, I believe, considered to be programmer errors.<br><br>Are you going outside that scope - and dealing with 'untrusted' ASTs potentially generated from other versions of Clang? Coming from unknown sources, etc?<br></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div>Generating AST by hand, or taking them from elsewhere is known to be very fragile, and we don’t really have a check for it.</div><div>In that case assertions are a *good* thing, as at least they say that something went wrong as opposed to e.g. silently ignoring the AST.</div></div></div></blockquote><div><br>Depends what the source is/what they're doing - might be a "best effort" sort of thing. "Hey, I have some AST lying around - analyze whatever parts of it you can".<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div><div dir="ltr">If so, then, yeah - forking is probably the only good bet, I would imagine.<br></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div>A proper fix here would be to e.g. have an ASTVerifier which could take an AST DAG and then return whether it’s valid or not.</div></div></div></blockquote><div><br>Yep, that'd be the ideal - though that's a lot of work (the set of invariants is large/not well known - basically all we have is "ASTs are correct by construction in Clang" as the definition of invariants) & I could imagine that these folks aren't in a position to do all that work - so "try to load it and bail out on assertions/crashes" might be the best tradeoff for them.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><div>Any solutions bypassing asserts are *bad* as they actively hide bugs, which decreases the overall quality.</div><div><br></div><div>Displaying errors to users is a different matter entirely, and should be done in a different layer.</div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><br></div><blockquote type="cite"><div><div dir="ltr"><br>If you're within that scope - patches and bugs are likely to be quite welcome. Though, again, if you're dealing with totally arbitrary C++ - Clang is far from fuzzer-clean and it's not too hard to find (especially/mostly invalid) inputs that can crash Clang - if those would present a major problem to your product, then again forking may be necessary (as much as it might be nice to cleanup all/most of those cases, it would probably be a large undertaking).<br><br><div class="gmail_quote"><div dir="ltr">On Sun, Jun 24, 2018 at 2:25 AM Gábor Márton via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>We are struggling with the same problem with my team. We work on cross translation unit (CTU) static analysis. Sometimes the AST node of a function imported by the ASTImporter is not complete or wrong which causes assertion failures in the analysis engine or in the checkers. However if we had known that the function could not be imported then we wouldn't have started to import that, this way preventing the assert to fire and the crash of the process. Note, when the assertions are disabled, usually a segmentation fault happens.</div><div><br></div><div>The original intention of the assertions in the LLVM infrastructure is to signal programming errors. They are really useful to catch errors - made by the LLVM/Clang programmer - as early as possible.<br></div><div>See <a href="https://llvm.org/docs/CodingStandards.html#assert-liberally" target="_blank">https://llvm.org/docs/CodingStandards.html#assert-liberally</a> .</div><div><div style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Generally, we say that we should distinguish between programming errors and errors in the input. Programming errors should be handled with asserts, errors in the input should be handled with other means (some projects use exceptions).</span></div></div><div><div style="text-decoration-style:initial;text-decoration-color:initial">However, in our special case of the CTU, the AST is an input. Which can be wrongly assembled by the ASTImporter. Unfortunately the checkers cannot distinguish whether the error is because of a programming error in the checker itself or whether the AST as an input is wrong.<br></div></div><div style="text-decoration-style:initial;text-decoration-color:initial"><br></div><div style="text-decoration-style:initial;text-decoration-color:initial">Here is what we thought so far: Do a fork before the actual import and if the child died then don't start the import at all. This seems quite complicated and performance issues may arise. Also this may just hide some real errors, so we did not start to implement it. Rather we have the strategy now to gradually fix all the assertions. Sooner or later the ASTImporter will provide a complete and correct AST and the checkers are also changed to be able to handle a crippled AST. Until then we have crashes but every week we have fewer. And very importantly, we plan to hide the crash report from our users since it is useful only to the developers.</div><div style="text-decoration-style:initial;text-decoration-color:initial"><br></div><div style="text-decoration-style:initial;text-decoration-color:initial">Hope this helps,</div><div style="text-decoration-style:initial;text-decoration-color:initial">Gabor</div><div><br></div><div>   <br></div></div><br><div class="gmail_quote"><div dir="ltr">On Sun, Jun 24, 2018 at 10:24 AM alan snape via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">My team is developing a static analysis tool based on clang and llvm, but the assertion failures in the source code of llvm and clang will always crash the program execution, which is not acceptable in a **stable product**. The tool analyzes the functions one by one in the Call Graph SCC order, so is there any way to tolerate the assertion failures and continue the analysis on the next function when assertion failures occur on calling some APIs of clang and llvm? (crashes only the analysis of the function<span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span> </span>(the analysis methods of the FunctionDecl)</span> being analyzed, not the entire program)<div><br></div><div>Thanks,</div><div>Ella</div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>
_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br></div></blockquote></div></div></blockquote></div></div>