[llvm-commits] PATCH: lld ReaderELF.cpp update

[Sid Manning]

On 07/20/12 11:52, Sid Manning wrote:
>
> The updated patch is attached and I detailed the changes below.

I can't commit to llvm but I know others that can.  Can this and the 
ELF.h changes be committed?

Thanks,

>
> If this patch is ok then it seems to me that the next logical thing to
> work on would be the ELFWriter. Along those lines I've started looking
> at the Mach-O writer as my guide. The writer portion is more complex
> than the reader and I think that making interim drops will be helpful so
> that you guys and others keep tabs on the direction and coding decisions
> being made.
>
> In looking at, WriterMachO.cpp the first thing I noticed was it depended
> on FileOutputBuffer but there is a patch out there for it so I will pull
> that in and see how it works.
>
> A range of different processors will use ELF each with their own set of
> relocation types. Is my reading of the code correct that
> SectionChunk::write's applyFixup call where the target specific relocs
> will be handled? That will be nice if that works that way.
>
>
> On 07/19/12 16:46, Michael Spencer wrote:
>
>>> + virtual uint64_t size() const {
>>> +
>>> + // Common symbols are not allocated in object files so
>>> + // their size is zero.
>>> + if ((Symbol->getType() == llvm::ELF::STT_COMMON)
>>> + || Symbol->st_shndx == llvm::ELF::SHN_COMMON)
>>> + return (uint64_t)0;
>>> +
>>> + return (uint64_t) Symbol->st_size;
>>> + }
>>
>> This is still wrong. It should return the data size.
>>
> The attached patch fixes this: return Data.size();
>
>>> +
>>> + virtual Alignment alignment() const {
>>> +
>>> + // Unallocated common symbols specify their alignment
>>> + // constraints in st_value.
>>> + if ((Symbol->getType() == llvm::ELF::STT_COMMON)
>>> + || Symbol->st_shndx == llvm::ELF::SHN_COMMON) {
>>> + return (Alignment (Symbol->st_value));
>>
>> Space between Alignment and (. This occurs a few other places.
>>
> This also changed: return (Alignment(Symbol->st_value));
>
>
>>>
>>
>> Other than those few comments it looks good.
>>
>> - Michael Spencer
>
> Thanks
>
>
>


-- 
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum