[Lldb-commits] [PATCH] D55998: ELF: create "container" sections from PT_LOAD segments

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1841-1844
+struct SegmentAddressInfo {
+  addr_t Address;
+  addr_t Size;
+};
----------------
Use existing range code from Range.h?
```
#include "lldb/Utility/Range.h"
typedef Range<lldb::addr_t, lldb::addr_t> SegmentAddressInfo;
```



================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1846-1850
+struct SectionAddressInfo {
+  SectionSP Segment;
+  addr_t Address;
+  addr_t Size;
+};
----------------
Use SegmentAddressInfo using either of:

```
struct SectionAddressInfo : public SegmentAddressInfo {
  SectionSP Segment;
};
```
or 
```
struct SectionAddressInfo {
  SegmentAddressInfo Range;
  SectionSP Segment;
};
```


================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1954
+
+    ConstString Name(("PT_LOAD" + llvm::Twine(LoadID++)).str());
+    uint32_t Log2Align = llvm::Log2_64(std::max<elf_xword>(PHdr.p_align, 1));
----------------
Maybe add square brackets around section name? "PT_LOAD[0]" "PT_LOAD[1]"? I am fine either way, just throwing this out there


================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.h:229
 
+  lldb::user_id_t SegmentID(size_t PHdrIndex) const { return ~PHdrIndex; }
+
----------------
Make this static and move to ObjectFileElf.cpp? It doesn't need to be a method.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55998/new/

https://reviews.llvm.org/D55998