[llvm-commits] [PATCH] Fix undefined behavior in the Mips backend

[Akira Hatanaka]

Also, the type of MaskedImm should be uint64_t in this line:

void MipsAnalyzeImmediate::GetInstSeqLs(int64_t Imm, unsigned RemSize,
                                         InstSeqLs &SeqLs) {
   int64_t MaskedImm = Imm & (((uint64_t)-1) >> (64 - Size));

These should remain int64_t:

void MipsAnalyzeImmediate::ReplaceADDiuSLLWithLUi(InstSeq &Seq) {
  ...
  int64_t Imm = SignExtend64<16>(Seq[0].ImmOpnd);
  int64_t ShiftedImm = Imm << (Seq[1].ImmOpnd - 16);


Thanks for pointing out this problem.

On Fri, Feb 24, 2012 at 11:43 AM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> I think the possibility of undefined behavior remains even after
> applying the patch, since the uint64_t value is converted to int64_t
> when it is passed to GetInstSeqLs. Is that right?
>
> If that is the case, all functions should take "uint64_t Imm" instead
> of "int64_t Imm" as you have suggested.
>
> On Fri, Feb 24, 2012 at 2:30 AM, Ahmed Charles <ahmedcharles at gmail.com> wrote:
>> This patch maintains current behavior while avoiding UB. I'd like
>> someone with more understanding of the Mips backend to suggest whether
>> all of these functions should take a uint64_t or not, since they are
>> just representing 'bits', rather than signed values.
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>