[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm sorry Omair, my comment was meant for Jorge, not you. `@skipIfXml` is not 
the right decorator here, although it will probably kinda fix the failure 
(because the bot does not have xml support either). I *think* the compressed 
section support is controlled by presence of zlib.

Since the bot is not immediately broken, I think we can leave bde5d31 
 in to 
avoid the churn. Jorge, could you replace the xml thingy with a more 
appropriate decorator?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142672

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbde5d31e96f5: Revert Revert [lldb] Make 
SBSection::GetSectionData call Section… (authored by omjavaid).

Changed prior to commit:
  https://reviews.llvm.org/D142672?vs=492839=493267#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142672

Files:
  lldb/source/API/SBSection.cpp
  lldb/test/API/python_api/section/TestSectionAPI.py
  lldb/test/API/python_api/section/compressed-sections.yaml

Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -11,6 +11,8 @@
 
 class SectionAPITestCase(TestBase):
 
+@no_debug_info_test
+@skipIfXmlSupportMissing
 def test_get_target_byte_size(self):
 d = {'EXE': 'b.out'}
 self.build(dictionary=d)
@@ -38,6 +40,8 @@
 self.assertIsNotNone(data_section)
 self.assertEqual(data_section.target_byte_size, 1)
 
+@no_debug_info_test
+@skipIfXmlSupportMissing
 def test_get_alignment(self):
 exe = self.getBuildArtifact("aligned.out")
 self.yaml2obj("aligned.yaml", exe)
@@ -48,3 +52,20 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+@no_debug_info_test
+@skipIfXmlSupportMissing
+def test_compressed_section_data(self):
+exe = self.getBuildArtifact("compressed-sections.out")
+self.yaml2obj("compressed-sections.yaml", exe)
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# exe contains a single section with SHF_COMPRESSED. Check that
+# GetSectionData returns the uncompressed data and not the raw contents
+# of the section.
+section = target.modules[0].sections[0]
+section_data = section.GetSectionData().uint8s
+self.assertEqual(section_data,
+ [0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80, 0x90])
+
Index: lldb/source/API/SBSection.cpp
===
--- lldb/source/API/SBSection.cpp
+++ lldb/source/API/SBSection.cpp
@@ -182,35 +182,10 @@
   SBData sb_data;
   SectionSP section_sp(GetSP());
   if (section_sp) {
-const uint64_t sect_file_size = section_sp->GetFileSize();
-if (sect_file_size > 0) {
-  ModuleSP module_sp(section_sp->GetModule());
-  if (module_sp) {
-ObjectFile *objfile = module_sp->GetObjectFile();
-if (objfile) {
-  const uint64_t sect_file_offset =
-  objfile->GetFileOffset() + section_sp->GetFileOffset();
-  const uint64_t file_offset = sect_file_offset + offset;
-  uint64_t file_size = size;
-  if (file_size == UINT64_MAX) {
-file_size = section_sp->GetByteSize();
-if (file_size > offset)
-  file_size -= offset;
-else
-  file_size = 0;
-  }
-  auto data_buffer_sp = FileSystem::Instance().CreateDataBuffer(
-  objfile->GetFileSpec().GetPath(), file_size, file_offset);
-  if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0) {
-DataExtractorSP data_extractor_sp(
-new DataExtractor(data_buffer_sp, objfile->GetByteOrder(),
-  objfile->GetAddressByteSize()));
-
-sb_data.SetOpaque(data_extractor_sp);
-  }
-}
-  }
-}
+DataExtractor section_data;
+section_sp->GetSectionData(section_data);
+sb_data.SetOpaque(
+std::make_shared(section_data, offset, size));
   }
   return sb_data;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm pretty sure that's because that bot builds without zlib support. You'll 
need to add something like `@skipIfXmlSupportMissing` to this test (I think we 
don't have a decorator for zlib now, so you'll probably need to add one).

While you're at it, you might as well slap `@no_debug_info_test` on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142672

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid reopened this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

LLDB windows buildbots were broken by the TestSectionAPI.py test. I dont
have full context of the commit to fix it. Reverting it temporarily.

https://lab.llvm.org/buildbot/#/builders/83/builds/28617
https://lab.llvm.org/buildbot/#/builders/219/builds/180


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142672

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
jgorbe marked an inline comment as done.
Closed by commit rG805600c7d573: [lldb] Make SBSection::GetSectionData call 
Section::GetSectionData. (authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142672

Files:
  lldb/source/API/SBSection.cpp
  lldb/test/API/python_api/section/TestSectionAPI.py
  lldb/test/API/python_api/section/compressed-sections.yaml


Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -48,3 +48,18 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+def test_compressed_section_data(self):
+exe = self.getBuildArtifact("compressed-sections.out")
+self.yaml2obj("compressed-sections.yaml", exe)
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# exe contains a single section with SHF_COMPRESSED. Check that
+# GetSectionData returns the uncompressed data and not the raw contents
+# of the section.
+section = target.modules[0].sections[0]
+section_data = section.GetSectionData().uint8s
+self.assertEqual(section_data,
+ [0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80, 0x90])
+
Index: lldb/source/API/SBSection.cpp
===
--- lldb/source/API/SBSection.cpp
+++ lldb/source/API/SBSection.cpp
@@ -182,35 +182,10 @@
   SBData sb_data;
   SectionSP section_sp(GetSP());
   if (section_sp) {
-const uint64_t sect_file_size = section_sp->GetFileSize();
-if (sect_file_size > 0) {
-  ModuleSP module_sp(section_sp->GetModule());
-  if (module_sp) {
-ObjectFile *objfile = module_sp->GetObjectFile();
-if (objfile) {
-  const uint64_t sect_file_offset =
-  objfile->GetFileOffset() + section_sp->GetFileOffset();
-  const uint64_t file_offset = sect_file_offset + offset;
-  uint64_t file_size = size;
-  if (file_size == UINT64_MAX) {
-file_size = section_sp->GetByteSize();
-if (file_size > offset)
-  file_size -= offset;
-else
-  file_size = 0;
-  }
-  auto data_buffer_sp = FileSystem::Instance().CreateDataBuffer(
-  objfile->GetFileSpec().GetPath(), file_size, file_offset);
-  if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0) {
-DataExtractorSP data_extractor_sp(
-new DataExtractor(data_buffer_sp, objfile->GetByteOrder(),
-  objfile->GetAddressByteSize()));
-
-sb_data.SetOpaque(data_extractor_sp);
-  }
-}
-  }
-}
+DataExtractor section_data;
+section_sp->GetSectionData(section_data);
+sb_data.SetOpaque(
+std::make_shared(section_data, offset, size));
   }
   return sb_data;
 }


Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -48,3 +48,18 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+def test_compressed_section_data(self):
+exe = self.getBuildArtifact("compressed-sections.out")
+self.yaml2obj("compressed-sections.yaml", exe)
+  

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe marked an inline comment as done.
jgorbe added inline comments.



Comment at: lldb/source/API/SBSection.cpp:187-189
+DataExtractorSP result_data_sp =
+std::make_shared(section_data, offset, size);
+sb_data.SetOpaque(result_data_sp);

dblaikie wrote:
> Probably either use `std::move` when passing `result_data_sp` to `SetOpaque`, 
> or roll the expressions together (to avoid an unnecessary copy of a ref 
> counted smart pointer, that would cause extra increment/decrement of the ref 
> count)
Done, inlined `result_data_sp` into the `SetOpaque` call. Thanks!


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

https://reviews.llvm.org/D142672

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe updated this revision to Diff 492602.
jgorbe added a comment.

Addressing review comment.


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

https://reviews.llvm.org/D142672

Files:
  lldb/source/API/SBSection.cpp
  lldb/test/API/python_api/section/TestSectionAPI.py
  lldb/test/API/python_api/section/compressed-sections.yaml


Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -48,3 +48,18 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+def test_compressed_section_data(self):
+exe = self.getBuildArtifact("compressed-sections.out")
+self.yaml2obj("compressed-sections.yaml", exe)
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# exe contains a single section with SHF_COMPRESSED. Check that
+# GetSectionData returns the uncompressed data and not the raw contents
+# of the section.
+section = target.modules[0].sections[0]
+section_data = section.GetSectionData().uint8s
+self.assertEqual(section_data,
+ [0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80, 0x90])
+
Index: lldb/source/API/SBSection.cpp
===
--- lldb/source/API/SBSection.cpp
+++ lldb/source/API/SBSection.cpp
@@ -182,35 +182,10 @@
   SBData sb_data;
   SectionSP section_sp(GetSP());
   if (section_sp) {
-const uint64_t sect_file_size = section_sp->GetFileSize();
-if (sect_file_size > 0) {
-  ModuleSP module_sp(section_sp->GetModule());
-  if (module_sp) {
-ObjectFile *objfile = module_sp->GetObjectFile();
-if (objfile) {
-  const uint64_t sect_file_offset =
-  objfile->GetFileOffset() + section_sp->GetFileOffset();
-  const uint64_t file_offset = sect_file_offset + offset;
-  uint64_t file_size = size;
-  if (file_size == UINT64_MAX) {
-file_size = section_sp->GetByteSize();
-if (file_size > offset)
-  file_size -= offset;
-else
-  file_size = 0;
-  }
-  auto data_buffer_sp = FileSystem::Instance().CreateDataBuffer(
-  objfile->GetFileSpec().GetPath(), file_size, file_offset);
-  if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0) {
-DataExtractorSP data_extractor_sp(
-new DataExtractor(data_buffer_sp, objfile->GetByteOrder(),
-  objfile->GetAddressByteSize()));
-
-sb_data.SetOpaque(data_extractor_sp);
-  }
-}
-  }
-}
+DataExtractor section_data;
+section_sp->GetSectionData(section_data);
+sb_data.SetOpaque(
+std::make_shared(section_data, offset, size));
   }
   return sb_data;
 }


Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -48,3 +48,18 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+def test_compressed_section_data(self):
+exe = self.getBuildArtifact("compressed-sections.out")
+self.yaml2obj("compressed-sections.yaml", exe)
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# exe contains a single section with SHF_COMPRESSED. Check that
+# 

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/source/API/SBSection.cpp:187-189
+DataExtractorSP result_data_sp =
+std::make_shared(section_data, offset, size);
+sb_data.SetOpaque(result_data_sp);

Probably either use `std::move` when passing `result_data_sp` to `SetOpaque`, 
or roll the expressions together (to avoid an unnecessary copy of a ref counted 
smart pointer, that would cause extra increment/decrement of the ref count)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142672

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142672: [lldb] Make SBSection::GetSectionData call Section::GetSectionData.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added a reviewer: labath.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

`SBSection::GetSectionData` and `Section::GetSectionData` are
implemented differently, and the `SBSection` method doesn't handle
compressed sections correctly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142672

Files:
  lldb/source/API/SBSection.cpp
  lldb/test/API/python_api/section/TestSectionAPI.py
  lldb/test/API/python_api/section/compressed-sections.yaml


Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -48,3 +48,18 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+def test_compressed_section_data(self):
+exe = self.getBuildArtifact("compressed-sections.out")
+self.yaml2obj("compressed-sections.yaml", exe)
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# exe contains a single section with SHF_COMPRESSED. Check that
+# GetSectionData returns the uncompressed data and not the raw contents
+# of the section.
+section = target.modules[0].sections[0]
+section_data = section.GetSectionData().uint8s
+self.assertEqual(section_data,
+ [0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80, 0x90])
+
Index: lldb/source/API/SBSection.cpp
===
--- lldb/source/API/SBSection.cpp
+++ lldb/source/API/SBSection.cpp
@@ -182,35 +182,11 @@
   SBData sb_data;
   SectionSP section_sp(GetSP());
   if (section_sp) {
-const uint64_t sect_file_size = section_sp->GetFileSize();
-if (sect_file_size > 0) {
-  ModuleSP module_sp(section_sp->GetModule());
-  if (module_sp) {
-ObjectFile *objfile = module_sp->GetObjectFile();
-if (objfile) {
-  const uint64_t sect_file_offset =
-  objfile->GetFileOffset() + section_sp->GetFileOffset();
-  const uint64_t file_offset = sect_file_offset + offset;
-  uint64_t file_size = size;
-  if (file_size == UINT64_MAX) {
-file_size = section_sp->GetByteSize();
-if (file_size > offset)
-  file_size -= offset;
-else
-  file_size = 0;
-  }
-  auto data_buffer_sp = FileSystem::Instance().CreateDataBuffer(
-  objfile->GetFileSpec().GetPath(), file_size, file_offset);
-  if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0) {
-DataExtractorSP data_extractor_sp(
-new DataExtractor(data_buffer_sp, objfile->GetByteOrder(),
-  objfile->GetAddressByteSize()));
-
-sb_data.SetOpaque(data_extractor_sp);
-  }
-}
-  }
-}
+DataExtractor section_data;
+section_sp->GetSectionData(section_data);
+DataExtractorSP result_data_sp =
+std::make_shared(section_data, offset, size);
+sb_data.SetOpaque(result_data_sp);
   }
   return sb_data;
 }


Index: lldb/test/API/python_api/section/compressed-sections.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/section/compressed-sections.yaml
@@ -0,0 +1,11 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.compressed
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+Content: 010008000100789c533070084828689809c802c1
Index: lldb/test/API/python_api/section/TestSectionAPI.py
===
--- lldb/test/API/python_api/section/TestSectionAPI.py
+++ lldb/test/API/python_api/section/TestSectionAPI.py
@@ -48,3 +48,18 @@
 section = target.modules[0].sections[0]
 self.assertEqual(section.GetAlignment(), 0x1000)
 self.assertEqual(section.alignment, 0x1000)
+
+def test_compressed_section_data(self):
+exe