vitalybuka accepted this revision.
vitalybuka added a reviewer: mcgrathr.
vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/hwasan/hwasan_linux.cpp:430
 
+void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
+  CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse
----------------
mcgrathr wrote:
> vitalybuka wrote:
> > leonardchan wrote:
> > > leonardchan wrote:
> > > > mcgrathr wrote:
> > > > > Most of this code can actually be reused for Fuchsia (just not 
> > > > > necessarily in Thread::Init).
> > > > > It's probably better to split it up for reuse rather than just moving 
> > > > > the whole thing to linux-specific.
> > > > So maybe something like the current patch? Then on Fuchsia we could (1) 
> > > > have a custom implementation of `InitStackAndTls`, (2) wrap the call to 
> > > > `Thread::InitStackRingBuffer` in `Thread::Init` with a `#if 
> > > > SANITIZER_FUCHSIA` so it's not called before thread creation, then (2) 
> > > > call `Thread::InitStackRingBuffer` in the 
> > > > `__sanitizer_thread_start_hook` hook.
> > > Sorry. Meant wrapping with a `#if !SANITIZER_FUCHSIA`.
> > in other sanitizers only GetThreadStackAndTls is platform specific
> > 
> > in other sanitizers only GetThreadStackAndTls is platform specific
> 
> That's not entirely accurate.  AsanThread::SetThreadStackAndTls might be a 
> model here.  But asan on Fuchsia also splits up the initialization into 
> pre-creation and on-created-thread portions (AsanThread::Init is called 
> before thread creation, and AsanThreadRegistry::StartThread is called on the 
> created thread).
> 
Patch itself is LGTM.
But I am not sure if @mcgrathr is asking for particular changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104248

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to