[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