<div dir="ltr">It'd still be good to have a test case for this - perhaps you could narrow it down by putting a (temporary/not committed) assertion that would fire if the container was modified within the loop? Then use that to more deterministically reduce a test case.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 28, 2017 at 3:43 PM Dimitry Andric via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dim<br>
Date: Thu Dec 28 15:42:44 2017<br>
New Revision: 321545<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321545&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=321545&view=rev</a><br>
Log:<br>
Avoid modifying DbgInfo while looping in salvageDebuginfo<br>
<br>
Summary:<br>
I have been getting rather difficult to reproduce SIGBUS crashes when<br>
compiling certain FreeBSD sources, and their stack traces pointed<br>
squarely at `SelectionDAG::salvageDebugInfo()`:<br>
<br>
```<br>
Core was generated by `/usr/obj/share/dim/src/freebsd/clang600-import/amd64.amd64/tmp/usr/bin/cc -cc1 -'.<br>
Program terminated with signal SIGBUS, Bus error.<br>
#0 isInvalidated () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:115<br>
115 bool isInvalidated() const { return Invalid; }<br>
(gdb) bt<br>
#0 isInvalidated () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:115<br>
#1 salvageDebugInfo () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7116<br>
#2 0x00000000033b2516 in operator() () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:3595<br>
#3 __invoke<(lambda at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:3593:59) &, llvm::SDNode *, llvm::SDNode *> () at /usr/include/c++/v1/type_traits:4323<br>
#4 __call<(lambda at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:3593:59) &, llvm::SDNode *, llvm::SDNode *> () at /usr/include/c++/v1/__functional_base:349<br>
#5 operator() () at /usr/include/c++/v1/functional:1562<br>
#6 0x00000000033b0817 in operator() () at /usr/include/c++/v1/functional:1916<br>
#7 NodeDeleted () at /share/dim/src/freebsd/clang600-import/contrib/llvm/include/llvm/CodeGen/SelectionDAG.h:293<br>
#8 0x0000000003529dde in RemoveDeadNodes () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:610<br>
#9 0x00000000035556df in MorphNodeTo () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6794<br>
#10 0x00000000033a9acc in MorphNode () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2594<br>
#11 0x00000000033ac80b in SelectCodeCommon () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:3601<br>
#12 0x00000000023d464b in SelectCode () at /usr/obj/share/dim/src/freebsd/clang600-import/amd64.amd64/tmp/obj-tools/lib/clang/libllvm/X86GenDAGISel.inc:282902<br>
#13 Select () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:3072<br>
#14 0x00000000033a5afa in DoInstructionSelection () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:988<br>
#15 0x00000000033a4e1a in CodeGenAndEmitDAG () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:868<br>
#16 0x00000000033a2643 in SelectAllBasicBlocks () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1624<br>
#17 0x000000000339f158 in runOnMachineFunction () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:466<br>
#18 0x00000000023d03c4 in runOnMachineFunction () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:175<br>
#19 0x00000000035cc8c2 in runOnFunction () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/MachineFunctionPass.cpp:62<br>
#20 0x00000000030dca9a in runOnFunction () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/IR/LegacyPassManager.cpp:1520<br>
#21 0x00000000030dccf3 in runOnModule () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/IR/LegacyPassManager.cpp:1541<br>
#22 0x00000000030dd228 in runOnModule () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/IR/LegacyPassManager.cpp:1597<br>
#23 run () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/IR/LegacyPassManager.cpp:1700<br>
#24 0x00000000014db578 in EmitAssembly () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/CodeGen/BackendUtil.cpp:815<br>
#25 EmitBackendOutput () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/CodeGen/BackendUtil.cpp:1181<br>
#26 0x00000000014d5b26 in HandleTranslationUnit () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/CodeGen/CodeGenAction.cpp:292<br>
#27 0x0000000001c4c332 in ParseAST () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/Parse/ParseAST.cpp:159<br>
#28 0x00000000015d546c in Execute () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:897<br>
#29 0x0000000001cec311 in ExecuteAction () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:991<br>
#30 0x00000000014b4f81 in ExecuteCompilerInvocation () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:252<br>
#31 0x00000000014aa73f in cc1_main () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/tools/driver/cc1_main.cpp:221<br>
#32 0x00000000014b2928 in ExecuteCC1Tool () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/tools/driver/driver.cpp:309<br>
#33 main () at /share/dim/src/freebsd/clang600-import/contrib/llvm/tools/clang/tools/driver/driver.cpp:388<br>
(gdb) frame 1<br>
#1 salvageDebugInfo () at /share/dim/src/freebsd/clang600-import/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7116<br>
7116 if (DV->isInvalidated())<br>
(gdb) disassemble<br>
Dump of assembler code for function salvageDebugInfo():<br>
[...]<br>
0x0000000003557348 <+744>: nopl 0x0(%rax,%rax,1)<br>
0x0000000003557350 <+752>: mov (%r12),%r13<br>
=> 0x0000000003557354 <+756>: cmpb $0x0,0x31(%r13)<br>
0x0000000003557359 <+761>: jne 0x35573b0 <salvageDebugInfo()+848><br>
(gdb) info registers<br>
[...]<br>
r13 0x5a5a5a5a5a5a5a5a 6510615555426900570<br>
```<br>
<br>
The `0x5a5a5a5a5a5a5a5a` value in `r13` indicates the memory was either<br>
uninitialized, or already freed.<br>
<br>
Unfortunately I do not have a simple self-contained test case for this.<br>
However, it seems pretty clear that the call to `AddDbgValue()` in<br>
`salvageDebugInfo()` causes the problems, since it modifies<br>
`SelectionDag::DbgInfo` while looping through one of its DenseMaps:<br>
<br>
```<br>
void SelectionDAG::salvageDebugInfo(SDNode &N) {<br>
[...]<br>
for (auto DV : GetDbgValues(&N)) {<br>
if (DV->isInvalidated())<br>
continue;<br>
[...]<br>
AddDbgValue(Clone, N0.getNode(), false);<br>
[...]<br>
}<br>
}<br>
```<br>
<br>
At least, if I comment out the `AddDbgValue()` call, the crashes go<br>
away. I propose to change this function slightly, similar to the<br>
`SelectionDAG::transferDbgValues()` function just above it, to save the<br>
cloned SDDbgValues in a separate SmallVector, and only call<br>
AddDbgValue() on them after the for loop is done.<br>
<br>
Reviewers: aprantl, bogner, bkramer, davide<br>
<br>
Reviewed By: davide<br>
<br>
Subscribers: davide, krytarowski, JDevlieghere, emaste, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D41589" rel="noreferrer" target="_blank">https://reviews.llvm.org/D41589</a><br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=321545&r1=321544&r2=321545&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=321545&r1=321544&r2=321545&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Thu Dec 28 15:42:44 2017<br>
@@ -7128,6 +7128,8 @@ void SelectionDAG::transferDbgValues(SDV<br>
void SelectionDAG::salvageDebugInfo(SDNode &N) {<br>
if (!N.getHasDebugValue())<br>
return;<br>
+<br>
+ SmallVector<SDDbgValue *, 2> ClonedDVs;<br>
for (auto DV : GetDbgValues(&N)) {<br>
if (DV->isInvalidated())<br>
continue;<br>
@@ -7151,13 +7153,16 @@ void SelectionDAG::salvageDebugInfo(SDNo<br>
SDDbgValue *Clone =<br>
getDbgValue(DV->getVariable(), DIExpr, N0.getNode(), N0.getResNo(),<br>
DV->isIndirect(), DV->getDebugLoc(), DV->getOrder());<br>
+ ClonedDVs.push_back(Clone);<br>
DV->setIsInvalidated();<br>
- AddDbgValue(Clone, N0.getNode(), false);<br>
DEBUG(dbgs() << "SALVAGE: Rewriting"; N0.getNode()->dumprFull(this);<br>
dbgs() << " into " << *DIExpr << '\n');<br>
}<br>
}<br>
}<br>
+<br>
+ for (SDDbgValue *Dbg : ClonedDVs)<br>
+ AddDbgValue(Dbg, Dbg->getSDNode(), false);<br>
}<br>
<br>
namespace {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>