[PATCH] [10/10] ELF/Aarch64: Add overflow checks for relocation writes

Adhemerval Zanella adhemerval.zanella at linaro.org
Tue Apr 7 12:35:50 PDT 2015


Hi, 

Thanks for the review.


On 07-04-2015 13:38, Rui Ueyama wrote:
> ================
> Comment at: lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp:161
> @@ -159,1 +160,3 @@
> +                                   int64_t A) {
>    int32_t result = (int32_t)((S + A) - P);
> +  if (!isInt<27>(result))
> ----------------
> Does the result of S+A-P always fit within int32_t? If not, upper bits are cleared by converting an int64_t result into int32_t, which would result in false test pass in the following isInt<27>() check. Maybe we want to always use int64_t types here to avoid confusion and possible errors.

Good question, I assume it will not since current code it already 
casting to int32_t.  Anyway, I think using int64_t and checking
for overflow is a better approach indeed.

>
> ================
> Comment at: lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp:180
> @@ -174,1 +179,3 @@
> +  if (!isInt<20>(result))
> +    return make_out_of_range_reloc_error();
>    result &= 0x01FFFFC;
> ----------------
> The same comment applies here.
>
> ================
> Comment at: lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp:356
> @@ -342,1 +355,3 @@
> +                                                           int64_t A) {
>    int32_t result = S + A;
> +  if (!isUInt<24>(result))
> ----------------
> Ditto
>
> http://reviews.llvm.org/D8870
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>




More information about the llvm-commits mailing list