[lld] r230138 - [ELF][Writer] Use Path to create AtomSection.
Rui Ueyama
ruiu at google.com
Thu Mar 5 13:48:32 PST 2015
Then maybe we need a test. Also I hope you read replies a bit carefully in
future so that we don't need 4 round trips to just let you know that there
seems a functionality change despite the commit message.
On Thu, Mar 5, 2015 at 1:42 PM, Shankar Easwaran <shankare at codeaurora.org>
wrote:
> Ah ok, so there was a little change in functionality, which clearly showed
> there was a section ordering issue that would have resulted with this
> change.
>
>
> On 3/5/2015 3:16 PM, Rui Ueyama wrote:
>
>> Seems that cannot describe the change you made to MIPS. This change makes
>> it something special for MIPS if a section name is ".text". Is this really
>> for debugging and reporting errors?
>>
>> On Thu, Mar 5, 2015 at 1:12 PM, Shankar Easwaran <shankare at codeaurora.org
>> >
>> wrote:
>>
>> This code helps to debug and check for errors when there is something
>>> wrong with the final layout of the image.
>>>
>>> Hope this helps.
>>>
>>> Shankar Easwaran
>>>
>>>
>>> On 3/5/2015 2:56 PM, Rui Ueyama wrote:
>>>
>>> So, why did you have to fix that FIXME in the first place? This patch
>>>> cannot fix anything if it didn't change any functionality.
>>>>
>>>> On Thu, Mar 5, 2015 at 12:54 PM, Shankar Easwaran <
>>>> shankare at codeaurora.org>
>>>> wrote:
>>>>
>>>> - // FIXME: We really need the file path here in the SectionKey, when
>>>>
>>>>> that
>>>>> - // is available, replace the sectionKey that has outputSectionName
>>>>> to
>>>>> the
>>>>> - // inputSectionName.
>>>>>
>>>>> Shankar Easwaran
>>>>>
>>>>>
>>>>> On 3/5/2015 2:51 PM, Rui Ueyama wrote:
>>>>>
>>>>> What did you address? The commit message says that there's no
>>>>>
>>>>>> functionality
>>>>>> change by this commit.
>>>>>>
>>>>>> On Thu, Mar 5, 2015 at 12:41 PM, Shankar Easwaran <
>>>>>> shankare at codeaurora.org>
>>>>>> wrote:
>>>>>>
>>>>>> Sorry, I missed your ping.
>>>>>>
>>>>>> There was a FIXME comment that I addressed as part of the commit.
>>>>>>> Previously I had to disable because of the RoundTripPasses.
>>>>>>>
>>>>>>> Shankar Easwaran
>>>>>>>
>>>>>>>
>>>>>>> On 3/5/2015 2:29 PM, Rui Ueyama wrote:
>>>>>>>
>>>>>>> Ping. This added extra code but no additional functionality? I'll
>>>>>>> roll
>>>>>>>
>>>>>>> this
>>>>>>>> back if it is so.
>>>>>>>>
>>>>>>>> On Wed, Feb 25, 2015 at 3:30 PM, Rui Ueyama <ruiu at google.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Ping?
>>>>>>>>
>>>>>>>> On Sun, Feb 22, 2015 at 7:36 PM, Rui Ueyama <ruiu at google.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> On Sat, Feb 21, 2015 at 7:49 AM, Shankar Easwaran <
>>>>>>>>>
>>>>>>>>> shankare at codeaurora.org> wrote:
>>>>>>>>>
>>>>>>>>>> Author: shankare
>>>>>>>>>>
>>>>>>>>>> Date: Sat Feb 21 09:49:34 2015
>>>>>>>>>>
>>>>>>>>>>> New Revision: 230138
>>>>>>>>>>>
>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=230138&view=rev
>>>>>>>>>>> Log:
>>>>>>>>>>> [ELF][Writer] Use Path to create AtomSection.
>>>>>>>>>>>
>>>>>>>>>>> Now since the correct file path for atoms is available and not
>>>>>>>>>>> clobbered,
>>>>>>>>>>> commit r222309 which was reverted previously can be added back.
>>>>>>>>>>>
>>>>>>>>>>> No change in functionality.
>>>>>>>>>>>
>>>>>>>>>>> Why do we need this new code if it doesn't change anything?
>>>>>>>>>>>
>>>>>>>>>>> Modified:
>>>>>>>>>>>
>>>>>>>>>> lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h
>>>>>>>>>>
>>>>>>>>>>> lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h
>>>>>>>>>>>
>>>>>>>>>>> Modified: lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h
>>>>>>>>>>> URL:
>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/
>>>>>>>>>>> ReaderWriter/ELF/
>>>>>>>>>>> DefaultLayout.h?rev=230138&r1=230137&r2=230138&view=diff
>>>>>>>>>>>
>>>>>>>>>>> ============================================================
>>>>>>>>>>> ==================
>>>>>>>>>>> --- lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h (original)
>>>>>>>>>>> +++ lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h Sat Feb 21
>>>>>>>>>>> 09:49:34
>>>>>>>>>>> 2015
>>>>>>>>>>> @@ -91,24 +91,26 @@ public:
>>>>>>>>>>> // The sections are created using
>>>>>>>>>>> // SectionName, contentPermissions
>>>>>>>>>>> struct SectionKey {
>>>>>>>>>>> - SectionKey(StringRef name, DefinedAtom::ContentPermissions
>>>>>>>>>>> perm)
>>>>>>>>>>> - : _name(name), _perm(perm) {
>>>>>>>>>>> - }
>>>>>>>>>>> + SectionKey(StringRef name, DefinedAtom::ContentPermissions
>>>>>>>>>>> perm,
>>>>>>>>>>> + StringRef path)
>>>>>>>>>>> + : _name(name), _perm(perm), _path(path) {}
>>>>>>>>>>>
>>>>>>>>>>> // Data members
>>>>>>>>>>> StringRef _name;
>>>>>>>>>>> DefinedAtom::ContentPermissions _perm;
>>>>>>>>>>> + StringRef _path;
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> struct SectionKeyHash {
>>>>>>>>>>> int64_t operator()(const SectionKey &k) const {
>>>>>>>>>>> - return llvm::hash_combine(k._name, k._perm);
>>>>>>>>>>> + return llvm::hash_combine(k._name, k._perm, k._path);
>>>>>>>>>>> }
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> struct SectionKeyEq {
>>>>>>>>>>> bool operator()(const SectionKey &lhs, const SectionKey
>>>>>>>>>>> &rhs)
>>>>>>>>>>> const
>>>>>>>>>>> {
>>>>>>>>>>> - return ((lhs._name == rhs._name) && (lhs._perm ==
>>>>>>>>>>> rhs._perm));
>>>>>>>>>>> + return ((lhs._name == rhs._name) && (lhs._perm ==
>>>>>>>>>>> rhs._perm)
>>>>>>>>>>> &&
>>>>>>>>>>> + (lhs._path == rhs._path));
>>>>>>>>>>> }
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> @@ -181,9 +183,10 @@ public:
>>>>>>>>>>> virtual StringRef getOutputSectionName(StringRef
>>>>>>>>>>> inputSectionName)
>>>>>>>>>>> const;
>>>>>>>>>>>
>>>>>>>>>>> /// \brief Gets or creates a section.
>>>>>>>>>>> - AtomSection<ELFT> *getSection(
>>>>>>>>>>> - StringRef name, int32_t contentType,
>>>>>>>>>>> - DefinedAtom::ContentPermissions contentPermissions);
>>>>>>>>>>> + AtomSection<ELFT> *
>>>>>>>>>>> + getSection(StringRef name, int32_t contentType,
>>>>>>>>>>> + DefinedAtom::ContentPermissions
>>>>>>>>>>> contentPermissions,
>>>>>>>>>>> + StringRef path);
>>>>>>>>>>>
>>>>>>>>>>> /// \brief Gets the segment for a output section
>>>>>>>>>>> virtual Layout::SegmentType getSegmentType(Section<ELFT>
>>>>>>>>>>> *section)
>>>>>>>>>>> const;
>>>>>>>>>>> @@ -530,22 +533,19 @@ AtomSection<ELFT> *DefaultLayout<ELFT>::
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> template <class ELFT>
>>>>>>>>>>> -AtomSection<ELFT> *DefaultLayout<ELFT>::getSection(
>>>>>>>>>>> - StringRef sectionName, int32_t contentType,
>>>>>>>>>>> - DefinedAtom::ContentPermissions permissions) {
>>>>>>>>>>> - // FIXME: We really need the file path here in the SectionKey,
>>>>>>>>>>> when
>>>>>>>>>>> that
>>>>>>>>>>> - // is available, replace the sectionKey that has
>>>>>>>>>>> outputSectionName
>>>>>>>>>>> to
>>>>>>>>>>> the
>>>>>>>>>>> - // inputSectionName.
>>>>>>>>>>> - StringRef outputSectionName = getOutputSectionName(
>>>>>>>>>>> sectionName);
>>>>>>>>>>> - const SectionKey sectionKey(outputSectionName, permissions);
>>>>>>>>>>> +AtomSection<ELFT> *
>>>>>>>>>>> +DefaultLayout<ELFT>::getSection(StringRef sectionName, int32_t
>>>>>>>>>>> contentType,
>>>>>>>>>>> + DefinedAtom::ContentPermissions
>>>>>>>>>>> permissions,
>>>>>>>>>>> + StringRef path) {
>>>>>>>>>>> + const SectionKey sectionKey(sectionName, permissions, path);
>>>>>>>>>>> + SectionOrder sectionOrder =
>>>>>>>>>>> + getSectionOrder(sectionName, contentType, permissions);
>>>>>>>>>>> auto sec = _sectionMap.find(sectionKey);
>>>>>>>>>>> if (sec != _sectionMap.end())
>>>>>>>>>>> return sec->second;
>>>>>>>>>>> - SectionOrder sectionOrder =
>>>>>>>>>>> - getSectionOrder(sectionName, contentType, permissions);
>>>>>>>>>>> AtomSection<ELFT> *newSec =
>>>>>>>>>>> createSection(sectionName, contentType, permissions,
>>>>>>>>>>> sectionOrder);
>>>>>>>>>>> - newSec->setOutputSectionName(outputSectionName);
>>>>>>>>>>> + newSec->setOutputSectionName(getOutputSectionName(
>>>>>>>>>>> sectionName));
>>>>>>>>>>> newSec->setOrder(sectionOrder);
>>>>>>>>>>> _sections.push_back(newSec);
>>>>>>>>>>> _sectionMap.insert(std::make_pair(sectionKey, newSec));
>>>>>>>>>>> @@ -565,8 +565,8 @@ ErrorOr<const lld::AtomLayout &> Default
>>>>>>>>>>> const DefinedAtom::ContentType contentType =
>>>>>>>>>>> definedAtom->contentType();
>>>>>>>>>>>
>>>>>>>>>>> StringRef sectionName = getInputSectionName(
>>>>>>>>>>> definedAtom);
>>>>>>>>>>> - AtomSection<ELFT> *section =
>>>>>>>>>>> - getSection(sectionName, contentType, permissions);
>>>>>>>>>>> + AtomSection<ELFT> *section = getSection(
>>>>>>>>>>> + sectionName, contentType, permissions,
>>>>>>>>>>> definedAtom->file().path());
>>>>>>>>>>>
>>>>>>>>>>> // Add runtime relocations to the .rela section.
>>>>>>>>>>> for (const auto &reloc : *definedAtom) {
>>>>>>>>>>>
>>>>>>>>>>> Modified: lld/trunk/lib/ReaderWriter/
>>>>>>>>>>> ELF/Mips/MipsTargetHandler.h
>>>>>>>>>>> URL:
>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/
>>>>>>>>>>> ReaderWriter/ELF/Mips/MipsTargetHandler.h?rev=
>>>>>>>>>>> 230138&r1=230137&r2=230138&view=diff
>>>>>>>>>>>
>>>>>>>>>>> ============================================================
>>>>>>>>>>> ==================
>>>>>>>>>>> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h
>>>>>>>>>>> (original)
>>>>>>>>>>> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h Sat
>>>>>>>>>>> Feb
>>>>>>>>>>> 21
>>>>>>>>>>> 09:49:34 2015
>>>>>>>>>>> @@ -67,6 +67,17 @@ public:
>>>>>>>>>>> return *_gpDispAtom;
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> + /// \brief Return the section order for a input section
>>>>>>>>>>> + virtual Layout::SectionOrder getSectionOrder(StringRef name,
>>>>>>>>>>> + int32_t
>>>>>>>>>>> contentType,
>>>>>>>>>>> + int32_t
>>>>>>>>>>> contentPermissions) {
>>>>>>>>>>> + if ((contentType == DefinedAtom::typeStub) &&
>>>>>>>>>>> (name.startswith(".text")))
>>>>>>>>>>> + return DefaultLayout<ELFType>::ORDER_TEXT;
>>>>>>>>>>> +
>>>>>>>>>>> + return DefaultLayout<ELFType>::getSectionOrder(name,
>>>>>>>>>>> contentType,
>>>>>>>>>>> +
>>>>>>>>>>> contentPermissions);
>>>>>>>>>>> + }
>>>>>>>>>>> +
>>>>>>>>>>> private:
>>>>>>>>>>> llvm::BumpPtrAllocator _alloc;
>>>>>>>>>>> MipsGOTSection<ELFType> *_gotSection;
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> llvm-commits mailing list
>>>>>>>>>>> llvm-commits at cs.uiuc.edu
>>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>>
>>>>>>>>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora
>>>>>>>>>> Forum,
>>>>>>>>>>
>>>>>>>>> hosted
>>>>>>> by the Linux Foundation
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>>>> hosted
>>>>> by the Linux Foundation
>>>>>
>>>>>
>>>>>
>>>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
>>> by the Linux Foundation
>>>
>>>
>>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150305/8190c5d8/attachment.html>
More information about the llvm-commits
mailing list