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

[Dan Liew via Phabricator via llvm-commits]

delcypher marked an inline comment as done.
delcypher added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp:109
+  // `new_env_entry` should be `=`.
+  uptr CopyEnv(char **current_env, const char *new_env_entry) {
+    uptr new_entry_index = 0;
----------------
kubamracek wrote:
> I would like this to be done with a sequence of call for helper functions: 1) Copy, 2) AddOrReplace. Please separate the symbolizer-specific logic (printing the warning, using `custom_env_` as the destination) from the env manipulation logic.
To make that work `AddOrReplace` would **also** need to be given the storage for the entry being added (as well as the storage for the copy of the environment array) because AtosSymbolizerProcess owns the storage (`atos_mach_port_env_entry_`).

These helper functions you are proposing will be extremely cumbersome to use (due to needing to pass the storage) which makes we wonder if they will **ever be re-used** elsewhere in the code. Do you foresee places in the code where we would want to use them? If the answer is no then I think refactoring now would be premature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77706