kusmour added inline comments.

================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:592
           if (data_sp->GetByteSize() < length)
-            data_sp = MapFileData(file, -1, file_offset);
+            data_sp = MapFileData(file, length, file_offset);
           if (data_sp)
----------------
splhack wrote:
> bulbazord wrote:
> > 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?
> 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'
Reading from [[ 
https://android.googlesource.com/platform/bionic/+/master/android-changes-for-ndk-developers.md#opening-shared-libraries-directly-from-an-apk
 | 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`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152757

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

Reply via email to