[Lldb-commits] [PATCH] D106584: [lldb] Improve checking of file cache read eligibility for mach-O

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

In D106584#2901529 <https://reviews.llvm.org/D106584#2901529>, @vsk wrote:

> Hey Augusto, thanks for tackling this, I'm just now slowly paging things in.
>
> Is this a correct statement of the problem: LLDB is failing to disable its file cache optimization when reading writable segments (say, __DATA) from a MachO sourced from the shared cache?
>
> If that's right, then I wonder whether you considered "simply" doing a bounds check on the address? The shared region should be mapped at a fixed virtual range in the debuggee process, and we can determine that range using dyld APIs.

Hey Vedant! Not exactly. Jason's description is spot on.

@jasonmolenda thanks for putting so much thought into this!

> (post-shared-cache on-disk-binaries may have the same offsets as it was in the original shared cache -- but it's actually going to be a MORE subtle bug in that case because people plug in different iPhones and Xcode only extracts the shared cache for one of them if they're all running the same build. But different model iphones will have different shared caches, so using the offsets from the post-shared-cache on-disk-binary-image from one device for another device will be wrong.)

I finally got a watch to test with, and found that the bug still happens on dylibs extracted from the shared cache, and this might be the problem.

I'm updating the patch and keeping only the assert, and think of another way of solving the problem.



================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:7040
+  // from, since offsets may be changed by the shared cache builder.
+  bool contains_split_info = ContainsLoadCommand(LC_SEGMENT_SPLIT_INFO);
+
----------------
vsk wrote:
> Is LC_SEGMENT_SPLIT_INFO present if and only if the MachO is from the shared cache?
No, it means that a dylib is //eligible// for being incorporated in the shared cache later on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584