[PATCH] D145214: [TSAN] add support for riscv64

[Alex Fan via Phabricator via cfe-commits]

alexfanqi added inline comments.


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_platform.h:971
     };
-    const uptr indicator = 0x0e0000000000ull;
+    const uptr indicator = 0x0f0000000000ull;
     const uptr ind_lsb = 1ull << LeastSignificantSetBitIndex(indicator);
----------------
jrtc27 wrote:
> ?
This is changed for sv48. Tsan compresses the address to 44 bits and uses the top 3 bits (42-44) to uncompress it by comparing with the corresponding bits in the mapping. So the 42-44th bits of each app mapping range must be all different from each other.  But 3 bits are not enough to distinguish mappings in this scheme of MappingRiscv64_48, so I increment it to 4 bits. An alternative of this would be reducing app mapping size by half.


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:270-271
+#elif SANITIZER_RISCV64
+  // the bottom half of vma is allocated for userspace
+  vmaSize = vmaSize + 1;
+# if !SANITIZER_GO
----------------
jrtc27 wrote:
> Why don't other architectures need the +1?
I am not too sure about other architectures. Perhaps, because riscv kernel divides vma range further by 2. https://elixir.bootlin.com/linux/latest/source/arch/riscv/include/asm/pgtable.h#L785
By https://docs.kernel.org/riscv/vm-layout.html, botton half is exposed for userspace, top half is for kernel.
The available userspace virtual address size for arm64 with same bit size is exactly double of riscv. For example, 39bit -> 0x4000000000(riscv) vs 0x8000000000(arm64) 



================
Comment at: compiler-rt/test/tsan/test.h:77
+#elif defined(__riscv) && __riscv_xlen == 64
+const int kPCInc = 2;
 #else
----------------
jrtc27 wrote:
> What's this the length of? RVC is optional.
The same length as in GetPreviousInstructionPc. Without it, I got java_race_pc, race_range_pc test failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214