[llvm-branch-commits] [lldb] 85dfcaa - [LLDB] MinidumpParser: Prefer executable module even at higher address
Author: Joseph Tremoulet
Date: 2021-01-14T13:17:57-05:00
New Revision: 85dfcaadc5f0920dc8ecbece6c786701b8f45ab4
URL:
https://github.com/llvm/llvm-project/commit/85dfcaadc5f0920dc8ecbece6c786701b8f45ab4
DIFF:
https://github.com/llvm/llvm-project/commit/85dfcaadc5f0920dc8ecbece6c786701b8f45ab4.diff
LOG: [LLDB] MinidumpParser: Prefer executable module even at higher address
When a program maps one of its own modules for reading, and then
crashes, breakpad can emit two entries for that module in the
ModuleList. We have logic to identify this case by checking permissions
on mapped memory regions and report just the module with an executable
region. As currently written, though, the check is asymmetric -- the
entry with the executable region must be the second one encountered for
the preference to kick in.
This change makes the logic symmetric, so that the first-encountered
module will similarly be preferred if it has an executable region but
the second-encountered module does not. This happens for example when
the module in question is the executable itself, which breakpad likes to
report first -- we need to ignore the other entry for that module when
we see it later, even though it may be mapped at a lower virtual
address.
Reviewed By: clayborg
Differential Revision: https://reviews.llvm.org/D94629
Added:
Modified:
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/unittests/Process/minidump/MinidumpParserTest.cpp
Removed:
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index e16f86cca1c2..61106ebcc430 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -391,19 +391,23 @@ std::vector
MinidumpParser::GetFilteredModuleList() {
filtered_modules.push_back(&module);
} else {
// We have a duplicate module entry. Check the linux regions to see if
- // the module we already have is not really a mapped executable. If it
- // isn't check to see if the current duplicate module entry is a real
- // mapped executable, and if so, replace it. This can happen when a
- // process mmap's in the file for an executable in order to read bytes
- // from the executable file. A memory region mapping will exist for the
- // mmap'ed version and for the loaded executable, but only one will have
- // a consecutive region that is executable in the memory regions.
+ // either module is not really a mapped executable. If one but not the
+ // other is a real mapped executable, prefer the executable one. This
+ // can happen when a process mmap's in the file for an executable in
+ // order to read bytes from the executable file. A memory region mapping
+ // will exist for the mmap'ed version and for the loaded executable, but
+ // only one will have a consecutive region that is executable in the
+ // memory regions.
auto dup_module = filtered_modules[iter->second];
ConstString name(*ExpectedName);
- if (!CheckForLinuxExecutable(name, linux_regions,
- dup_module->BaseOfImage) &&
- CheckForLinuxExecutable(name, linux_regions, module.BaseOfImage)) {
-filtered_modules[iter->second] = &module;
+ bool is_executable =
+ CheckForLinuxExecutable(name, linux_regions, module.BaseOfImage);
+ bool dup_is_executable =
+ CheckForLinuxExecutable(name, linux_regions,
dup_module->BaseOfImage);
+
+ if (is_executable != dup_is_executable) {
+if (is_executable)
+ filtered_modules[iter->second] = &module;
continue;
}
// This module has been seen. Modules are sometimes mentioned multiple
diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index 69046af283eb..e3f23c5fe33a 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -792,6 +792,47 @@ TEST_F(MinidumpParserTest,
MinidumpDuplicateModuleMappedSecond) {
EXPECT_EQ(0x400du, filtered_modules[0]->BaseOfImage);
}
+TEST_F(MinidumpParserTest, MinidumpDuplicateModuleMappedSecondHigh) {
+ ASSERT_THAT_ERROR(SetUpFromYaml(R"(
+--- !minidump
+Streams:
+ - Type:ModuleList
+Modules:
+ - Base of Image: 0x400d3000
+Size of Image: 0x2000
+Module Name: '/usr/lib/libc.so'
+CodeView Record: ''
+ - Base of Image: 0x400d
+Size of Image: 0x1000
+Module Name: '/usr/lib/libc.so'
+CodeView Record: ''
+ - Type:LinuxMaps
+Text: |
+ 400d-400d2000 r--p b3:04 227/usr/lib/libc.so
+ 400d2000-400d3000 rw-p
[llvm-branch-commits] [llvm] 40cd262 - Loop peeling: check that latch is conditional branch
Author: Joseph Tremoulet
Date: 2021-01-20T11:01:16-05:00
New Revision: 40cd262c4339c8cbd67bf5c96c4a052ae02a8660
URL:
https://github.com/llvm/llvm-project/commit/40cd262c4339c8cbd67bf5c96c4a052ae02a8660
DIFF:
https://github.com/llvm/llvm-project/commit/40cd262c4339c8cbd67bf5c96c4a052ae02a8660.diff
LOG: Loop peeling: check that latch is conditional branch
Loop peeling assumes that the loop's latch is a conditional branch. Add
a check to canPeel that explicitly checks for this, and testcases that
otherwise fail an assertion when trying to peel a loop whose back-edge
is a switch case or the non-unwind edge of an invoke.
Reviewed By: skatkov, fhahn
Differential Revision: https://reviews.llvm.org/D94995
Added:
Modified:
llvm/lib/Transforms/Utils/LoopPeel.cpp
llvm/test/Transforms/LoopUnroll/peel-loop-conditions.ll
Removed:
diff --git a/llvm/lib/Transforms/Utils/LoopPeel.cpp
b/llvm/lib/Transforms/Utils/LoopPeel.cpp
index 27a61a207868..cb5fee7d28e6 100644
--- a/llvm/lib/Transforms/Utils/LoopPeel.cpp
+++ b/llvm/lib/Transforms/Utils/LoopPeel.cpp
@@ -115,7 +115,12 @@ bool llvm::canPeel(Loop *L) {
// This can be an indication of two
diff erent things:
// 1) The loop is not rotated.
// 2) The loop contains irreducible control flow that involves the latch.
- if (L->getLoopLatch() != L->getExitingBlock())
+ const BasicBlock *Latch = L->getLoopLatch();
+ if (Latch != L->getExitingBlock())
+return false;
+
+ // Peeling is only supported if the latch is a branch.
+ if (!isa(Latch->getTerminator()))
return false;
return true;
diff --git a/llvm/test/Transforms/LoopUnroll/peel-loop-conditions.ll
b/llvm/test/Transforms/LoopUnroll/peel-loop-conditions.ll
index f0fbf3d6d49b..fa7e13397e25 100644
--- a/llvm/test/Transforms/LoopUnroll/peel-loop-conditions.ll
+++ b/llvm/test/Transforms/LoopUnroll/peel-loop-conditions.ll
@@ -1140,5 +1140,69 @@ for.end:
ret void
}
+; Invoke is not a conditional branch that we can optimize,
+; so this shouldn't be peeled at all. This is a reproducer
+; for a bug where evaluating the loop would fail an assertion.
+define void @test17() personality i8* undef{
+; CHECK-LABEL: @test17(
+; CHECK-NEXT: body:
+; CHECK-NEXT:br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT:[[CONST:%.*]] = phi i64 [ -33, [[LOOP]] ], [ -20,
[[BODY:%.*]] ]
+; CHECK-NEXT:invoke void @f1()
+; CHECK-NEXT:to label [[LOOP]] unwind label [[EH_UNW_LOOPEXIT:%.*]]
+; CHECK: eh.Unw.loopexit:
+; CHECK-NEXT:[[LPAD_LOOPEXIT:%.*]] = landingpad { i8*, i32 }
+; CHECK-NEXT:catch i8* null
+; CHECK-NEXT:ret void
+;
+body:
+ br label %loop
+
+loop:
+ %const = phi i64 [ -33, %loop ], [ -20, %body ]
+ invoke void @f1()
+ to label %loop unwind label %eh.Unw.loopexit
+
+eh.Unw.loopexit:
+ %lpad.loopexit = landingpad { i8*, i32 }
+ catch i8* null
+ ret void
+}
+
+; Testcase reduced from PR48812. We expect no peeling
+; because the latch terminator is a switch.
+define void @test18(i32* %p) {
+; CHECK-LABEL: @test18(
+; CHECK-NEXT: init:
+; CHECK-NEXT:br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT:[[CONST:%.*]] = phi i32 [ 40, [[INIT:%.*]] ], [ 0,
[[LATCH:%.*]] ]
+; CHECK-NEXT:br label [[LATCH]]
+; CHECK: latch:
+; CHECK-NEXT:[[CONTROL:%.*]] = load volatile i32, i32* [[P:%.*]], align 4
+; CHECK-NEXT:switch i32 [[CONTROL]], label [[EXIT:%.*]] [
+; CHECK-NEXT:i32 2, label [[LOOP]]
+; CHECK-NEXT:]
+; CHECK: exit:
+; CHECK-NEXT:ret void
+;
+init:
+ br label %loop
+
+loop:
+ %const = phi i32 [ 40, %init ], [ 0, %latch ]
+ br label %latch
+
+latch:
+ %control = load volatile i32, i32* %p
+ switch i32 %control, label %exit [
+ i32 2, label %loop
+ ]
+
+exit:
+ ret void
+}
+
declare void @init()
declare void @sink()
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
