[llvm] r210497 - Removing an "if (!this)" check from two print methods. The condition will
Philip Reames
listmail at philipreames.com
Tue Jun 10 10:24:36 PDT 2014
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
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list