[PATCH] D139686: [lsan] Add lsan support for loongarch64

[Youling Tang via Phabricator via cfe-commits]

tangyouling added inline comments.


================
Comment at: compiler-rt/lib/lsan/lsan_common.cpp:281
+#  elif defined(__loongarch_lp64)
+  return ((p >> 47) == 0);
 #  else
----------------
xen0n wrote:
> tangyouling wrote:
> > xen0n wrote:
> > > Since our VM layout is actually flexible, would it be better to document this, like "Support only the most common VM layout on LoongArch that allows 47 bits of user-space VMA"? Exact wording could be optimized, I'm only describing the gist here.
> > > Since our VM layout is actually flexible, would it be better to document this, like "Support only the most common VM layout on LoongArch that allows 47 bits of user-space VMA"? Exact wording could be optimized, I'm only describing the gist here.
> > 
> > How about "Allow 47-bit user-space VMA at current"?
> "at present"; if you plan to revisit this later for properly supporting variable VM layouts, omitting the justification is fine to me.
Support for variable VM layout has not been planned for some time.


================
Comment at: compiler-rt/test/asan/TestCases/Linux/leak_check_segv.cpp:4
 // REQUIRES: leak-detection
-#include <stdlib.h>
+#include <sanitizer/lsan_interface.h>
 #include <stdio.h>
----------------
xen0n wrote:
> tangyouling wrote:
> > xen0n wrote:
> > > Why unnecessarily reorder things, especially putting this //in front of// the standard library includes?
> > > Why unnecessarily reorder things, especially putting this //in front of// the standard library includes?
> > 
> > An automatic adjustment of the format.
> So you mean you're actually unable to avoid this diff damage?
I will try to avoid this unnecessary auto-formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139686