[PATCH] D83247: [compiler-rt][asan][hwasan] Refactor shadow setup into sanitizer_common (NFCI)

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka accepted this revision.
vitalybuka added a comment.
This revision is now accepted and ready to land.

Thank you! LGTM

Windows should work. For the rest we need to rely on bots.



================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_common.h:135
+uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
+                      uptr &high_mem_end, uptr min_shadow_base_alignment = 0);
+
----------------
tejohnson wrote:
> tejohnson wrote:
> > vitalybuka wrote:
> > > and maybe drop default 0 and make out argument the last one?
> > > 
> > > Also high_mem_end in this interface looks unrelated. Maybe mapped shadow size? Better to land as is and followup later.
> > > 
> > 
> > > Also high_mem_end in this interface looks unrelated. Maybe mapped shadow size? Better to land as is and followup later.
> > 
> > I'm not sure I understand the comment about it being unrelated, or what you are asking me to do here? Do you mean just change the parameter name?
> Made the new out parameter last.
> I'm not sure I understand the comment about it being unrelated, or what you are asking me to do here? Do you mean just change the parameter name?

Looking at function interface, it's about shadow. For reader is likely non-intuitive to understand why there is high_mem_end out parameter.
Pure renaming is not going to help. Maybe out parameter with actually mapped side.
Anyway I am not sure what could be a nicer approach for now. So we can keep it as-is and maybe fix later.

btw. it could be nice to write something into high_mem_end, even just 0, on other platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83247