<!DOCTYPE html>
<html>
<head>
<title></title>
<style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style>
</head>
<body><div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;"><br></div>
<div><br></div>
<div>On Mon, Jun 25, 2018, at 3:22 PM, David Blaikie via cfe-dev wrote:<br></div>
<blockquote type="cite"><div dir="ltr"><div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;"><br></div>
<div defang_data-gmailquote="yes"><div dir="ltr">On Mon, Jun 25, 2018 at 12:16 PM George Karpenkov <<a href="mailto:ekarpenkov@apple.com">ekarpenkov@apple.com</a>> wrote:<br></div>
<blockquote defang_data-gmailquote="yes" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;"><div style="word-wrap:break-word;"><div><div style="font-family:Arial;"><br></div>
<blockquote type="cite"><div>On Jun 25, 2018, at 12:13 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div>
<div style="font-family:Arial;"><br></div>
<div><div dir="ltr"><div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;"><br></div>
<div defang_data-gmailquote="yes"><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 defang_data-gmailquote="yes" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;"><div style="word-wrap:break-word;"><div><div style="font-family:Arial;"><br></div>
<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">cfe-dev@lists.llvm.org</a>> wrote:<br></div>
<div style="font-family:Arial;"><br></div>
<div><div dir="ltr"><div style="font-family:Arial;">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></div>
<div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;">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>
</div>
</blockquote><div><br></div>
</div>
</div>
<div style="word-wrap:break-word;"><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.<br></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.<br></div>
</div>
</div>
</blockquote><div><div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;">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>
</div>
<blockquote defang_data-gmailquote="yes" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;"><div style="word-wrap:break-word;"><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;"><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.<br></div>
</div>
</div>
</blockquote><div><div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;">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>
</div>
</div>
</div>
</div>
</blockquote></div>
</div>
</blockquote></div>
</div>
</blockquote><div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;">One alternative is to have a special build where you redefine asserts to throw exceptions. Accomplishes similar goals without crashing the process or requiring forking. I've found (not with Clang) that customers are happy when they can copy-and-paste the contents of a dialog which says "BUG!" as opposed to a full on crash, especially if you can guarantee that the state of the application is safe (as it should be, if that particular set of assertions fires.)<br></div>
<div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;">When the exception is thrown, you could silently log the relevant information and ask the user to submit as a bug over email.<br></div>
<div style="font-family:Arial;"><br></div>
<div style="font-family:Arial;">Sohail</div>
<div style="font-family:Arial;"><br></div>
</body>
</html>