<div dir="ltr">Hi Vedant,<div><br></div><div>not on top of my head. Dmitriy, can you please take a look?</div><div><br></div><div>krasin</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 27, 2017 at 12:43 PM, Vedant Kumar <span dir="ltr"><<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Ivan,<br>
<br>
I saw a bot failure on your job after this commit:<br>
<br>
<a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/5467/steps/tsan%20analyze/logs/stdio" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/<wbr>builders/sanitizer-x86_64-<wbr>linux-autoconf/builds/5467/<wbr>steps/tsan%20analyze/logs/<wbr>stdio</a><br>
<a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/5467/steps/build%20release%20tsan%20with%20clang/logs/stdio" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/<wbr>builders/sanitizer-x86_64-<wbr>linux-autoconf/builds/5467/<wbr>steps/build%20release%20tsan%<wbr>20with%20clang/logs/stdio</a><br>
<br>
However, I cannot reproduce it locally with a stage2 TSAN build.<br>
<br>
After staring at my diff I couldn't find anything that would explain the failure. No other bots seem upset.<br>
<br>
Do you have any idea about what's going on? Let me know if you want me to revert...<br>
<span class="HOEnZb"><font color="#888888"><br>
vedant<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On Feb 27, 2017, at 11:46 AM, Vedant Kumar via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: vedantk<br>
> Date: Mon Feb 27 13:46:19 2017<br>
> New Revision: 296374<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=296374&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=296374&view=rev</a><br>
> Log:<br>
> [ubsan] Factor out logic to emit a range check. NFC.<br>
><br>
> This is a readability improvement, but it will also help prep an<br>
> upcoming patch to detect UB loads from bitfields.<br>
><br>
> Modified:<br>
> cfe/trunk/lib/CodeGen/CGExpr.<wbr>cpp<br>
> cfe/trunk/lib/CodeGen/<wbr>CodeGenFunction.h<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.<wbr>cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=296374&r1=296373&r2=296374&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/CodeGen/<wbr>CGExpr.cpp?rev=296374&r1=<wbr>296373&r2=296374&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- cfe/trunk/lib/CodeGen/CGExpr.<wbr>cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/CGExpr.<wbr>cpp Mon Feb 27 13:46:19 2017<br>
> @@ -1301,6 +1301,46 @@ llvm::MDNode *CodeGenFunction::getRangeF<br>
> return MDHelper.createRange(Min, End);<br>
> }<br>
><br>
> +bool CodeGenFunction::<wbr>EmitScalarRangeCheck(llvm::<wbr>Value *Value, QualType Ty,<br>
> + SourceLocation Loc) {<br>
> + bool HasBoolCheck = SanOpts.has(SanitizerKind::<wbr>Bool);<br>
> + bool HasEnumCheck = SanOpts.has(SanitizerKind::<wbr>Enum);<br>
> + if (!HasBoolCheck && !HasEnumCheck)<br>
> + return false;<br>
> +<br>
> + bool IsBool = hasBooleanRepresentation(Ty) ||<br>
> + NSAPI(CGM.getContext()).<wbr>isObjCBOOLType(Ty);<br>
> + bool NeedsBoolCheck = HasBoolCheck && IsBool;<br>
> + bool NeedsEnumCheck = HasEnumCheck && Ty->getAs<EnumType>();<br>
> + if (!NeedsBoolCheck && !NeedsEnumCheck)<br>
> + return false;<br>
> +<br>
> + llvm::APInt Min, End;<br>
> + if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool))<br>
> + return true;<br>
> +<br>
> + SanitizerScope SanScope(this);<br>
> + llvm::Value *Check;<br>
> + --End;<br>
> + if (!Min) {<br>
> + Check = Builder.CreateICmpULE(<br>
> + Value, llvm::ConstantInt::get(<wbr>getLLVMContext(), End));<br>
> + } else {<br>
> + llvm::Value *Upper = Builder.CreateICmpSLE(<br>
> + Value, llvm::ConstantInt::get(<wbr>getLLVMContext(), End));<br>
> + llvm::Value *Lower = Builder.CreateICmpSGE(<br>
> + Value, llvm::ConstantInt::get(<wbr>getLLVMContext(), Min));<br>
> + Check = Builder.CreateAnd(Upper, Lower);<br>
> + }<br>
> + llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(Loc),<br>
> + EmitCheckTypeDescriptor(Ty)};<br>
> + SanitizerMask Kind =<br>
> + NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool;<br>
> + EmitCheck(std::make_pair(<wbr>Check, Kind), SanitizerHandler::<wbr>LoadInvalidValue,<br>
> + StaticArgs, EmitCheckValue(Value));<br>
> + return true;<br>
> +}<br>
> +<br>
> llvm::Value *CodeGenFunction::<wbr>EmitLoadOfScalar(Address Addr, bool Volatile,<br>
> QualType Ty,<br>
> SourceLocation Loc,<br>
> @@ -1353,35 +1393,9 @@ llvm::Value *CodeGenFunction::EmitLoadOf<br>
> false /*ConvertTypeToTag*/);<br>
> }<br>
><br>
> - bool IsBool = hasBooleanRepresentation(Ty) ||<br>
> - NSAPI(CGM.getContext()).<wbr>isObjCBOOLType(Ty);<br>
> - bool NeedsBoolCheck = SanOpts.has(SanitizerKind::<wbr>Bool) && IsBool;<br>
> - bool NeedsEnumCheck =<br>
> - SanOpts.has(SanitizerKind::<wbr>Enum) && Ty->getAs<EnumType>();<br>
> - if (NeedsBoolCheck || NeedsEnumCheck) {<br>
> - SanitizerScope SanScope(this);<br>
> - llvm::APInt Min, End;<br>
> - if (getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool)) {<br>
> - --End;<br>
> - llvm::Value *Check;<br>
> - if (!Min)<br>
> - Check = Builder.CreateICmpULE(<br>
> - Load, llvm::ConstantInt::get(<wbr>getLLVMContext(), End));<br>
> - else {<br>
> - llvm::Value *Upper = Builder.CreateICmpSLE(<br>
> - Load, llvm::ConstantInt::get(<wbr>getLLVMContext(), End));<br>
> - llvm::Value *Lower = Builder.CreateICmpSGE(<br>
> - Load, llvm::ConstantInt::get(<wbr>getLLVMContext(), Min));<br>
> - Check = Builder.CreateAnd(Upper, Lower);<br>
> - }<br>
> - llvm::Constant *StaticArgs[] = {<br>
> - EmitCheckSourceLocation(Loc),<br>
> - EmitCheckTypeDescriptor(Ty)<br>
> - };<br>
> - SanitizerMask Kind = NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool;<br>
> - EmitCheck(std::make_pair(<wbr>Check, Kind), SanitizerHandler::<wbr>LoadInvalidValue,<br>
> - StaticArgs, EmitCheckValue(Load));<br>
> - }<br>
> + if (EmitScalarRangeCheck(Load, Ty, Loc)) {<br>
> + // In order to prevent the optimizer from throwing away the check, don't<br>
> + // attach range metadata to the load.<br>
> } else if (CGM.getCodeGenOpts().<wbr>OptimizationLevel > 0)<br>
> if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))<br>
> Load->setMetadata(llvm::<wbr>LLVMContext::MD_range, RangeInfo);<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/<wbr>CodeGenFunction.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=296374&r1=296373&r2=296374&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/CodeGen/<wbr>CodeGenFunction.h?rev=296374&<wbr>r1=296373&r2=296374&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- cfe/trunk/lib/CodeGen/<wbr>CodeGenFunction.h (original)<br>
> +++ cfe/trunk/lib/CodeGen/<wbr>CodeGenFunction.h Mon Feb 27 13:46:19 2017<br>
> @@ -2866,6 +2866,13 @@ public:<br>
> /// representation to its value representation.<br>
> llvm::Value *EmitFromMemory(llvm::Value *Value, QualType Ty);<br>
><br>
> + /// Check if the scalar \p Value is within the valid range for the given<br>
> + /// type \p Ty.<br>
> + ///<br>
> + /// Returns true if a check is needed (even if the range is unknown).<br>
> + bool EmitScalarRangeCheck(llvm::<wbr>Value *Value, QualType Ty,<br>
> + SourceLocation Loc);<br>
> +<br>
> /// EmitLoadOfScalar - Load a scalar value from an address, taking<br>
> /// care to appropriately convert from the memory representation to<br>
> /// the LLVM value representation.<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br></div>