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

[Sagar Thakur via llvm-commits]

sagar added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:943-947
@@ -942,7 +942,7 @@
                        "move $7,%4;\n"
                        /* Store the fifth argument on stack
                         * if we are using 32-bit abi.
                         */
 #if SANITIZER_WORDSIZE == 32
                        "lw %5,16($29);\n"
 #else
----------------
dsanders wrote:
> It's not directly related to this patch but: The comment says 'store' but the instruction is a load from the compiler-generated stack frame.
Yes, thanks for catching that, the instruction should have been a store. None of the 32-bit sanitizers use this code as of now but we will still fix it for the sake of completeness.

================
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"
----------------
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.


http://reviews.llvm.org/D18753