<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Dec 2, 2013 at 4:26 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Mon, Dec 2, 2013 at 4:21 PM, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Mon, Dec 2, 2013 at 2:07 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
>><br>
>> > Add a helper function getDebugInfoVersionFromModule to return the debug<br>
>> > info<br>
>> > version number for a module.<br>
>> ><br>
>><br>
>> I think this should be getDebugMetadataVersionFromModule since this<br>
>> could be easily confused with the dwarf version that someone has asked<br>
>> for.<br>
><br>
><br>
> Done in r196172.<br>
>><br>
>><br>
>> > "Verifier/module-flags-1.ll" checks for verification errors.<br>
>> > It will seg fault when calling getDebugInfoVersionFromModule because of<br>
>> > the<br>
>> > incorrect format for module flags in the testing case. We make<br>
>> > getModuleFlagsMetadata more robust by checking for error conditions.<br>
>> ><br>
>><br>
>> I'm not sure what's going on here, can you elaborate?<br>
><br>
><br>
> We have the code path and the verification path. Code path should not seg<br>
> fault when the format of a module flag is incorrect (as in the testing case<br>
> Verifier/module-flags-1.ll).<br>
> module-flags-1.ll tries to make sure that the verifier dumps the right error<br>
> message when the format of a module flag is incorrect.<br>
><br>
> Before this patch, the code path will seg fault because we don't perform<br>
> error checking in the code path as shown below.<br>
><br>
<br>
</div>You seem to be hard coding certain operations leading me to believe<br>
there's some specific format here that you're looking for, but it's<br>
not obvious what it is</blockquote><div><br></div><div>Can you be specific on where I am hard coding operations? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- are you saying that if you turn off the<br>
verifier we'll crash on certain types of bitcode files that normally<br>
we wouldn't be able to read anyhow since we'd fail verification?<br></blockquote><div><br></div><div>No, it does not matter whether the verifier is on or off. The code path should not seg fault when the input bit code has incorrect module flag.</div>
<div><br></div><div>The reason my patch exposes the seg fault is that we parse the module flag before</div><div>the verifier and the testing case will seg fault before getting the verification error.</div><div><br></div><div>
Manman</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> Cheers,<br>
> Manman<br>
><br>
>><br>
>><br>
>> > Modified: llvm/trunk/lib/IR/Module.cpp<br>
>> > URL:<br>
>> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Module.cpp?rev=196158&r1=196157&r2=196158&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Module.cpp?rev=196158&r1=196157&r2=196158&view=diff</a><br>
>> ><br>
>> > ==============================================================================<br>
>> > --- llvm/trunk/lib/IR/Module.cpp (original)<br>
>> > +++ llvm/trunk/lib/IR/Module.cpp Mon Dec 2 15:29:56 2013<br>
>> > @@ -318,11 +318,16 @@ getModuleFlagsMetadata(SmallVectorImpl<M<br>
>> ><br>
>> > for (unsigned i = 0, e = ModFlags->getNumOperands(); i != e; ++i) {<br>
>> > MDNode *Flag = ModFlags->getOperand(i);<br>
>> > - ConstantInt *Behavior = cast<ConstantInt>(Flag->getOperand(0));<br>
>> > - MDString *Key = cast<MDString>(Flag->getOperand(1));<br>
>> > - Value *Val = Flag->getOperand(2);<br>
>> > -<br>
>> > Flags.push_back(ModuleFlagEntry(ModFlagBehavior(Behavior->getZExtValue()),<br>
>> > - Key, Val));<br>
>> > + if (Flag->getNumOperands() >= 3 &&<br>
>> > isa<ConstantInt>(Flag->getOperand(0)) &&<br>
>> > + isa<MDString>(Flag->getOperand(1))) {<br>
>> > + // Check the operands of the MDNode before accessing the<br>
>> > operands.<br>
>> > + // The verifier will actually catch these failures.<br>
>> > + ConstantInt *Behavior = cast<ConstantInt>(Flag->getOperand(0));<br>
>> > + MDString *Key = cast<MDString>(Flag->getOperand(1));<br>
>> > + Value *Val = Flag->getOperand(2);<br>
>> > +<br>
>> > Flags.push_back(ModuleFlagEntry(ModFlagBehavior(Behavior->getZExtValue()),<br>
>> > + Key, Val));<br>
>> > + }<br>
>> > }<br>
>> > }<br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > llvm-commits mailing list<br>
>> > <a href="mailto:llvm-commits@cs.uiuc.edu">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/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>