[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[joshua-arch1]

Summary

libsanitizer/

    
* sanitizer_common/sanitizer_common.h (ModuleArch): New enumerator.
(ModuleArchToString): New architecture option.
* sanitizer_common/sanitizer_symbolizer_libcdep.cpp (GetArgV): New architecture option.

Up till now, only UndefinedBehaviorSanitizer(UBSan) is functionally supported for RISC-V on GCC. With ASan enabled in this patch, AddressSanitizer(ASan) and KernelAddressSANitizer(KASAN) will be both supported with "-fsanitize=address" added when compiling and running the tests.

[kito-cheng]

Hi Jun:

GCC part should review at gcc-patch mailing list instead of here, for other files (sanitizer_platform.h, sanitizer_common.h and sanitizer_symbolizer_libcdep.cpp), it's right place to review, but it should using the diff which generated within LLVM source-tree, and GCC will sync libsanitizer after LLVM is accepted.

libsanitizer/sanitizer_common/sanitizer_common.h

675 ↗

(On Diff #286483)

Seems like you also need to update asan_symbolize.py according this comment, don't remove that I know it not exist in gcc source tree, but LLVM source tree have that[1].

[1] https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/asan/scripts/asan_symbolize.py

libsanitizer/sanitizer_common/sanitizer_platform.h

221 ↗

(On Diff #286483)

This marco is not necessary to define if you don't use anywhere.

libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp

252 ↗

(On Diff #286483)

Same as above, don't remove this.

[joshua-arch1]

In D86195#2225532, @kito-cheng wrote:

Hi Jun:

GCC part should review at gcc-patch mailing list instead of here, for other files (sanitizer_platform.h, sanitizer_common.h and sanitizer_symbolizer_libcdep.cpp), it's right place to review, but it should using the diff which generated within LLVM source-tree, and GCC will sync libsanitizer after LLVM is accepted.

What about config/riscv/riscv.c? Should I place it here to review?

[kito-cheng]

Hi Jun:

GCC part should review at gcc-patch mailing list instead of here, for other files (sanitizer_platform.h, sanitizer_common.h and sanitizer_symbolizer_libcdep.cpp), it's right place to review, but it should using the diff which generated within LLVM source-tree, and GCC will sync libsanitizer after LLVM is accepted.

What about config/riscv/riscv.c? Should I place it here to review?

For config/riscv/riscv.c, it's part of GCC source tree, so there is wrong place to review :P

[joshua-arch1]

This comment was removed by joshua-arch1.

[zzheng]

can you generate the diff with full context?

git format-patch -U99999

[joshua-arch1]

1. to remove unnecessary definitions
2. to generate the diff with full context
3. to generate the diff on LLVM source tree

[joshua-arch1]

In D86195#2226367, @zzheng wrote:

can you generate the diff with full context?

git format-patch -U99999

Hi I have updated the patch. Now it has full context.

[joshua-arch1]

In D86195#2225591, @kito-cheng wrote:

Hi Jun:

GCC part should review at gcc-patch mailing list instead of here, for other files (sanitizer_platform.h, sanitizer_common.h and sanitizer_symbolizer_libcdep.cpp), it's right place to review, but it should using the diff which generated within LLVM source-tree, and GCC will sync libsanitizer after LLVM is accepted.

What about config/riscv/riscv.c? Should I place it here to review?

For config/riscv/riscv.c, it's part of GCC source tree, so there is wrong place to review :P

I have updated the patch based on LLVM source tree. When will it be reviewed?

[luismarques]

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp

273–274

The preprocessor definition should be __riscv, and the arch riscv32 or riscv64. Given that this is incorrect, I suspect this patch needs much more testing. When you update the patch please update the description with more details of why you think this is in good shape to be merged. Adding more tests would make that easier.

[joshua-arch1]

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp

273–274

The preprocessor definition should be __riscv, and the arch riscv32 or riscv64. Given that this is incorrect, I suspect this patch needs much more testing. When you update the patch please update the description with more details of why you think this is in good shape to be merged. Adding more tests would make that easier.

Hi, I have updated my patch and its summary.

[luismarques]

RISC-V support for ASan has been merged (see D87582 and its predecessors). This patch should probably be abandoned. Thank you.

compiler-rt/lib/sanitizer_common/sanitizer_common.h

enum ModuleArch {
   kModuleArchUnknown,
   kModuleArchI386,
   kModuleArchX86_64,
   kModuleArchX86_64H,
   kModuleArchARMV6,
   kModuleArchARMV7,
   kModuleArchARMV7S,
   kModuleArchARMV7K,
-  kModuleArchARM64
+  kModuleArchARM64,
+  kModuleArchRISCV
 };
 
 // Opens the file 'file_name" and reads up to 'max_len' bytes.
 // The resulting buffer is mmaped and stored in '*buff'.
 // Returns true if file was successfully opened and read.
 bool ReadFileToVector(const char *file_name,
                       InternalMmapVectorNoCtor<char> *buff,
                       uptr max_len = 1 << 26, error_t *errno_p = nullptr);
switch (arch) {
     case kModuleArchARMV7:
       return "armv7";
     case kModuleArchARMV7S:
       return "armv7s";
     case kModuleArchARMV7K:
       return "armv7k";
     case kModuleArchARM64:
       return "arm64";
+    case kModuleArchRISCV:
+      return "riscv";
   }
   CHECK(0 && "Invalid module arch");
   return "";
 }
 
 const uptr kModuleUUIDSize = 16;
 const uptr kMaxSegName = 16;
 

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp

 #elif defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
     const char* const kSymbolizerArch = "--default-arch=powerpc64";
 #elif defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
     const char* const kSymbolizerArch = "--default-arch=powerpc64le";
 #elif defined(__s390x__)
     const char* const kSymbolizerArch = "--default-arch=s390x";
 #elif defined(__s390__)
     const char* const kSymbolizerArch = "--default-arch=s390";
+#elif defined(__riscv)
+    const char* const kSymbolizerArch = "--default-arch=riscv";
 #else
     const char* const kSymbolizerArch = "--default-arch=unknown";
 #endif
 
     const char *const inline_flag = common_flags()->symbolize_inline_frames
                                         ? "--inlines"
                                         : "--no-inlines";
     int i = 0;