ObjectFileELF: permit thread-local sections with overlapping file addresses

[labath]

Summary

In an attempt to make file-address-based lookups more predictable, in D55998
we started ignoring sections which would result in file address
overlaps. It turns out this was too aggressive because thread-local
sections typically will have file addresses which apear to overlap
regular data/code. This does not cause a problem at runtime because
thread-local sections are loaded into memory using special logic, but it
can cause problems for lldb when trying to lookup objects by their file
address.

This patch changes ObjectFileELF to permit thread-local sections to
overlap regular ones by essentially giving them a separate address
space. It also makes them more symmetrical to regular sections by
creating container sections from PT_TLS segments.

Simultaneously, the patch changes the regular file address lookup logic
to ignore sections with the thread-specific bit set. I believe this is
what the users looking up file addresses would typically expect, as
looking up thread-local data generally requires more complex logic (e.g.
DWARF has a special opcode for that).

[clayborg]

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1816–1817

Maybe ask segment provider to get the next segment name?

ConstString Name(provider.GetNextSegmentName());

And have the llvm::formatv call be in a the VMAddressProvider::GetNextSegmentName()?

[MaskRay]

It turns out this was too aggressive because thread-local
sections typically will have file addresses which apear to overlap
regular data/code. This does not cause a problem at runtime because
thread-local sections are loaded into memory using special logic, but it
can cause problems for lldb when trying to lookup objects by their file
address.

Yes :) This can happen with .tbss (SHT_NOBITS) overlapping another section (usually .init_array, but .got and .data are also possible). SHT_NOBITS sections are not allocated bytes in the file so they may overlap with a subsequent section.

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

69

.tbss 0x1000 NOBITS

.tdata 0x1010 PROGBITS

Move .tdata before .tbss (0xff0) to make the example more realistic?

.tdata has a larger address than .tbss. I think this is impossible in ld.bfd, but you can make .tbss go before .tdata with a broken lld linker script.

[MaskRay]

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

45

Do you mind explaining more how you'd like to improve file-address-based lookups for PT_TLS?

(lldb) image lookup -a 0x1010

Address: a.o[0x00001010] (a.o.PT_LOAD[0]..tdata + 0)

This is the current output before the change.

Yes PT_TLS can be seen as a separate address space. At runtime a TLS block is allocated (mmap) for each thread and they access TLS through their thread pointer. The address will be very different from the PT_LOAD address.

[labath]

Do you mind explaining more how you'd like to improve file-address-based lookups for PT_TLS?

I don't have this fully thought through (I was hoping this would develop as use cases start showing up), but...

Are you referring to the "image lookup" command specifically, or is it a more general question about the internals of lldb too?

Regarding "image lookup", the simplest way would be to add a "--tls" flag to look in the "tls" address space. Or even a more generic "address space" flag, as there are people interested in more address spaces. Or, we could just change the command to find and display multiple matches. But then the test here would need to be changed, as my main interest is that the correct address is found when evaluating DW_OP_addr and friends -- the "image lookup" thing is just a proxy.

As for internal interfaces, I guess similar options would be possible, but there I'm even more fuzzy about which ones are better because I don't know what are the ways in which this may be used. I know that the DW_OP_form_tls_address lookup currently completely ignores the "file" addresses of the sections and just straight to "load" addresses and real memory. This is not completely surprising as you need a thread to see thread local data, and if you have a thread, you have a live process to query. However, I can see how it might be interesting to be able to see the initial value of a thread local variable much like we can display the initial value of a global variable without launching a process. For this case, a flag to Section::ContainsFileAddress saying "yes, I want to look up in thread-local sections now" would suffice, but I don't know if this is the only use case...

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

69

I'll change the order here. The thing I was trying to test here is that addresses in .data are found regardless of whether it comes before or after a tls section. I think already having two TLS segments is somewhat unrealistic, and I could make it more real by splitting this into two tests, but it did not seem necessary, as lldb does not care about details like this.

[labath]

• update according to review comments

[MaskRay]

Are you referring to the "image lookup" command specifically, or is it a more general question about the internals of lldb too?

Both:) This patch doesn't change the Address: a.o[0x00001010] (a.o.PT_LOAD[0]..tdata + 0) output so I was puzzled what this patch intends to do.

However, I can see how it might be interesting to be able to see the initial value of a thread local variable much like we can display the initial value of a global variable without launching a process. For this case, a flag to Section::ContainsFileAddress saying "yes, I want to look up in thread-local sections now" would suffice, but I don't know if this is the only use case...

Yes, inspecting the initial value of a thread-local variable is a use case. To that end, can this be done by introducing another member variable instead of overloading m_sections_up with a new purpose (adding PT_TLS)? If PT_TLS is recorded in a different variable, the change below can be deleted.

 bool Section::ContainsFileAddress(addr_t vm_addr) const {
   const addr_t file_addr = GetFileAddress();
-  if (file_addr != LLDB_INVALID_ADDRESS) {
+  if (file_addr != LLDB_INVALID_ADDRESS && !IsThreadSpecific()) {

(An adjacent pair of PT_LOAD segments can load the same file contents, e.g. PT_LOAD [0x150, 0x1234) and [0x1234, 0x1800) will transform to mmap calls with ranges [0, 0x2000) and [0x1000, 0x2000) at runtime if the runtime page size = 0x1000. They share one page in the file. If you ask what a specific offset in the file is mapped to, there can be multiple PT_LOAD segments (physical -> VMA is not unique). Fortunately the reverse mapping VMA -> physical offset can be treated as unique in practice ([p_vaddr,p_vaddr+p_memsz) ranges do not overlap).)

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

69

Multiple PT_TLS is unrealistic. None of glibc/musl/FreeBSD rtld supports more than 1 PT_TLS (they will just pick the last one and ignore the others).

[MaskRay]

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

63

.data = .tbss = 0x1010 is a more realistic scenario.

Normally, a SHT_PROGBITS section may overlap with a SHT_NOBITS section, but two SHT_PROGBITS sections do not overlap (ld has an on-by-default check ld --check-sections). Linkers allocate file bytes for SHT_PROGBITS sections so their occupied bytes cannot be reused by other sections (without fixing addresses with a linker script).

[labath]

In D65282#1602244, @MaskRay wrote:

Are you referring to the "image lookup" command specifically, or is it a more general question about the internals of lldb too?

Both:) This patch doesn't change the Address: a.o[0x00001010] (a.o.PT_LOAD[0]..tdata + 0) output so I was puzzled what this patch intends to do.

What do you mean by "doesn't change"? After this patch the addresses always resolve to the .data section..

However, I can see how it might be interesting to be able to see the initial value of a thread local variable much like we can display the initial value of a global variable without launching a process. For this case, a flag to Section::ContainsFileAddress saying "yes, I want to look up in thread-local sections now" would suffice, but I don't know if this is the only use case...

Yes, inspecting the initial value of a thread-local variable is a use case. To that end, can this be done by introducing another member variable instead of overloading m_sections_up with a new purpose (adding PT_TLS)? If PT_TLS is recorded in a different variable, the change below can be deleted.

I think that would be pretty significant departure from the current design of lldb. Lldb expects that the "section list" of a module will contain all of the module's sections (and before I started messing with these functions, it did). This includes non-loadable sections like .debug_info et al. While one could concieve a world where tls sections are in a special "tls" section list, I am not sure this is actually useful -- if we're going to think of the tls addresses as address spaces, then its reasonable to have more than two address spaces one day (there are people interested in that), and so we couldn't have a fixed set of section lists.

 bool Section::ContainsFileAddress(addr_t vm_addr) const {
   const addr_t file_addr = GetFileAddress();
-  if (file_addr != LLDB_INVALID_ADDRESS) {
+  if (file_addr != LLDB_INVALID_ADDRESS && !IsThreadSpecific()) {

(An adjacent pair of PT_LOAD segments can load the same file contents, e.g. PT_LOAD [0x150, 0x1234) and [0x1234, 0x1800) will transform to mmap calls with ranges [0, 0x2000) and [0x1000, 0x2000) at runtime if the runtime page size = 0x1000. They share one page in the file. If you ask what a specific offset in the file is mapped to, there can be multiple PT_LOAD segments (physical -> VMA is not unique). Fortunately the reverse mapping VMA -> physical offset can be treated as unique in practice ([p_vaddr,p_vaddr+p_memsz) ranges do not overlap).)

I am not 100% what you mean by this, but I think there's some confusion about names of things here. In lldb terms, a "file address" is the "load address, as it is written in the file. It is not the "physical offset within the file", which lldb calls "file offset". Unfortunately, this terminology has caused a lot of confusion in the past, but I don't know what would be the best way to resolve this. How does lld call these things? I guess there's less confusion there as lld does not have to care about real, memory, load addresses...

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

63

Interesting. I can that easily, but I'm wondering, do you know the reason for that? Is it just how it falls out of the default linker processing of things, or would something actually break if I assigned identical addresses to two SHT_PROGBITS sections?

69

Ok, so let's go for two tests then.

[labath]

Split the test into two

[MaskRay]

In D65282#1602293, @labath wrote:

In D65282#1602244, @MaskRay wrote:

Are you referring to the "image lookup" command specifically, or is it a more general question about the internals of lldb too?

Both:) This patch doesn't change the Address: a.o[0x00001010] (a.o.PT_LOAD[0]..tdata + 0) output so I was puzzled what this patch intends to do.

What do you mean by "doesn't change"? After this patch the addresses always resolve to the .data section..

However, I can see how it might be interesting to be able to see the initial value of a thread local variable much like we can display the initial value of a global variable without launching a process. For this case, a flag to Section::ContainsFileAddress saying "yes, I want to look up in thread-local sections now" would suffice, but I don't know if this is the only use case...

Yes, inspecting the initial value of a thread-local variable is a use case. To that end, can this be done by introducing another member variable instead of overloading m_sections_up with a new purpose (adding PT_TLS)? If PT_TLS is recorded in a different variable, the change below can be deleted.

I think that would be pretty significant departure from the current design of lldb. Lldb expects that the "section list" of a module will contain all of the module's sections (and before I started messing with these functions, it did). This includes non-loadable sections like .debug_info et al. While one could concieve a world where tls sections are in a special "tls" section list, I am not sure this is actually useful -- if we're going to think of the tls addresses as address spaces, then its reasonable to have more than two address spaces one day (there are people interested in that), and so we couldn't have a fixed set of section lists.

I see. If that would be a significant departure, the current approach should be the choice. I didn't non-SHF_ALLOC sections are also in the list. If .debug_info et all are in the list, I don't see any problem to have PT_TLS in the list since those PT_LOAD are already in the list.

 bool Section::ContainsFileAddress(addr_t vm_addr) const {
   const addr_t file_addr = GetFileAddress();
-  if (file_addr != LLDB_INVALID_ADDRESS) {
+  if (file_addr != LLDB_INVALID_ADDRESS && !IsThreadSpecific()) {

(An adjacent pair of PT_LOAD segments can load the same file contents, e.g. PT_LOAD [0x150, 0x1234) and [0x1234, 0x1800) will transform to mmap calls with ranges [0, 0x2000) and [0x1000, 0x2000) at runtime if the runtime page size = 0x1000. They share one page in the file. If you ask what a specific offset in the file is mapped to, there can be multiple PT_LOAD segments (physical -> VMA is not unique). Fortunately the reverse mapping VMA -> physical offset can be treated as unique in practice ([p_vaddr,p_vaddr+p_memsz) ranges do not overlap).)

I am not 100% what you mean by this, but I think there's some confusion about names of things here. In lldb terms, a "file address" is the "load address, as it is written in the file. It is not the "physical offset within the file", which lldb calls "file offset". Unfortunately, this terminology has caused a lot of confusion in the past, but I don't know what would be the best way to resolve this. How does lld call these things? I guess there's less confusion there as lld does not have to care about real, memory, load addresses...

Maybe we can refer to these things with ELF terminology: p_offset (offsets in the file)/p_vaddr (VMA)/p_paddr (LMA)...

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

63

https://github.com/llvm-mirror/lld/blob/master/ELF/Writer.cpp#L2412

SHT_PROGBITS sections occupy space in the file but SHT_NOBITS sections don't. The linker doesn't allocate the same byte for different sections, unless you fix the VMA/LMA with a linker script. So usually SHT_PROGBITS sections cannot overlap.

[labath]

Thanks for sharing your knowledge about linkers and dynamic loaders. I have found it very useful.

[JDevlieghere]

I'm seeing some really weird behavior for the following two tests and I'm honestly kind of puzzled.

ObjectFile/ELF/PT_LOAD-overlap-PT_TLS.yaml
ObjectFile/ELF/PT_TLS-overlap-PT_LOAD.yaml

They fail in the same way for a standalone build, both on macOS (http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/1009/) and Linux (https://ci.swift.org/view/swift-master-rebranch/job/oss-lldb-master-rebranch-incremental-linux-ubuntu-18_04/8/).

For a standalone build the image lookups returns:

(lldb) image lookup -a 0x1000
      Address: PT_LOAD-overlap-PT_TLS.yaml.tmp[0x00001000] (PT_LOAD-overlap-PT_TLS.yaml.tmp..tbss + 0)

While for a regular in-tree build the image lookup returns:

(lldb) image lookup -a 0x1000
      Address: PT_LOAD-overlap-PT_TLS.yaml.tmp[0x00001000] (PT_LOAD-overlap-PT_TLS.yaml.tmp.PT_LOAD[0]..data + 0)

I scp'd the binaries between my local machine and the bot and they don't affect the outcome, it's just lldb that's different... Pavel, can you think of *anything* that might cause this?

[labath]

It took me a while, but I tracked this down to the lack of % in front of lldb in the RUN: commands. We don't have an "lldb" substitution, so this ends up running whatever it finds on the path. Normally this does not matter because we add the build dir to the lit path, but for some reason this is not happening in a standalone build (wild guess: probably we just add the "llvm" build dir).

lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml

+# Overlapping PT_LOAD and PT_TLS segments should be able to exist side by side.
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# RUN: lldb %t -o "image lookup -a 0x1000" -b | FileCheck --check-prefix=LOOKUP %s
+
+# CHECK:        Index: 0
+# CHECK-NEXT:   ID: 0xffffffffffffffff
+# CHECK-NEXT:   Name: PT_TLS[0]
+# CHECK-NEXT:   Type: container
+# CHECK-NEXT:   Permissions: rw-
+# CHECK-NEXT:   Thread specific: yes
+# CHECK-NEXT:   VM address: 0x1000
+# CHECK-NEXT:   VM size: 16
+# CHECK-NEXT:   File size: 0
+# CHECK-NEXT:   Showing 1 subsections
+
+# CHECK:        Index: 1
+# CHECK-NEXT:   ID: 0xfffffffffffffffe
+# CHECK-NEXT:   Name: PT_LOAD[0]
+# CHECK-NEXT:   Type: container
+# CHECK-NEXT:   Permissions: rw-
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x1000
+# CHECK-NEXT:   VM size: 16
+# CHECK-NEXT:   File size: 16
+# CHECK-NEXT:   Showing 1 subsections
+
+# LOOKUP-LABEL: image lookup -a 0x1000
+# LOOKUP:       Address: {{.*}}.PT_LOAD[0]..data + 0)
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_ARM
+Sections:
+  - Name:            .tbss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_ALLOC, SHF_WRITE, SHF_TLS ]
+    Address:         0x1000
+    AddressAlign:    0x4
+    Size:            0x10
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_WRITE ]
+    Address:         0x1000
+    AddressAlign:    0x4
+    Size:            0x10
+ProgramHeaders:
+  - Type: PT_TLS
+    Flags: [ PF_R, PF_W ]
+    VAddr: 0x1000
+    Align: 0x4
+    Sections:
+      - Section: .tbss
+  - Type: PT_LOAD
+    Flags: [ PF_W, PF_R ]
+    VAddr: 0x1000
+    Align: 0x4
+    Sections:
+      - Section: .data

lit/Modules/ELF/PT_TLS-overlap-PT_LOAD.yaml

+# Overlapping PT_LOAD and PT_TLS segments should be able to exist side by side.
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# RUN: lldb %t -o "image lookup -a 0x1000" -b | FileCheck --check-prefix=LOOKUP %s
+
+# CHECK:        Index: 0
+# CHECK-NEXT:   ID: 0xffffffffffffffff
+# CHECK-NEXT:   Name: PT_LOAD[0]
+# CHECK-NEXT:   Type: container
+# CHECK-NEXT:   Permissions: rw-
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x1000
+# CHECK-NEXT:   VM size: 16
+# CHECK-NEXT:   File size: 16
+# CHECK-NEXT:   Showing 1 subsections
+
+# CHECK:        Index: 1
+# CHECK-NEXT:   ID: 0xfffffffffffffffe
+# CHECK-NEXT:   Name: PT_TLS[0]
+# CHECK-NEXT:   Type: container
+# CHECK-NEXT:   Permissions: rw-
+# CHECK-NEXT:   Thread specific: yes
+# CHECK-NEXT:   VM address: 0x1000
+# CHECK-NEXT:   VM size: 16
+# CHECK-NEXT:   File size: 0
+# CHECK-NEXT:   Showing 1 subsections
+
+# LOOKUP-LABEL: image lookup -a 0x1000
+# LOOKUP:       Address: {{.*}}.PT_LOAD[0]..data + 0)
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_ARM
+Sections:
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_WRITE ]
+    Address:         0x1000
+    AddressAlign:    0x4
+    Size:            0x10
+  - Name:            .tbss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_ALLOC, SHF_WRITE, SHF_TLS ]
+    Address:         0x1000
+    AddressAlign:    0x4
+    Size:            0x10
+ProgramHeaders:
+  - Type: PT_LOAD
+    Flags: [ PF_W, PF_R ]
+    VAddr: 0x1000
+    Align: 0x4
+    Sections:
+      - Section: .data
+  - Type: PT_TLS
+    Flags: [ PF_R, PF_W ]
+    VAddr: 0x1000
+    Align: 0x4
+    Sections:
+      - Section: .tbss

source/Core/Section.cpp

#ifdef LLDB_CONFIGURATION_DEBUG
   // sections.
   assert(GetModule().get());
 #endif
   return true;
 }
 
 bool Section::ContainsFileAddress(addr_t vm_addr) const {
   const addr_t file_addr = GetFileAddress();
-  if (file_addr != LLDB_INVALID_ADDRESS) {
+  if (file_addr != LLDB_INVALID_ADDRESS && !IsThreadSpecific()) {
     if (file_addr <= vm_addr) {
       const addr_t offset = (vm_addr - file_addr) * m_target_byte_size;
       return offset < GetByteSize();
     }
   }
   return false;
 }
 

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

using VMMap = llvm::IntervalMap<addr_t, SectionSP, 4,
                                        llvm::IntervalMapHalfOpenInfo<addr_t>>;
 
   ObjectFile::Type ObjectType;
   addr_t NextVMAddress = 0;
   VMMap::Allocator Alloc;
   VMMap Segments = VMMap(Alloc);
   VMMap Sections = VMMap(Alloc);
   lldb_private::Log *Log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
+  size_t SegmentCount = 0;
+  std::string SegmentName;
 
   VMRange GetVMRange(const ELFSectionHeader &H) {
     addr_t Address = H.sh_addr;
     addr_t Size = H.sh_flags & SHF_ALLOC ? H.sh_size : 0;
     if (ObjectType == ObjectFile::Type::eTypeObjectFile && Segments.empty() && (H.sh_flags & SHF_ALLOC)) {
       NextVMAddress =
           llvm::alignTo(NextVMAddress, std::max<addr_t>(H.sh_addralign, 1));
       Address = NextVMAddress;
       NextVMAddress += Size;
     }
     return VMRange(Address, Size);
   }
 
 public:
-  VMAddressProvider(ObjectFile::Type Type) : ObjectType(Type) {}
+  VMAddressProvider(ObjectFile::Type Type, llvm::StringRef SegmentName)
+      : ObjectType(Type), SegmentName(SegmentName) {}
+
+  std::string GetNextSegmentName() const {
+    return llvm::formatv("{0}[{1}]", SegmentName, SegmentCount).str();
+  }
 
   llvm::Optional<VMRange> GetAddressInfo(const ELFProgramHeader &H) {
     if (H.p_memsz == 0) {
-      LLDB_LOG(Log,
-               "Ignoring zero-sized PT_LOAD segment. Corrupt object file?");
+      LLDB_LOG(Log, "Ignoring zero-sized {0} segment. Corrupt object file?",
+               SegmentName);
       return llvm::None;
     }
 
     if (Segments.overlaps(H.p_vaddr, H.p_vaddr + H.p_memsz)) {
-      LLDB_LOG(Log,
-               "Ignoring overlapping PT_LOAD segment. Corrupt object file?");
+      LLDB_LOG(Log, "Ignoring overlapping {0} segment. Corrupt object file?",
+               SegmentName);
       return llvm::None;
     }
     return VMRange(H.p_vaddr, H.p_memsz);
   }
 
   llvm::Optional<SectionAddressInfo> GetAddressInfo(const ELFSectionHeader &H) {
     VMRange Range = GetVMRange(H);
     SectionSP Segment;
llvm::Optional<SectionAddressInfo> GetAddressInfo(const ELFSectionHeader &H) {
     }
     if (Segment)
       Range.Slide(-Segment->GetFileAddress());
     return SectionAddressInfo{Segment, Range};
   }
 
   void AddSegment(const VMRange &Range, SectionSP Seg) {
     Segments.insert(Range.GetRangeBase(), Range.GetRangeEnd(), std::move(Seg));
+    ++SegmentCount;
   }
 
   void AddSection(SectionAddressInfo Info, SectionSP Sect) {
     if (Info.Range.GetByteSize() == 0)
       return;
     if (Info.Segment)
       Info.Range.Slide(Info.Segment->GetFileAddress());
     Sections.insert(Info.Range.GetRangeBase(), Info.Range.GetRangeEnd(),
                     std::move(Sect));
   }
 };
 }
 
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
   if (m_sections_up)
     return;
 
   m_sections_up = llvm::make_unique<SectionList>();
-  VMAddressProvider address_provider(GetType());
+  VMAddressProvider regular_provider(GetType(), "PT_LOAD");
+  VMAddressProvider tls_provider(GetType(), "PT_TLS");
 
-  size_t LoadID = 0;
   for (const auto &EnumPHdr : llvm::enumerate(ProgramHeaders())) {
     const ELFProgramHeader &PHdr = EnumPHdr.value();
-    if (PHdr.p_type != PT_LOAD)
+    if (PHdr.p_type != PT_LOAD && PHdr.p_type != PT_TLS)
       continue;
 
-    auto InfoOr = address_provider.GetAddressInfo(PHdr);
+    VMAddressProvider &provider =
+        PHdr.p_type == PT_TLS ? tls_provider : regular_provider;
+    auto InfoOr = provider.GetAddressInfo(PHdr);
     if (!InfoOr)
       continue;
 
-    ConstString Name(("PT_LOAD[" + llvm::Twine(LoadID++) + "]").str());
     uint32_t Log2Align = llvm::Log2_64(std::max<elf_xword>(PHdr.p_align, 1));
     SectionSP Segment = std::make_shared<Section>(
-        GetModule(), this, SegmentID(EnumPHdr.index()), Name,
-        eSectionTypeContainer, InfoOr->GetRangeBase(), InfoOr->GetByteSize(),
-        PHdr.p_offset, PHdr.p_filesz, Log2Align, /*flags*/ 0);
+        GetModule(), this, SegmentID(EnumPHdr.index()),
+        ConstString(provider.GetNextSegmentName()), eSectionTypeContainer,
+        InfoOr->GetRangeBase(), InfoOr->GetByteSize(), PHdr.p_offset,
+        PHdr.p_filesz, Log2Align, /*flags*/ 0);
     Segment->SetPermissions(GetPermissions(PHdr));
+    Segment->SetIsThreadSpecific(PHdr.p_type == PT_TLS);
     m_sections_up->AddSection(Segment);
 
-    address_provider.AddSegment(*InfoOr, std::move(Segment));
+    provider.AddSegment(*InfoOr, std::move(Segment));
   }
 
   ParseSectionHeaders();
   if (m_section_headers.empty())
     return;
 
   for (SectionHeaderCollIter I = std::next(m_section_headers.begin());
        I != m_section_headers.end(); ++I) {
     const ELFSectionHeaderInfo &header = *I;
 
     ConstString &name = I->section_name;
     const uint64_t file_size =
         header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
 
-    auto InfoOr = address_provider.GetAddressInfo(header);
+    VMAddressProvider &provider =
+        header.sh_flags & SHF_TLS ? tls_provider : regular_provider;
+    auto InfoOr = provider.GetAddressInfo(header);
     if (!InfoOr)
       continue;
 
     SectionType sect_type = GetSectionType(header);
 
     const uint32_t target_bytes_size =
         GetTargetByteSize(sect_type, m_arch_spec);
 
SectionSP section_sp(new Section(
         log2align,           // Alignment of the section
         header.sh_flags,     // Flags for this section.
         target_bytes_size)); // Number of host bytes per target byte
 
     section_sp->SetPermissions(GetPermissions(header));
     section_sp->SetIsThreadSpecific(header.sh_flags & SHF_TLS);
     (InfoOr->Segment ? InfoOr->Segment->GetChildren() : *m_sections_up)
         .AddSection(section_sp);
-    address_provider.AddSection(std::move(*InfoOr), std::move(section_sp));
+    provider.AddSection(std::move(*InfoOr), std::move(section_sp));
   }
 
   // For eTypeDebugInfo files, the Symbol Vendor will take care of updating the
   // unified section list.
   if (GetType() != eTypeDebugInfo)
     unified_section_list = *m_sections_up;
 }