r296374 - [ubsan] Factor out logic to emit a range check. NFC.

Dmitry Vyukov via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 27 13:08:42 PST 2017


Ivan, you are listed as admin of
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf
Please issue a clobber for it.

If it does not help, the bot runs the following script:
https://github.com/llvm-mirror/zorg/blob/master/zorg/buildbot/builders/sanitizers/buildbot_standard.sh
It builds clang with clang, but the first clang is just a normal
clang, it's not build tsan or anything as far as I see.


On Mon, Feb 27, 2017 at 10:00 PM, Vedant Kumar <vsk at apple.com> wrote:
> Hm, still can't reproduce this with a normal stage2-RA build of clang+compiler_rt.
>
> And it looks like the next build on the job has the same issue:
>
>   http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/5468/steps/annotate/logs/stdio
>
> Do you think there could be stale build products on the machine? Could we try to run a 'make clean' in the top-level build dir?
>
> vedant
>
>> On Feb 27, 2017, at 12:51 PM, Dmitry Vyukov <dvyukov at google.com> wrote:
>>
>> Does not seem to be related to tsan. It's just that somebody called a
>> directory with compiler tsan_release_build, but that seems to be the
>> only relation to tsan. Otherwise looks like a violated assertion in
>> clang.
>>
>>
>> On Mon, Feb 27, 2017 at 9:46 PM, Ivan Krasin <krasin at google.com> wrote:
>>> Hi Vedant,
>>>
>>> not on top of my head. Dmitriy, can you please take a look?
>>>
>>> krasin
>>>
>>> On Mon, Feb 27, 2017 at 12:43 PM, Vedant Kumar <vsk at apple.com> wrote:
>>>>
>>>> Hi Ivan,
>>>>
>>>> I saw a bot failure on your job after this commit:
>>>>
>>>>
>>>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/5467/steps/tsan%20analyze/logs/stdio
>>>>
>>>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/5467/steps/build%20release%20tsan%20with%20clang/logs/stdio
>>>>
>>>> However, I cannot reproduce it locally with a stage2 TSAN build.
>>>>
>>>> After staring at my diff I couldn't find anything that would explain the
>>>> failure. No other bots seem upset.
>>>>
>>>> Do you have any idea about what's going on? Let me know if you want me to
>>>> revert...
>>>>
>>>> vedant
>>>>
>>>>> On Feb 27, 2017, at 11:46 AM, Vedant Kumar via cfe-commits
>>>>> <cfe-commits at lists.llvm.org> wrote:
>>>>>
>>>>> Author: vedantk
>>>>> Date: Mon Feb 27 13:46:19 2017
>>>>> New Revision: 296374
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=296374&view=rev
>>>>> Log:
>>>>> [ubsan] Factor out logic to emit a range check. NFC.
>>>>>
>>>>> This is a readability improvement, but it will also help prep an
>>>>> upcoming patch to detect UB loads from bitfields.
>>>>>
>>>>> Modified:
>>>>>   cfe/trunk/lib/CodeGen/CGExpr.cpp
>>>>>   cfe/trunk/lib/CodeGen/CodeGenFunction.h
>>>>>
>>>>> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=296374&r1=296373&r2=296374&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
>>>>> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Feb 27 13:46:19 2017
>>>>> @@ -1301,6 +1301,46 @@ llvm::MDNode *CodeGenFunction::getRangeF
>>>>>  return MDHelper.createRange(Min, End);
>>>>> }
>>>>>
>>>>> +bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType
>>>>> Ty,
>>>>> +                                           SourceLocation Loc) {
>>>>> +  bool HasBoolCheck = SanOpts.has(SanitizerKind::Bool);
>>>>> +  bool HasEnumCheck = SanOpts.has(SanitizerKind::Enum);
>>>>> +  if (!HasBoolCheck && !HasEnumCheck)
>>>>> +    return false;
>>>>> +
>>>>> +  bool IsBool = hasBooleanRepresentation(Ty) ||
>>>>> +                NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
>>>>> +  bool NeedsBoolCheck = HasBoolCheck && IsBool;
>>>>> +  bool NeedsEnumCheck = HasEnumCheck && Ty->getAs<EnumType>();
>>>>> +  if (!NeedsBoolCheck && !NeedsEnumCheck)
>>>>> +    return false;
>>>>> +
>>>>> +  llvm::APInt Min, End;
>>>>> +  if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true,
>>>>> IsBool))
>>>>> +    return true;
>>>>> +
>>>>> +  SanitizerScope SanScope(this);
>>>>> +  llvm::Value *Check;
>>>>> +  --End;
>>>>> +  if (!Min) {
>>>>> +    Check = Builder.CreateICmpULE(
>>>>> +        Value, llvm::ConstantInt::get(getLLVMContext(), End));
>>>>> +  } else {
>>>>> +    llvm::Value *Upper = Builder.CreateICmpSLE(
>>>>> +        Value, llvm::ConstantInt::get(getLLVMContext(), End));
>>>>> +    llvm::Value *Lower = Builder.CreateICmpSGE(
>>>>> +        Value, llvm::ConstantInt::get(getLLVMContext(), Min));
>>>>> +    Check = Builder.CreateAnd(Upper, Lower);
>>>>> +  }
>>>>> +  llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(Loc),
>>>>> +                                  EmitCheckTypeDescriptor(Ty)};
>>>>> +  SanitizerMask Kind =
>>>>> +      NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool;
>>>>> +  EmitCheck(std::make_pair(Check, Kind),
>>>>> SanitizerHandler::LoadInvalidValue,
>>>>> +            StaticArgs, EmitCheckValue(Value));
>>>>> +  return true;
>>>>> +}
>>>>> +
>>>>> llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool
>>>>> Volatile,
>>>>>                                               QualType Ty,
>>>>>                                               SourceLocation Loc,
>>>>> @@ -1353,35 +1393,9 @@ llvm::Value *CodeGenFunction::EmitLoadOf
>>>>>                                      false /*ConvertTypeToTag*/);
>>>>>  }
>>>>>
>>>>> -  bool IsBool = hasBooleanRepresentation(Ty) ||
>>>>> -                NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
>>>>> -  bool NeedsBoolCheck = SanOpts.has(SanitizerKind::Bool) && IsBool;
>>>>> -  bool NeedsEnumCheck =
>>>>> -      SanOpts.has(SanitizerKind::Enum) && Ty->getAs<EnumType>();
>>>>> -  if (NeedsBoolCheck || NeedsEnumCheck) {
>>>>> -    SanitizerScope SanScope(this);
>>>>> -    llvm::APInt Min, End;
>>>>> -    if (getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true,
>>>>> IsBool)) {
>>>>> -      --End;
>>>>> -      llvm::Value *Check;
>>>>> -      if (!Min)
>>>>> -        Check = Builder.CreateICmpULE(
>>>>> -          Load, llvm::ConstantInt::get(getLLVMContext(), End));
>>>>> -      else {
>>>>> -        llvm::Value *Upper = Builder.CreateICmpSLE(
>>>>> -          Load, llvm::ConstantInt::get(getLLVMContext(), End));
>>>>> -        llvm::Value *Lower = Builder.CreateICmpSGE(
>>>>> -          Load, llvm::ConstantInt::get(getLLVMContext(), Min));
>>>>> -        Check = Builder.CreateAnd(Upper, Lower);
>>>>> -      }
>>>>> -      llvm::Constant *StaticArgs[] = {
>>>>> -        EmitCheckSourceLocation(Loc),
>>>>> -        EmitCheckTypeDescriptor(Ty)
>>>>> -      };
>>>>> -      SanitizerMask Kind = NeedsEnumCheck ? SanitizerKind::Enum :
>>>>> SanitizerKind::Bool;
>>>>> -      EmitCheck(std::make_pair(Check, Kind),
>>>>> SanitizerHandler::LoadInvalidValue,
>>>>> -                StaticArgs, EmitCheckValue(Load));
>>>>> -    }
>>>>> +  if (EmitScalarRangeCheck(Load, Ty, Loc)) {
>>>>> +    // In order to prevent the optimizer from throwing away the check,
>>>>> don't
>>>>> +    // attach range metadata to the load.
>>>>>  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
>>>>>    if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
>>>>>      Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
>>>>>
>>>>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=296374&r1=296373&r2=296374&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
>>>>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Mon Feb 27 13:46:19 2017
>>>>> @@ -2866,6 +2866,13 @@ public:
>>>>>  /// representation to its value representation.
>>>>>  llvm::Value *EmitFromMemory(llvm::Value *Value, QualType Ty);
>>>>>
>>>>> +  /// Check if the scalar \p Value is within the valid range for the
>>>>> given
>>>>> +  /// type \p Ty.
>>>>> +  ///
>>>>> +  /// Returns true if a check is needed (even if the range is unknown).
>>>>> +  bool EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
>>>>> +                            SourceLocation Loc);
>>>>> +
>>>>>  /// EmitLoadOfScalar - Load a scalar value from an address, taking
>>>>>  /// care to appropriately convert from the memory representation to
>>>>>  /// the LLVM value representation.
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>
>


More information about the cfe-commits mailing list