vitalybuka added a comment.
Can you please use "Edit related revisions" and link other patches of the
features into the stack?
================
Comment at: llvm/lib/Target/X86/X86InstrCompiler.td:272-274
+ [(int_asan_check_memaccess GR64NoR8:$addr, (i64 timm:$shadowbase),
+ (i32 timm:$iswrite), (i32 timm:$accesssizeindex), (i32 timm:$mappingscale),
+ (i32 timm:$orshadowoffset))]>,
----------------
shadowbase, mappingscale and orshadowoffset are known to runtime, we should not
have them as part of intrinsic
================
Comment at: llvm/lib/Target/X86/X86MCInstLower.cpp:1330
+ if (!TM.getTargetTriple().isOSBinFormatELF()) {
+ report_fatal_error("llvm.hwa`san.check.memaccess only supported on ELF");
+ return;
----------------
This does not look right
================
Comment at: llvm/lib/Target/X86/X86MCInstLower.cpp:1334
+
+ unsigned Reg = MI.getOperand(0).getReg().id();
+ uint64_t ShadowBase = MI.getOperand(1).getImm();
----------------
Why this called reg when it's address?
================
Comment at: llvm/lib/Target/X86/X86MCInstLower.cpp:1340
+ MCSymbol *&Sym = AsanMemaccessSymbols[{
+ Reg, ShadowBase, IsWrite, AccessSizeIndex, MI.getOperand(4).getImm(),
+ MI.getOperand(5).getImm()}];
----------------
Why shadow is an argument? Both runtime and instrumented code know the shadow.
================
Comment at: llvm/lib/Target/X86/X86MCInstLower.cpp:1340-1341
+ MCSymbol *&Sym = AsanMemaccessSymbols[{
+ Reg, ShadowBase, IsWrite, AccessSizeIndex, MI.getOperand(4).getImm(),
+ MI.getOperand(5).getImm()}];
+ if (!Sym) {
----------------
vitalybuka wrote:
> Why shadow is an argument? Both runtime and instrumented code know the shadow.
could you please unpack these to into local variables for consistency?
================
Comment at: llvm/lib/Target/X86/X86MCInstLower.cpp:1369
+ .addReg(X86::R8)
+ .addImm(MappingScale),
+ STI);
----------------
I assumed the goal to move this shadow math into runtime code
so we have less instructions in the instrumented code.
Also MappingScale and OrShadowOffset are constants.
It's a little bit not nice that ShadowBase can be dynamic, so each
__asan_check_ will have to load it,
but let's for now care about fixed shadow which is constant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107850/new/
https://reviews.llvm.org/D107850
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
