[llvm] r210497 - Removing an "if (!this)" check from two print methods. The condition will
Richard Trieu
rtrieu at google.com
Thu Jun 12 23:04:19 PDT 2014
I've created and uploaded a patch that would put the printing back for null
pointers. See:
http://reviews.llvm.org/D4131
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.
On Tue, Jun 10, 2014 at 10:24 AM, Philip Reames <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
>> 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/20140612/6901cafe/attachment.html>
More information about the llvm-commits
mailing list