<div dir="ltr"><div>I've created and uploaded a patch that would put the printing back for null pointers. See:</div><div><br></div><a href="http://reviews.llvm.org/D4131">http://reviews.llvm.org/D4131</a><br><div><br>
</div><div>I am a little unsure about your statement. The compiler would not have the information to deduce that the pointers are always non-null, so these new checks would be included in all compiles, not just the Debug+Asserts build.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 10, 2014 at 10:24 AM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Richard,<br>
<br>
Your change looks like it will have the effect of making several printing paths less robust against null pointers. While these aren't expected, I have seen cases when debugging that the object had gotten corrupted and printing the null value was helpful (instead of crashing). (i.e. some internal field was set to null when it shouldn't be)<br>
<br>
Could you extend the callers to print the null message instead of simply asserting? This would preserve existing behaviour of a Debug+Asserts build. (In Release, these would have been optimized away in most cases anyways.)<br>
<br>
Philip<br>
<br>
On 06/09/2014 03:53 PM, Richard Trieu wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rtrieu<br>
Date: Mon Jun 9 17:53:16 2014<br>
New Revision: 210497<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=210497&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=210497&view=rev</a><br>
Log:<br>
Removing an "if (!this)" check from two print methods. The condition will<br>
never be true in a well-defined context. The checking for null pointers<br>
has been moved into the caller logic so it does not rely on undefined behavior.<br>
<br>
Modified:<br>
llvm/trunk/lib/Analysis/IPA/<u></u>CallGraphSCCPass.cpp<br>
llvm/trunk/lib/Analysis/<u></u>IVUsers.cpp<br>
llvm/trunk/lib/Analysis/<u></u>LoopPass.cpp<br>
llvm/trunk/lib/Analysis/<u></u>RegionPass.cpp<br>
llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/AsmPrinter.cpp<br>
llvm/trunk/lib/IR/AsmWriter.<u></u>cpp<br>
llvm/trunk/lib/IR/Core.cpp<br>
llvm/trunk/lib/Transforms/<u></u>Instrumentation/DebugIR.cpp<br>
<br>
Modified: llvm/trunk/lib/Analysis/IPA/<u></u>CallGraphSCCPass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Analysis/IPA/CallGraphSCCPass.<u></u>cpp?rev=210497&r1=210496&r2=<u></u>210497&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/Analysis/IPA/<u></u>CallGraphSCCPass.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/IPA/<u></u>CallGraphSCCPass.cpp Mon Jun 9 17:53:16 2014<br>
@@ -602,8 +602,10 @@ namespace {<br>
bool runOnSCC(CallGraphSCC &SCC) override {<br>
Out << Banner;<br>
- for (CallGraphSCC::iterator I = SCC.begin(), E = SCC.end(); I != E; ++I)<br>
+ for (CallGraphSCC::iterator I = SCC.begin(), E = SCC.end(); I != E; ++I) {<br>
+ assert((*I)->getFunction() && "Expecting non-null Function");<br>
(*I)->getFunction()->print(<u></u>Out);<br>
+ }<br>
return false;<br>
}<br>
};<br>
<br>
Modified: llvm/trunk/lib/Analysis/<u></u>IVUsers.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IVUsers.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Analysis/IVUsers.cpp?rev=<u></u>210497&r1=210496&r2=210497&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/Analysis/<u></u>IVUsers.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/<u></u>IVUsers.cpp Mon Jun 9 17:53:16 2014<br>
@@ -287,6 +287,7 @@ void IVUsers::print(raw_ostream &OS, con<br>
OS << ")";<br>
}<br>
OS << " in ";<br>
+ assert(UI->getUser() != nullptr && "Expected non-null User");<br>
UI->getUser()->print(OS);<br>
OS << '\n';<br>
}<br>
<br>
Modified: llvm/trunk/lib/Analysis/<u></u>LoopPass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopPass.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Analysis/LoopPass.cpp?rev=<u></u>210497&r1=210496&r2=210497&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/Analysis/<u></u>LoopPass.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/<u></u>LoopPass.cpp Mon Jun 9 17:53:16 2014<br>
@@ -45,6 +45,7 @@ public:<br>
for (Loop::block_iterator b = L->block_begin(), be = L->block_end();<br>
b != be;<br>
++b) {<br>
+ assert((*b) != nullptr && "Expecting non-null block");<br>
(*b)->print(Out);<br>
}<br>
return false;<br>
<br>
Modified: llvm/trunk/lib/Analysis/<u></u>RegionPass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/RegionPass.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Analysis/RegionPass.cpp?rev=<u></u>210497&r1=210496&r2=210497&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/Analysis/<u></u>RegionPass.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/<u></u>RegionPass.cpp Mon Jun 9 17:53:16 2014<br>
@@ -195,8 +195,10 @@ public:<br>
bool runOnRegion(Region *R, RGPassManager &RGM) override {<br>
Out << Banner;<br>
- for (const auto &BB : R->blocks())<br>
+ for (const auto &BB : R->blocks()) {<br>
+ assert(BB != nullptr && "Expecting non-null Block");<br>
BB->print(Out);<br>
+ }<br>
return false;<br>
}<br>
<br>
Modified: llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/AsmPrinter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>CodeGen/AsmPrinter/AsmPrinter.<u></u>cpp?rev=210497&r1=210496&r2=<u></u>210497&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/AsmPrinter.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/AsmPrinter.cpp Mon Jun 9 17:53:16 2014<br>
@@ -1867,6 +1867,7 @@ static void emitGlobalConstantFP(const C<br>
SmallString<8> StrVal;<br>
CFP->getValueAPF().toString(<u></u>StrVal);<br>
+ assert(CFP->getType() != nullptr && "Expecting non-null Type");<br>
CFP->getType()->print(AP.<u></u>OutStreamer.GetCommentOS());<br>
AP.OutStreamer.GetCommentOS() << ' ' << StrVal << '\n';<br>
}<br>
<br>
Modified: llvm/trunk/lib/IR/AsmWriter.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/AsmWriter.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/IR/<u></u>AsmWriter.cpp?rev=210497&r1=<u></u>210496&r2=210497&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/IR/AsmWriter.<u></u>cpp (original)<br>
+++ llvm/trunk/lib/IR/AsmWriter.<u></u>cpp Mon Jun 9 17:53:16 2014<br>
@@ -2156,10 +2156,6 @@ void NamedMDNode::print(raw_ostream &ROS<br>
}<br>
void Type::print(raw_ostream &OS) const {<br>
- if (!this) {<br>
- OS << "<null Type>";<br>
- return;<br>
- }<br>
TypePrinting TP;<br>
TP.print(const_cast<Type*>(<u></u>this), OS);<br>
@@ -2172,10 +2168,6 @@ void Type::print(raw_ostream &OS) const<br>
}<br>
void Value::print(raw_ostream &ROS) const {<br>
- if (!this) {<br>
- ROS << "printing a <null> value\n";<br>
- return;<br>
- }<br>
formatted_raw_ostream OS(ROS);<br>
if (const Instruction *I = dyn_cast<Instruction>(this)) {<br>
const Function *F = I->getParent() ? I->getParent()->getParent() : nullptr;<br>
<br>
Modified: llvm/trunk/lib/IR/Core.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/IR/<u></u>Core.cpp?rev=210497&r1=210496&<u></u>r2=210497&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/IR/Core.cpp (original)<br>
+++ llvm/trunk/lib/IR/Core.cpp Mon Jun 9 17:53:16 2014<br>
@@ -281,6 +281,7 @@ char *LLVMPrintTypeToString(<u></u>LLVMTypeRef<br>
std::string buf;<br>
raw_string_ostream os(buf);<br>
+ assert(unwrap(Ty) != nullptr && "Expecting non-null Type");<br>
unwrap(Ty)->print(os);<br>
os.flush();<br>
@@ -531,6 +532,7 @@ char* LLVMPrintValueToString(<u></u>LLVMValueRe<br>
std::string buf;<br>
raw_string_ostream os(buf);<br>
+ assert(unwrap(Val) != nullptr && "Expecting non-null Value");<br>
unwrap(Val)->print(os);<br>
os.flush();<br>
<br>
Modified: llvm/trunk/lib/Transforms/<u></u>Instrumentation/DebugIR.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp?rev=210497&r1=210496&r2=210497&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Transforms/Instrumentation/<u></u>DebugIR.cpp?rev=210497&r1=<u></u>210496&r2=210497&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/Transforms/<u></u>Instrumentation/DebugIR.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/<u></u>Instrumentation/DebugIR.cpp Mon Jun 9 17:53:16 2014<br>
@@ -352,6 +352,7 @@ private:<br>
}<br>
std::string getTypeName(Type *T) {<br>
+ assert(T != nullptr && "Expecting non-null Type");<br>
std::string TypeName;<br>
raw_string_ostream TypeStream(TypeName);<br>
T->print(TypeStream);<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote>
<br>
</blockquote></div><br></div>