[all-commits] [llvm/llvm-project] 561e1c: [lldb] Fix GetRemoteSharedModule fallback logic

Fri Dec 11 14:35:57 PST 2020

  Branch: refs/heads/release/11.x
  Home:   https://github.com/llvm/llvm-project
  Commit: 561e1ce1a82e98df60074cef6b63f640f4ef712c
      https://github.com/llvm/llvm-project/commit/561e1ce1a82e98df60074cef6b63f640f4ef712c
  Author: Joseph Tremoulet <jotrem at microsoft.com>
  Date:   2020-12-11 (Fri, 11 Dec 2020)

  Changed paths:
    M lldb/source/Target/Platform.cpp
    M lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py

  Log Message:
  -----------
  [lldb] Fix GetRemoteSharedModule fallback logic

When the various methods of locating the module in GetRemoteSharedModule
fail, make sure we pass the original module spec to the bail-out call to
the provided resolver function.

Also make sure we consistently use the resolved module spec from the
various success paths.

Thanks to what appears to have been an accidentally inverted condition
(commit 85967fa applied the new condition to a path where GetModuleSpec
returns false, but should have applied it when GetModuleSpec returns
true), without this fix we only pass the original module spec in the
fallback if the original spec has no uuid (or has a uuid that somehow
matches the resolved module's uuid despite the call to GetModuleSpec
failing).  This manifested as a bug when processing a minidump file with
a user-provided sysroot, since in that case the resolver call was being
applied to resolved_module_spec (despite resolution failing), which did
not have the path of its file_spec set.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D88099

(cherry picked from commit 20f84257ac4ac54ceb5f581a6081fac6eff2a5a1)

  Commit: 98fa273339a474ae9dcc3295ef10e75e38589dda
      https://github.com/llvm/llvm-project/commit/98fa273339a474ae9dcc3295ef10e75e38589dda
  Author: Joseph Tremoulet <jotrem at microsoft.com>
  Date:   2020-12-11 (Fri, 11 Dec 2020)

  Changed paths:
    M lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py

  Log Message:
  -----------
  [lldb] Normalize paths in new test

The minidump-sysroot test I added in commit 20f84257 compares two paths
using a string comparison.  This causes the Windows buildbot to fail
because of mismatched forward slashes and backslashes.  Use
os.path.normcase to normalize before comparing.

(cherry picked from commit 4a55c98fa7bee1e5ab1504db20ca4d7c8a083111)

  Commit: 393eac16e497d2e7cf67d881ba33644060f35c79
      https://github.com/llvm/llvm-project/commit/393eac16e497d2e7cf67d881ba33644060f35c79
  Author: Greg Clayton <gclayton at fb.com>
  Date:   2020-12-11 (Fri, 11 Dec 2020)

  Changed paths:
    M lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
    M lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
    A lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-overflow.yaml
    A lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml
    A lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-breakpad-uuid-match.yaml
    A lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-facebook-uuid-match.yaml

  Log Message:
  -----------
  Add hashing of the .text section to ProcessMinidump.

Breakpad will always have a UUID for binaries when it creates minidump files. If an ELF files has a GNU build ID, it will use that. If it doesn't, it will create one by hashing up to the first 4096 bytes of the .text section. LLDB was not able to load these binaries even when we had the right binary because the UUID didn't match. LLDB will use the GNU build ID first as the main UUID for a binary and fallback onto a 8 byte CRC if a binary doesn't have one. With this fix, we will check for the Breakpad hash or the Facebook hash (a modified version of the breakpad hash that collides a bit less) and accept binaries when these hashes match.

Differential Revision: https://reviews.llvm.org/D86261

(cherry picked from commit 0e6c9a6e7940a2f8ee624358d828acffdb9ccca5)

  Commit: 93fffe98d5c2f6471928433a41b8cb546ef2abda
      https://github.com/llvm/llvm-project/commit/93fffe98d5c2f6471928433a41b8cb546ef2abda
  Author: Joseph Tremoulet <jotrem at microsoft.com>
  Date:   2020-12-11 (Fri, 11 Dec 2020)

  Changed paths:
    M lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
    M lldb/source/Plugins/Process/minidump/ProcessMinidump.h
    M lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
    A lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-decoy.yaml

  Log Message:
  -----------
  [lldb] Minidump: check for .text hash match with directory

When opening a minidump, we might discover that it reports a UUID for a
module that doesn't match the build ID, but rather a hash of the .text
section (according to either of two different hash functions, used by
breakpad and Facebook respectively).  The current logic searches for a
module by filename only to check the hash; this change updates it to
first search by directory+filename.  This is important when the
directory specified in the minidump must be interpreted relative to a
user-provided sysoort, as the leaf directory won't be in the search path
in that case.

Also add a regression test; without this change, module validation fails
because we have just the placeholder module which reports as its path
the platform path in the minidump.

Reviewed By: clayborg

Differential Revision: https://reviews.llvm.org/D89155

(cherry picked from commit d30797b4041ffe215b92d376af60c4f26a0555ae)

  Commit: b618cf7a378d85f5630e24d3cf74ad3202732d72
      https://github.com/llvm/llvm-project/commit/b618cf7a378d85f5630e24d3cf74ad3202732d72
  Author: Joseph Tremoulet <jotrem at microsoft.com>
  Date:   2020-12-11 (Fri, 11 Dec 2020)

  Changed paths:
    M lldb/include/lldb/Core/ModuleList.h
    M lldb/include/lldb/Target/Platform.h
    M lldb/source/Core/ModuleList.cpp
    M lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
    M lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
    M lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
    M lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
    M lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
    M lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
    M lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
    M lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
    M lldb/source/Target/Platform.cpp
    M lldb/source/Target/Target.cpp

  Log Message:
  -----------
  [lldb] GetSharedModule: Collect old modules in SmallVector

The various GetSharedModule methods have an optional out parameter for
the old module when a file has changed or been replaced, which the
Target uses to keep its module list current/correct.  We've been using
a single ModuleSP to track "the" old module, and this change switches
to using a SmallVector of ModuleSP, which has a couple benefits:
 - There are multiple codepaths which may discover an old module, and
   this centralizes the code for how to handle multiples in one place,
   in the Target code.  With the single ModuleSP, each place that may
   discover an old module is responsible for how it handles multiples,
   and the current code is inconsistent (some code paths drop the first
   old module, others drop the second).
 - The API will be more natural for identifying old modules in routines
   that work on sets, like ModuleList::ReplaceEquivalent (which I plan
   on updating to report old module(s) in a subsequent change to fix a
   bug).

I'm not convinced we can ever actually run into the case that multiple
old modules are found in the same GetOrCreateModule call, but I think
this change makes sense regardless, in light of the above.

When an old module is reported, Target::GetOrCreateModule calls
m_images.ReplaceModule, which doesn't allow multiple "old" modules; the
new code calls ReplaceModule for the first "old" module, and for any
subsequent old modules it logs the event and calls m_images.Remove.

Reviewed By: jingham

Differential Revision: https://reviews.llvm.org/D89156

(cherry picked from commit 61bfc703c3d36fbefc476cd3829065d983c1c792)

  Commit: abeec5d081f0dfbee305b70d7641c8e5af9a9335
      https://github.com/llvm/llvm-project/commit/abeec5d081f0dfbee305b70d7641c8e5af9a9335
  Author: Joseph Tremoulet <jotrem at microsoft.com>
  Date:   2020-12-11 (Fri, 11 Dec 2020)

  Changed paths:
    M lldb/include/lldb/Core/ModuleList.h
    M lldb/source/Core/ModuleList.cpp
    M lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

  Log Message:
  -----------
  [lldb] Report old modules from ModuleList::ReplaceEquivalent

This allows the Target to update its module list when loading a shared
module replaces an equivalent one.

A testcase is added which hits this codepath -- without the fix, the
target reports libbreakpad.so twice in its module list.

Reviewed By: jingham

Differential Revision: https://reviews.llvm.org/D89157

(cherry picked from commit d20aa7ca422145fb4d07e16c1d0aa7de9e3554ea)

Compare: https://github.com/llvm/llvm-project/compare/852f4d8eb6d3...abeec5d081f0