[PATCH] [lld] ELF: Support detection of relocation errors during processing

[Philip Reames]

On 01/15/2015 10:21 AM, Benoit Belley wrote:
> I completely agree that my concern is not specific to this patch!
>
> I would simply add my vote to having a alternative non-exiting way of reporting internal errors whenever possible. This is especially important for users of the MCJIT engine. We are using it in a very large application. Our QA and management does not have much tolerance and understanding for these brute force exits… Thankfully, we haven’t encountered too many of them.
If you find one that's not a bug in your frontend, please speak up. In 
practice, I haven't really seen these happen due to missing cases in the 
optimizer or anything like that.  (Well, not that didn't get fixed 
immediately.)
>
> I feel that don’t have enough experience yet with the LLVM source code to propose an alternative solution of llvm::report_fatal_report() but I am willing to help out if someone else can lead some of the effort.
This is going to be a case of "patches welcome" I think.  No current 
contributor has cared enough.
>
> Benoit
>
>
> Le 2015-01-15 à 13:02, Philip Reames <listmail at philipreames.com> a écrit :
>
>> Your concern is a legitimate one, but it's also not specific to this patch.  The JIT generally makes very little effort to report gracefully when a compile fails.  The current philosophy is more or less that the frontend is expected to generate IR that is valid to compile; if it doesn't, that's a fatal bug in the frontend.  There is a desire to change that, but no one has really volunteered to take that on.
>>
>> Philip
>>
>> On 01/15/2015 05:05 AM, Benoit Belley wrote:
>>> Hi Everyone,
>>>
>>> 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. Its effect will be to kill the entire host
>>> application simply because one LLVM module couldn¹t be jitted.
>>>
>>> Of course, fatal errors and assertions should never occur! ;-) And, if
>>> they never occur, then there isn¹t any issue. 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.
>>>
>>> Is that a legitimate concern ? Comments ?
>>>
>>> Cheers,
>>> Benoit
>>>
>>> Benoit Belley
>>> Sr Principal Developer
>>> M&E-Product Development Group
>>> Autodesk, Inc.
>>> www.autodesk.com <http://www.autodesk.com/>
>>>
>>>
>>>
>>> On 2015-01-14 17:58, "Rui Ueyama" <ruiu at google.com> wrote:
>>>
>>>> LGTM with these fixes.
>>>>
>>>>
>>>> REPOSITORY
>>>>   rL LLVM
>>>>
>>>> ================
>>>> Comment at: lib/ReaderWriter/ELF/SectionChunks.h:424
>>>> @@ +423,3 @@
>>>> +    for (const auto ref : *definedAtom) {
>>>> +      if (std::error_code EC = relHandler.applyRelocation(*writer,
>>>> buffer,
>>>> +                                                          *ai, *ref)) {
>>>> ----------------
>>>> s/EC/ec/
>>>>
>>>> ================
>>>> Comment at: lib/ReaderWriter/ELF/SectionChunks.h:431
>>>> @@ -398,1 +430,3 @@
>>>>    });
>>>> +  if (success == false)
>>>> +    llvm::report_fatal_error("relocating output");
>>>> ----------------
>>>> if (!success)
>>>>
>>>> http://reviews.llvm.org/D6827
>>>>
>>>> EMAIL PREFERENCES
>>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits