[PATCH] D78179: [Darwin] Fix symbolization for recent simulator runtimes.

[Dan Liew via Phabricator via llvm-commits]

delcypher marked 2 inline comments as done.
delcypher added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp:68
+  void LateInitialize() {
+    if (SANITIZER_IOSSIM) {
+      // `putenv()` may call malloc/realloc so it is only safe to do this
----------------
yln wrote:
> What's our stance on `if(<defined-value>)` vs `#ifdef`?  In compiler-rt we seem to usually use #ifdef.  Should we aim for consistency here? 
`SANITIZER_IOSSIM` is always defined to be `0` or `1`. I thought it was better than this code be compiled on all paths because not everyone builds for iOS simulators in open source.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp:102
+      // `atos_mach_port_env_var_entry_` variable with our current PID.
+      UpdateAtosEnvVar(pid_str_);
+    }
----------------
yln wrote:
> If you remove the unnecessary call in the constructor, then this is the only usage of the function and we can inline it.
@yln 

> If you remove the unnecessary call in the constructor, then this is the only usage of the function and we can inline it.

The call in the constructor is **essential**. When you call `putenv()` Darwin's LibC checks the data is well formed so we **have to initialize `atos_mach_port_env_var_entry_` before giving it to `putenv()`**.

Later on we need to modify `atos_mach_port_env_var_entry_`  again to have it contain the right PID so it made sense to refactor this repeated action into `UpdateAtosEnvVar()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78179