[llvm] r210497 - Removing an "if (!this)" check from two print methods. The condition will
Philip Reames
listmail at philipreames.com
Fri Jun 13 10:43:35 PDT 2014
On 06/12/2014 11:04 PM, Richard Trieu wrote:
> I've created and uploaded a patch that would put the printing back for
> null pointers. See:
>
> http://reviews.llvm.org/D4131
Thanks. I'll review that shortly.
>
> 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.
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.
>
>
> On Tue, Jun 10, 2014 at 10:24 AM, Philip Reames
> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>
> Richard,
>
> 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)
>
> 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.)
>
> Philip
>
> On 06/09/2014 03:53 PM, Richard Trieu wrote:
>
> Author: rtrieu
> Date: Mon Jun 9 17:53:16 2014
> New Revision: 210497
>
> URL: http://llvm.org/viewvc/llvm-project?rev=210497&view=rev
> Log:
> Removing an "if (!this)" check from two print methods. The
> condition will
> never be true in a well-defined context. The checking for
> null pointers
> has been moved into the caller logic so it does not rely on
> undefined behavior.
>
> Modified:
> llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
> llvm/trunk/lib/Analysis/IVUsers.cpp
> llvm/trunk/lib/Analysis/LoopPass.cpp
> llvm/trunk/lib/Analysis/RegionPass.cpp
> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> llvm/trunk/lib/IR/AsmWriter.cpp
> llvm/trunk/lib/IR/Core.cpp
> llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp
>
> Modified: llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp (original)
> +++ llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp Mon Jun
> 9 17:53:16 2014
> @@ -602,8 +602,10 @@ namespace {
> bool runOnSCC(CallGraphSCC &SCC) override {
> Out << Banner;
> - for (CallGraphSCC::iterator I = SCC.begin(), E =
> SCC.end(); I != E; ++I)
> + for (CallGraphSCC::iterator I = SCC.begin(), E =
> SCC.end(); I != E; ++I) {
> + assert((*I)->getFunction() && "Expecting non-null
> Function");
> (*I)->getFunction()->print(Out);
> + }
> return false;
> }
> };
>
> Modified: llvm/trunk/lib/Analysis/IVUsers.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IVUsers.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/IVUsers.cpp (original)
> +++ llvm/trunk/lib/Analysis/IVUsers.cpp Mon Jun 9 17:53:16 2014
> @@ -287,6 +287,7 @@ void IVUsers::print(raw_ostream &OS, con
> OS << ")";
> }
> OS << " in ";
> + assert(UI->getUser() != nullptr && "Expected non-null User");
> UI->getUser()->print(OS);
> OS << '\n';
> }
>
> Modified: llvm/trunk/lib/Analysis/LoopPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopPass.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/LoopPass.cpp (original)
> +++ llvm/trunk/lib/Analysis/LoopPass.cpp Mon Jun 9 17:53:16 2014
> @@ -45,6 +45,7 @@ public:
> for (Loop::block_iterator b = L->block_begin(), be =
> L->block_end();
> b != be;
> ++b) {
> + assert((*b) != nullptr && "Expecting non-null block");
> (*b)->print(Out);
> }
> return false;
>
> Modified: llvm/trunk/lib/Analysis/RegionPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/RegionPass.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/RegionPass.cpp (original)
> +++ llvm/trunk/lib/Analysis/RegionPass.cpp Mon Jun 9 17:53:16
> 2014
> @@ -195,8 +195,10 @@ public:
> bool runOnRegion(Region *R, RGPassManager &RGM) override {
> Out << Banner;
> - for (const auto &BB : R->blocks())
> + for (const auto &BB : R->blocks()) {
> + assert(BB != nullptr && "Expecting non-null Block");
> BB->print(Out);
> + }
> return false;
> }
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Mon Jun
> 9 17:53:16 2014
> @@ -1867,6 +1867,7 @@ static void emitGlobalConstantFP(const C
> SmallString<8> StrVal;
> CFP->getValueAPF().toString(StrVal);
> + assert(CFP->getType() != nullptr && "Expecting non-null
> Type");
> CFP->getType()->print(AP.OutStreamer.GetCommentOS());
> AP.OutStreamer.GetCommentOS() << ' ' << StrVal << '\n';
> }
>
> Modified: llvm/trunk/lib/IR/AsmWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/AsmWriter.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/AsmWriter.cpp (original)
> +++ llvm/trunk/lib/IR/AsmWriter.cpp Mon Jun 9 17:53:16 2014
> @@ -2156,10 +2156,6 @@ void NamedMDNode::print(raw_ostream &ROS
> }
> void Type::print(raw_ostream &OS) const {
> - if (!this) {
> - OS << "<null Type>";
> - return;
> - }
> TypePrinting TP;
> TP.print(const_cast<Type*>(this), OS);
> @@ -2172,10 +2168,6 @@ void Type::print(raw_ostream &OS) const
> }
> void Value::print(raw_ostream &ROS) const {
> - if (!this) {
> - ROS << "printing a <null> value\n";
> - return;
> - }
> formatted_raw_ostream OS(ROS);
> if (const Instruction *I = dyn_cast<Instruction>(this)) {
> const Function *F = I->getParent() ?
> I->getParent()->getParent() : nullptr;
>
> Modified: llvm/trunk/lib/IR/Core.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Core.cpp (original)
> +++ llvm/trunk/lib/IR/Core.cpp Mon Jun 9 17:53:16 2014
> @@ -281,6 +281,7 @@ char *LLVMPrintTypeToString(LLVMTypeRef
> std::string buf;
> raw_string_ostream os(buf);
> + assert(unwrap(Ty) != nullptr && "Expecting non-null Type");
> unwrap(Ty)->print(os);
> os.flush();
> @@ -531,6 +532,7 @@ char* LLVMPrintValueToString(LLVMValueRe
> std::string buf;
> raw_string_ostream os(buf);
> + assert(unwrap(Val) != nullptr && "Expecting non-null Value");
> unwrap(Val)->print(os);
> os.flush();
>
> Modified: llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp?rev=210497&r1=210496&r2=210497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/Instrumentation/DebugIR.cpp Mon
> Jun 9 17:53:16 2014
> @@ -352,6 +352,7 @@ private:
> }
> std::string getTypeName(Type *T) {
> + assert(T != nullptr && "Expecting non-null Type");
> std::string TypeName;
> raw_string_ostream TypeStream(TypeName);
> T->print(TypeStream);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140613/50c39a4a/attachment.html>
More information about the llvm-commits
mailing list