
<html>


<head>


<meta http-equiv="Content-Type" content="text/html; charset=euc-kr">


</head>


<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">


<div>


<div><font face="Calibri,sans-serif"><span style="font-size: 11.5pt;">Thanks for taking the time to answer. My comment was by no-mean implying </span><span style="font-size: 15px;">that</span><span style="font-size: 11.5pt;"> </span><span style="font-size: 15px;">I</span><span style="font-size: 11.5pt;"> was


 requesting any immediate code change. </span><span style="font-size: 15px;">I</span><span style="font-size: 11.5pt;"> completely agree, as </span><span style="font-size: 15px;">I</span><span style="font-size: 11.5pt;"> </span><span style="font-size: 15px;">mentioned,


 in my original comment, that calling report_fatal_error() is infinitely better than crashing on segfault.</span></font></div>


</div>


<div><font face="Calibri,sans-serif"><span style="font-size: 15px;"><br>


</span></font></div>


<div>I was under the impression that the Object reader would be used when using/implementing an MCJIT object cache. Is that the case ?</div>


<div><br>


</div>


<div>Benoit</div>


<div style="color: rgb(0, 0, 0); font-family: Calibri, sans-serif; font-size: 16px;">


<br>


</div>


<span id="OLK_SRC_BODY_SECTION" style="color: rgb(0, 0, 0); font-family: Calibri, sans-serif; font-size: 16px;">


<div style="font-family:Calibri; font-size:11pt; text-align:left; color:black; BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">


<span style="font-weight:bold">From: </span>Filipe Cabecinhas <<a href="mailto:filcab+llvm.phabricator@gmail.com">filcab+llvm.phabricator@gmail.com</a>><br>


<span style="font-weight:bold">Date: </span>jeudi 15 janvier 2015 14:17<br>


<span style="font-weight:bold">To: </span>"<a href="mailto:reviews+D6945+public+88af01f8557bf636@reviews.llvm.org">reviews+D6945+public+88af01f8557bf636@reviews.llvm.org</a>" <<a href="mailto:reviews+D6945+public+88af01f8557bf636@reviews.llvm.org">reviews+D6945+public+88af01f8557bf636@reviews.llvm.org</a>><br>


<span style="font-weight:bold">Cc: </span>"<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>" <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>>, Benoit Belley <<a href="mailto:benoit.belley@autodesk.com">benoit.belley@autodesk.com</a>>,


 "<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>" <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>>, "<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>


<span style="font-weight:bold">Subject: </span>Re: [PATCH] Report fatal errors instead of segfaulting/asserting on a few invalid accesses while reading MachO files.<br>


</div>


<div><br>


</div>


<blockquote id="MAC_OUTLOOK_ATTRIBUTION_BLOCKQUOTE" style="BORDER-LEFT: #b5c4df 5 solid; PADDING:0 0 0 5; MARGIN:0 0 0 5;">


<div>


<div>


<div dir="ltr">Hi Benoit,<br>


<div class="gmail_extra"><br>


<div class="gmail_quote">On Thu, Jan 15, 2015 at 5:22 AM, Benoit Belley <span dir="ltr">


<<a href="mailto:benoit.belley@autodesk.com" target="_blank">benoit.belley@autodesk.com</a>></span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


Hi Everyone,<br>


<br>


My understanding is that the ELF writer is also used by the MCJIT engine when filling an object buffer with jitted code. In that context, I am wondering if the call to llvm::report_fatal_error() is the appropriate way to report the error. It has the effect


 of killing the entire host application simply because one LLVM module couldn¹t be jitted.<br>


</blockquote>


<div>I'm only doing changes for Mach-O, but future similar changes might hit the ELF reader.</div>


<div><br>


</div>


<div>In any case, this impacts the readers, not the writers. And, more importantly, this is covering over assertions and segfaults for bad user input. I'm not calling report_fatal_error in places where our behavior was well-defined (like when we “recovered”


 from bad user input in some way).</div>


<div>I think you'll agree that report_fatal_error is preferable to any assert or segfault, which aren't recoverable (in general). :-)</div>


<div> </div>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


Of course, fatal errors and assertions should never occur! ;-) And, if they never occur, then there isn¹t any issue. Moreover, it is much better to exit the application after a proper error message has been emitted, then crashing on a segfault! So, this patch


 is definitely a step in the right direction.<br>


</blockquote>


<div>:-)</div>


<div><br>


</div>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


On the other hand, report_fatal_error() has the unfortunate consequence of killing our application before our users have any chance of saving their work.<br>


</blockquote>


<div>We can make libObject recover from this. But that will force us to change a lot os libObject's internals (and API), since most of it isn't really ready for recovering (even if recovering means just signalling an error occurred to the caller) from bad input.</div>


<div>I have no real proposal to change the API to one that is resilient to bad input, and there are other libObject criticisms that could be addressed in a possible rewrite of the API. We can start doing it, but I'm not an expert (or even very knowledgeable)


 on most libObject uses, so I can't propose a better API right now. I can participate in the discussion if you want to propose one, though. ;-)</div>


<div><br>


</div>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


Thus, I am wondering if there would be a way to indicate to the MCJIT engine that an internal error occurred while jitting a particular LLVM module. The compilation of that module would abort, but the application would not exit.<br>


<br>


Is that a legitimate concern ? Comments ?<br>


</blockquote>


<div>I don't really think this changes anything for the current users of libObject (whatever hits report_fatal_error was either reading bad memory, asserting, or screwing up the file's semantics in a way that was most likely not even compatible with whatever


 other tools exist (dynamic loaders, otool, etc), which made it useless to try to keep reading the file).</div>


<div><br>


</div>


<div><br>


</div>


<div>If you still have issues, or think we didn't account for something, let me know.</div>


<div><br>


</div>


<div>Regards,</div>


<div><br>


</div>


<div>  Filipe</div>


<div> </div>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<br>


Cheers,<br>


Benoit<br>


<span class="HOEnZb"><font color="#888888"><br>


Benoit Belley<br>


Sr Principal Developer<br>


M&E-Product Development Group<br>


Autodesk, Inc.<br>


<a href="http://www.autodesk.com" target="_blank">www.autodesk.com</a> <a href="http://www.autodesk.com/" target="_blank">


http://www.autodesk.com/</a><br>


</font></span>


<div class="HOEnZb">


<div class="h5"><br>


<br>


<a href="http://reviews.llvm.org/D6945" target="_blank">http://reviews.llvm.org/D6945</a><br>


<br>


EMAIL PREFERENCES<br>


  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">


http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>


<br>


<br>


</div>


</div>


</blockquote>


</div>


<br>


</div>


</div>


</div>


</div>


</blockquote>


</span>


</body>


</html>