[PATCH] D18753: [mips][sanitizer_common] Don't use `ld` in internal_clone() on 32-bit MIPS.

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 7 05:44:03 PDT 2016


dsanders added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:960-971
@@ -959,4 +959,14 @@
                        /* Call "fn(arg)". */
+#if SANITIZER_WORDSIZE == 32
+#ifdef __BIG_ENDIAN__
+                       "lw $25,4($29);\n"
+                       "lw $4,12($29);\n"
+#else
+                       "lw $25,0($29);\n"
+                       "lw $4,8($29);\n"
+#endif
+#else
                        "ld $25,0($29);\n"
                        "ld $4,8($29);\n"
+#endif
                        "jal $25;\n"
----------------
sagar wrote:
> dsanders wrote:
> > It's not directly related to this patch but I also noticed that some of the load instructions are loading from the compiler-generated stack frame ($29 is $sp). This is likely to be sensitive to compiler changes and the offsets are likely to differ between ABI's.
> Yes. For the 64-bit part the offsets will differ for N32 and N64. We should probably code it like this:
> 
> ```
> #if _MIPS_SIM == _MIPS_SIM_NABI32
>   "lw $25,0($29);\n"
>   "lw $4,4($29);\n"
> #else
>   "ld $25,0($29);\n"
>   "ld $4,8($29);\n"
> #endif
> ```
> Thanks again. If this looks good I will submit a patch for fixing these bugs.
It's also sensitive to compiler version and optimizations at the moment. I think it would be better to let the compiler figure out the address by making the relevant variable (fn?) one of the arguments to the inline assembly statement.


http://reviews.llvm.org/D18753





More information about the llvm-commits mailing list