[PATCH] D24060: [lld][ELF] Add support for -b binary

[Rui Ueyama via llvm-commits]

The ELF writer in this patch seems a bit too abstracted to use only in LLD
but a bit too less abstracted to be a generic library, so I'd think it'll
have to be improved (to whichever). But this is not something we should
worry too much about it, as you said, as this is a relatively small stuff.

On Fri, Sep 9, 2016 at 11:11 AM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> On Fri, Sep 9, 2016 at 10:47 AM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Is that your plan?
>>
>
> I'm not sure there is a "plan" per se, but Michael's patch has it as a
> local utility for the linker since that is how we have been using it. You
> can think about it similar to the CPIO utilities -- they start out as a
> local utility for ELF, then can be generalized to any other tools that need
> it. I haven't looked, but my guess is that llvm-dwp could use ELFCreator
> (or an evolved version) and potentially MC; also this may be useful for
> future tools like "llvm-strip" or "llvm-objcopy".
>
> I don't think we should worry too much about where it will live. We can
> easily move it as needed.
>
> -- Sean Silva
>
>
>>
>> On Fri, Sep 9, 2016 at 3:04 AM, Sean Silva <chisophugis at gmail.com> wrote:
>>
>>>
>>>
>>> On Thu, Sep 8, 2016 at 6:16 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>
>>>> I'm a little confused. I was thinking that you wanted to upstream your
>>>> local change to create ELF files in-memory to libSupport or something and
>>>> then remove this code from LLD. Was I wrong?
>>>>
>>>
>>> I don't have an issue with it living in libSupport (or potentially
>>> libObject since it fits there more naturally).
>>>
>>> -- Sean Silva
>>>
>>>
>>>>
>>>> On Thu, Sep 8, 2016 at 6:00 PM, Sean Silva <chisophugis at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, Sep 8, 2016 at 5:17 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>>>
>>>>>> OK, I don't want to make future upstreaming unnecessarily hard by
>>>>>> requesting changes to that file, so I'm okay with that file structure. That
>>>>>> said, please keep it in mind that this file would diverge from what it is
>>>>>> today once we submit this to the open source repository because people
>>>>>> would try to improve it, and we can't stop them if it is a good change.
>>>>>> Even I would try to make changes to that file if upstreaming won't be done
>>>>>> in a timely manner.
>>>>>>
>>>>>
>>>>> Feel free to commit any genuine improvements to the code; we merge
>>>>> daily and will pick them up. Just please do so in the context that this is
>>>>> shared independent functionality, even though upstream may not reflect that.
>>>>>
>>>>> -- Sean Silva
>>>>>
>>>>>
>>>>>>
>>>>>> On Thu, Sep 8, 2016 at 5:04 PM, Sean Silva <chisophugis at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Sep 8, 2016 at 3:19 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>>>>>
>>>>>>>> On Thu, Sep 8, 2016 at 3:14 PM, Sean Silva <chisophugis at gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> silvas added a subscriber: silvas.
>>>>>>>>>
>>>>>>>>> ================
>>>>>>>>> Comment at: ELF/InputFiles.cpp:736
>>>>>>>>> @@ +735,3 @@
>>>>>>>>> +template <class ELFT> std::unique_ptr<InputFile>
>>>>>>>>> BinaryFile::createELF() {
>>>>>>>>> +  ELFCreator<ELFT> ELF(ET_REL, Config->EMachine);
>>>>>>>>> +  auto DataSec = ELF.addSection(".data");
>>>>>>>>> ----------------
>>>>>>>>> ruiu wrote:
>>>>>>>>> > Bigcheese wrote:
>>>>>>>>> > > ruiu wrote:
>>>>>>>>> > > > How about this?
>>>>>>>>> > > Well, the code doesn't belong in ELFCreator.cpp. I can make a
>>>>>>>>> BinaryFile.cpp for it, but it's only 40 lines of code.
>>>>>>>>> > That distinction doesn't make much sense to me because this is
>>>>>>>>> part of the linker. If you have a concrete plan to move ELFCreator to some
>>>>>>>>> library, it may make sense, but it doesn't seem to happen soon.
>>>>>>>>> Like Michael already said, ELFCreator is already used in at least
>>>>>>>>> 2 other places for PS4. It doesn't make sense to move this
>>>>>>>>> BinaryFile-specific code into ELFCreator.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Is that change for PS4 upstreamed?
>>>>>>>>
>>>>>>>
>>>>>>> Not yet. Right now, we are required to keep those private, but we
>>>>>>> try to do things so that they are ready to be sent upstream if we get the
>>>>>>> approval to do so (e.g. we merge the latest LLD daily).
>>>>>>>
>>>>>>> -- Sean Silva
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160909/28d085cc/attachment.html>