Thanks for the feedback, updated patches attached.  Moved regression
test to clang, and also fix similar issue with "/=" using the wrong
type.

On Sun, Dec 30, 2012 at 2:48 AM, Richard Smith <rich...@metafoo.co.uk> wrote:
> -      StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.E->getType()));
> +      QualType ResTy = Info.E->getType();
> +      if (const CompoundAssignOperator *C =
> +            dyn_cast<CompoundAssignOperator>(Info.E))
> +        ResTy = C->getComputationResultType();
> +      StaticData.push_back(CGF.EmitCheckTypeDescriptor(ResTy));
>
> Could you just use Info.Ty here?

Good call, much better.

>
> Please add a test for Clang's test suite too. The tests in compiler-rt
> are intended to test compiler-rt itself (the formatting of the
> diagnostics, etc), not for testing that Clang produces correct data
> blocks (in particular, we should have sufficient coverage that we can
> refactor Clang's IRGen without running the compiler-rt tests).
>

Understood, thanks for the explanation.  Makes good sense.

> On Sun, Dec 30, 2012 at 12:13 AM, Will Dietz <wdie...@uiuc.edu> wrote:
>> See attached patches, thanks!
>>
>> Description:
>>
>> When checking "a += b" we were using the type of 'a' in the
>> diagnostic, instead of the type of the overflowing expression "a+b".
>> This was particularly problematic when 'a' was signed and 'b' was
>> unsigned.
>>
>> Okay to commit?
>>
>> ~Will
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits@cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>

Attachment: 0001-ubsan-Use-correct-type-for-compound-assignment-ops.patch
Description: Binary data

Attachment: 0001-ubsan-Check-for-appropriate-types-on-compound-assign.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to