[lld] r219176 - Do not use llvm_unreachable at reachable code.

[Rui Ueyama]

On Mon, Oct 6, 2014 at 6:25 PM, Saleem Abdulrasool <compnerd at compnerd.org>
wrote:

> On Mon, Oct 6, 2014 at 5:40 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Author: ruiu
>> Date: Mon Oct  6 19:40:54 2014
>> New Revision: 219176
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=219176&view=rev
>> Log:
>> Do not use llvm_unreachable at reachable code.
>>
>> These lines can be reachable if we give a broken or unsupported
>> input object file.
>>
>
> Should we really be relying on this for invalid input detection?  We
> should immediately identify if the object file is incorrect when we snarf
> the input object and it mismatches the target (I recently added that
> support).  Yes, I agree that it could be a broken input, but, shouldn't we
> have better detection for that in libObject rather than just happily
> consuming it?
>

This report_fatal_error is for unknown relocation types. That's different
from file magic. I don't think we want libObject to check if a relocation
type is within a known range for an architecture. If we make libObject to
check for that, it would return a reference only when a reference type
value is valid. That's fine. But we'd still want to keep this
now-really-unreachable error check anyway to cover all switch cases. That
doesn't seem a good deal to me to error out on an broken input file, which
is a rare event.


>
>> Modified:
>>     lld/trunk/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
>>
>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp?rev=219176&r1=219175&r2=219176&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp (original)
>> +++ lld/trunk/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp Mon Oct  6
>> 19:40:54 2014
>> @@ -558,7 +558,7 @@ void AtomChunk::applyRelocationsX86_32(u
>>              targetAddr - getSectionStartAddr(targetAddr, sectionRva);
>>          break;
>>        default:
>> -        llvm_unreachable("Unsupported relocation kind");
>> +        llvm::report_fatal_error("Unsupported relocation kind");
>>        }
>>      }
>>    }
>> @@ -618,7 +618,7 @@ void AtomChunk::applyRelocationsX86_64(u
>>          break;
>>        default:
>>          llvm::errs() << "Kind: " << (int)ref->kindValue() << "\n";
>> -        llvm_unreachable("Unsupported relocation kind");
>> +        llvm::report_fatal_error("Unsupported relocation kind");
>>        }
>>      }
>>    }
>> @@ -934,7 +934,7 @@ StringRef chooseSectionByContent(const D
>>    }
>>    llvm::errs() << "Atom: contentType=" << atom->contentType()
>>                 << " permission=" << atom->permissions() << "\n";
>> -  llvm_unreachable("Failed to choose section based on content");
>> +  llvm::report_fatal_error("Failed to choose section based on content");
>>  }
>>
>>  typedef std::map<StringRef, std::vector<const DefinedAtom *> >
>> AtomVectorMap;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141006/b6886bc6/attachment.html>