[lldb][ObjectFileELF] Set ModuleSpec file offset and size

[splhack]

Summary

In Android API level 23 and above, dynamic loader is able to load .so file
directly from APK.
https://android.googlesource.com/platform/bionic/+/master/
android-changes-for-ndk-developers.md#
opening-shared-libraries-directly-from-an-apk

ObjectFileELF::GetModuleSpecifications will load a .so file, which is page
aligned and uncompressed, directly from a zip file. However it does not
set the .so file offset and size to the ModuleSpec. Also crc32 calculation
uses more data than the .so file size.

Set the .so file offset and size to the ModuleSpec, and set the size to
MapFileData length argument. For normal file, file_offset should be zero,
and length should be the size of the file.

[splhack]

clang-format

[splhack]

fix EXPECT_EQ warnings

[splhack]

rebase onto D152712

[splhack]

rebase

[bulbazord]

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

602

A length of -1 means "map the whole file", if my understanding is correct. Why do we want to change this to length instead of just -1? Or is this file the entire zip file?

[splhack]

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

602

I'm now updating this diff to add comments to clarify the intention.

For Android zip .so file (D152759),
the ModuleSpec spec will have these info with this diff.

• object offset = .so file offset inside the zip file
• object size = .so file size

PlatformAndroid requires this module spec to call DownloadModuleSlice in order to download the .so file from the zip file using adb shell dd.

• file spec = "zip_path!/lib_path"
• file offset = .so file offset inside the zip file == dd skip parameter
• file size = .so file size == dd count parameter

And ObjectFileELF will calculate CRC32 if the ELF does not have GNU BuildID, that will use the data_sp size. Which is set by MapFileData. Currently -1 == the whole zip file size. (In fact, lldb-server is crashing due to SIGSEGV.) It is supposed to be the .so file size.
In the unittest, the test .so file has CRC32 7D6E4738. And it should be the same for the file size.

offset-test.bin contains
-1024bytes of '\0'

• liboffset-test.so (file offset = 1024, file size = 3600, CRC32 '7D6E4738')
• 16bytes of '\0'

[kusmour]

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

602

Reading from Opening shared libraries directly from an APK
Because the .so file is now page aligned and has offset, the actual size should <= the size of file

[clayborg]

LGTM. Anyone else have any comments?

[splhack]

add comments

[splhack]

added non zip .so file (normal file) case to the comment

[splhack]

fixed typo and rebase onto D153102

[splhack]

removed abandoned dependency

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

size_t ObjectFileELF::GetModuleSpecifications(
   if (ObjectFileELF::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize())) {
     DataExtractor data;
     data.SetData(data_sp);
     elf::ELFHeader header;
     lldb::offset_t header_offset = data_offset;
     if (header.Parse(data, &header_offset)) {
       if (data_sp) {
         ModuleSpec spec(file);
+        // In Android API level 23 and above, bionic dynamic linker is able to
+        // load .so file directly from zip file. In that case, .so file is
+        // page aligned and uncompressed, and this module spec should retain the
+        // .so file offset and file size to pass through the information from
+        // lldb-server to LLDB. For normal file, file_offset should be 0,
+        // length should be the size of the file.
+        spec.SetObjectOffset(file_offset);
+        spec.SetObjectSize(length);
 
         const uint32_t sub_type = subTypeFromElfHeader(header);
         spec.GetArchitecture().SetArchitecture(
             eArchTypeELF, header.e_machine, sub_type, header.e_ident[EI_OSABI]);
 
         if (spec.GetArchitecture().IsValid()) {
           llvm::Triple::OSType ostype;
           llvm::Triple::VendorType vendor;
if (header.Parse(data, &header_offset)) {
           assert(spec_ostype == ostype);
           if (spec_ostype != llvm::Triple::OSType::UnknownOS) {
             LLDB_LOGF(log,
                       "ObjectFileELF::%s file '%s' set ELF module OS type "
                       "from ELF header OSABI.",
                       __FUNCTION__, file.GetPath().c_str());
           }
 
+          // When ELF file does not contain GNU build ID, the later code will
+          // calculate CRC32 with this data_sp file_offset and length. It is
+          // important for Android zip .so file, which is a slice of a file,
+          // to not access the outside of the file slice range.
           if (data_sp->GetByteSize() < length)
-            data_sp = MapFileData(file, -1, file_offset);
+            data_sp = MapFileData(file, length, file_offset);
           if (data_sp)
             data.SetData(data_sp);
           // In case there is header extension in the section #0, the header we
           // parsed above could have sentinel values for e_phnum, e_shnum, and
           // e_shstrndx.  In this case we need to reparse the header with a
           // bigger data source to get the actual values.
           if (header.HasHeaderExtension()) {
             lldb::offset_t header_offset = data_offset;

lldb/unittests/ObjectFile/ELF/CMakeLists.txt

 add_lldb_unittest(ObjectFileELFTests
   TestObjectFileELF.cpp
 
   LINK_LIBS
     lldbPluginObjectFileELF
     lldbPluginSymbolFileSymtab
     lldbCore
     lldbUtilityHelpers
     LLVMTestingSupport
   )
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")

lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp

TEST_F(ObjectFileELFTest, GetModuleSpecifications_EarlySectionHeaders) {
   ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
   ModuleSpec Spec;
   ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
   UUID Uuid;
   Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9");
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  // The contents of offset-test.bin are
+  // -    0-1023: \0
+  // - 1024-4623: liboffset-test.so (offset: 1024, size: 3600, CRC32: 7D6E4738)
+  // - 4624-4639: \0
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+      1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
   .thumb_func
   _start:
       mov r0, #42
       mov r7, #1
       svc #0