<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 06/12/2014 11:04 PM, Richard Trieu
wrote:<br>
</div>
<blockquote
cite="mid:CAPUYKWkcfZGsTh1NM_uyAfuM_bWKP8gwgEoO9vNhP-fREQ6z5g@mail.gmail.com"
type="cite">
<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 moz-do-not-send="true" href="http://reviews.llvm.org/D4131">http://reviews.llvm.org/D4131</a><br>
</div>
</blockquote>
Thanks. I'll review that shortly.<br>
<blockquote
cite="mid:CAPUYKWkcfZGsTh1NM_uyAfuM_bWKP8gwgEoO9vNhP-fREQ6z5g@mail.gmail.com"
type="cite">
<div dir="ltr">
<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>
</blockquote>
You're correct. I thought I'd remember some changes recently to
mark this pointers nonnull, but I appear to have been mistaken. I
think I crossed that with reference types. Sorry for the
confusion. <br>
<blockquote
cite="mid:CAPUYKWkcfZGsTh1NM_uyAfuM_bWKP8gwgEoO9vNhP-fREQ6z5g@mail.gmail.com"
type="cite">
<div dir="ltr">
</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 moz-do-not-send="true"
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 moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project?rev=210497&view=rev"
target="_blank">http://llvm.org/viewvc/llvm-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/CallGraphSCCPass.cpp<br>
llvm/trunk/lib/Analysis/IVUsers.cpp<br>
llvm/trunk/lib/Analysis/LoopPass.cpp<br>
llvm/trunk/lib/Analysis/RegionPass.cpp<br>
llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
llvm/trunk/lib/IR/AsmWriter.cpp<br>
llvm/trunk/lib/IR/Core.cpp<br>
llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp<br>
<br>
Modified: llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
(original)<br>
+++ llvm/trunk/lib/Analysis/IPA/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(Out);<br>
+ }<br>
return false;<br>
}<br>
};<br>
<br>
Modified: llvm/trunk/lib/Analysis/IVUsers.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/Analysis/IVUsers.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/IVUsers.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/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/LoopPass.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/Analysis/LoopPass.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/LoopPass.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/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/RegionPass.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/Analysis/RegionPass.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/RegionPass.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/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/AsmPrinter/AsmPrinter.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
(original)<br>
+++ llvm/trunk/lib/CodeGen/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(StrVal);<br>
+ assert(CFP->getType() != nullptr &&
"Expecting non-null Type");<br>
CFP->getType()->print(AP.OutStreamer.GetCommentOS());<br>
AP.OutStreamer.GetCommentOS() << ' ' <<
StrVal << '\n';<br>
}<br>
<br>
Modified: llvm/trunk/lib/IR/AsmWriter.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/IR/AsmWriter.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/AsmWriter.cpp (original)<br>
+++ llvm/trunk/lib/IR/AsmWriter.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*>(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 moz-do-not-send="true"
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-project/llvm/trunk/lib/IR/Core.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<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(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(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/Instrumentation/DebugIR.cpp<br>
URL: <a moz-do-not-send="true"
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-project/llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp?rev=210497&r1=210496&r2=210497&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp
(original)<br>
+++ llvm/trunk/lib/Transforms/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>
_______________________________________________<br>
llvm-commits mailing list<br>
<a moz-do-not-send="true"
href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a moz-do-not-send="true"
href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits"
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote>
<br>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>