[TSan] Refactor ExternalAccess() to avoid unnecessary pop/push tag [NFC]

[yln]

Summary

• Avoid unnecessary frame & tag push/pops if memory access is ignored
• Rename function and add comment to make it clearer what the code does
• Make helper functions static and move inside #if !SANITIZER_GO

[yln]

This the follow-up I promised here:
https://reviews.llvm.org/D146264#4202953

there is no space the shadow cells [...] for the uptr-sized tag

Is this still true with the new runtime? @dvyukov

The call that actually models the memory access always uses CALLERPC. Is this how it should be? @kubamracek

MemoryAccess(thr, tsan_caller_pc, (uptr)addr, 1, typ);

Here is the original code before my modifications:
https://reviews.llvm.org/D32360

[dvyukov]

Is this still true with the new runtime?

It's even true-er :)
The shadow was shrunk from 8 bytes to 4 bytes.

[yln]

The call that actually models the memory access always uses CALLERPC. Is this how it should be?

Answering my own question: there is a test Darwin/external.cpp checking this, so I am assuming yes.

compiler-rt/lib/tsan/rtl/tsan_external.cpp

const char *GetObjectTypeFromTag(uptr tag) {
   return tag_data ? tag_data->object_type : nullptr;
 }
 
 const char *GetReportHeaderFromTag(uptr tag) {
   TagData *tag_data = GetTagData(tag);
   return tag_data ? tag_data->header : nullptr;
 }
 
-void InsertShadowStackFrameForTag(ThreadState *thr, uptr tag) {
-  FuncEntry(thr, (uptr)&registered_tags[tag]);
-}
-
 uptr TagFromShadowStackFrame(uptr pc) {
   uptr tag_count = atomic_load(&used_tags, memory_order_relaxed);
   void *pc_ptr = (void *)pc;
   if (pc_ptr < GetTagData(0) || pc_ptr > GetTagData(tag_count - 1))
     return 0;
   return (TagData *)pc_ptr - GetTagData(0);
 }
 
 #if !SANITIZER_GO
 
-void ExternalAccess(void *addr, uptr caller_pc, uptr tsan_caller_pc, void *tag,
-                    AccessType typ) {
+// We need to track tags for individual memory accesses, but there is no space
+// in the shadow cells for them.  Instead we push/pop them onto the thread
+// traces and ignore the extra tag frames when printing reports.
+static void PushTag(ThreadState *thr, uptr tag) {
+  FuncEntry(thr, (uptr)&registered_tags[tag]);
+}
+static void PopTag(ThreadState *thr) { FuncExit(thr); }
+
+static void ExternalAccess(void *addr, uptr caller_pc, uptr tsan_caller_pc,
+                           void *tag, AccessType typ) {
   CHECK_LT(tag, atomic_load(&used_tags, memory_order_relaxed));
+  bool in_ignored_lib;
+  if (caller_pc && libignore()->IsIgnored(caller_pc, &in_ignored_lib))
+    return;
+
   ThreadState *thr = cur_thread();
   if (caller_pc) FuncEntry(thr, caller_pc);
-  InsertShadowStackFrameForTag(thr, (uptr)tag);
-  bool in_ignored_lib;
-  if (!caller_pc || !libignore()->IsIgnored(caller_pc, &in_ignored_lib))
-    MemoryAccess(thr, tsan_caller_pc, (uptr)addr, 1, typ);
-  FuncExit(thr);
+  PushTag(thr, (uptr)tag);
+  PopTag(thr);
   if (caller_pc) FuncExit(thr);
 }
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 void *__tsan_external_register_tag(const char *object_type) {
   uptr new_tag = atomic_fetch_add(&used_tags, 1, memory_order_relaxed);
   CHECK_LT(new_tag, kExternalTagMax);