[PATCH] D131341: [RISC-V][HWASAN] Add tag mismatch routines for HWASAN required for RISC-V

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: compiler-rt/lib/hwasan/hwasan_setjmp_riscv64.S:47
+  CFI_STARTPROC
+  sd    ra,   0<<3(x10)
+  sd    s0,   1<<3(x10)
----------------
The size of jmp_buf, and the order of fields within it, is implementation-specific. Moreover, the latter is not part of the ABI, it can change AFAIK. Since longjmp gets intercepted too this is just about ok (though you haven't modified InternalLongjmp, and I don't get why that's implemented in C, that seems dodgy, then again so does this whole design that you're adding RISC-V support to), though technically you could end up storing to padding bytes in jmp_buf here that don't get copied if someone copies a jmp_buf (real-world code does copy jmp_buf's; tcsh is one example, speaking from experience of having broken jmp_buf copying).


================
Comment at: compiler-rt/lib/hwasan/hwasan_setjmp_riscv64.S:81
+  // This function is defined in hwasan_interceptors.cc
+  j    __sigjmp_save
+  CFI_ENDPROC
----------------
Tail calls must use `tail` not `j` otherwise you run into issues once the object this is linked into gets too big as JAL only has a 20-bit immediate (the exception is for jumping to functions within the same section in the same file, as those end up in a single input section and can't be split up so will be near each other in the output)


================
Comment at: compiler-rt/lib/hwasan/hwasan_setjmp_riscv64.S:99
+
+GNU_PROPERTY_BTI_PAC
----------------
This is an AArch64-ism (BTI and PAC are AArch64 extension names) so doesn't belong in RISC-V-specific code (though does *work* because it's #define'd to nothing on non-AArch64 so it can be used in architecture-independent assembly files, currently just the vfork interceptor wrapper).


================
Comment at: compiler-rt/lib/hwasan/hwasan_tag_mismatch_riscv64.S:69
+.global __hwasan_tag_mismatch_v2
+.type __hwasan_tag_mismatch_v2, %function
+__hwasan_tag_mismatch_v2:
----------------
ASM_TYPE_FUNCTION?


================
Comment at: compiler-rt/lib/hwasan/hwasan_tag_mismatch_riscv64.S:124
+
+  call __hwasan_tag_mismatch4
+  CFI_ENDPROC
----------------
call or tail? Bit weird to have a call at the end of a function, returning to it isn't useful and will fall off the end, though the AArch64 code does use bl not b so the equivalent is call...


================
Comment at: compiler-rt/lib/hwasan/hwasan_tag_mismatch_riscv64.S:127-128
+
+.Lfunc_end0:
+  .size __hwasan_tag_mismatch_v2, .Lfunc_end0-__hwasan_tag_mismatch_v2
+
----------------
ASM_SIZE?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131341