[cfe-commits] [Review] [ubsan] Fix type reported in compound assignment operations

Richard Smith richard at metafoo.co.uk
Mon Jan 7 14:12:09 PST 2013


Hey, sorry for the delay. LGTM.

On Mon, Jan 7, 2013 at 1:43 PM, Will Dietz <wdietz2 at uiuc.edu> wrote:
> Ping :).
>
> Updated patches for ToT attached.
>
> ~Will
>
> On Sun, Dec 30, 2012 at 4:13 PM, Will Dietz <wdietz2 at uiuc.edu> wrote:
>> 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 <richard at 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 <wdietz2 at 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 at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>



More information about the cfe-commits mailing list