[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;
----------------
delcypher wrote:
> 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.
@kubamracek I've come up with an abstraction for buffers that actually makes what you're asking for a lot more elegant and feasible. I'll rework my patches based on this.

```lang=c++
  template <class T>
  struct BufferRef {
    T *data;
    const uptr size; // number of elements of type T in buffer.
    BufferRef() : data(nullptr), size(0) {}
    BufferRef(T *data, uptr size) : data(data), size(size) {}
    T &operator[](uptr idx) {
      CHECK_LT(idx, size);
      return data[idx];
    }
    void copyFrom(BufferRef<T> &src) {
      CHECK_GE(size, src.size);
      internal_memcpy(data, src.data, sizeof(T) * src.size);
    }
  };
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77706