[PATCH] D69549: [Symbolizers] On Darwin compute function offset when possible.

[Dan Liew via Phabricator via llvm-commits]

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


================
Comment at: compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp:9
+// RUN: FileCheck -input-file=%t.output %s
+// RUN: FileCheck -check-prefix=BADADDR -input-file=%t.output %s
+#include <sanitizer/common_interface_defs.h>
----------------
delcypher wrote:
> yln wrote:
> > delcypher wrote:
> > > delcypher wrote:
> > > > yln wrote:
> > > > > Could this be replaced with `--implicit-check-not='function_offset:0x0` ?
> > > > > Maybe also add `--implicit-check-not='function_offset:0xF`.
> > > > > Also in above test.
> > > > Maybe. I've never used this feature. I had to do the check as two separate FileCheck invocations because otherwise FileCheck failed to detect `function_offset:0x0`.
> > > I tried using `--implicit-check-not=`. It doesn't work for this use case. This flag adds implicit CHECK-NOT in between patterns, it won't apply to lines that match the CHECK patterns. The existing patterns are the same lines we want to apply CHECK-NOT on but that doesn't work here. Initially I thought it was working but that's because the `CHECK-NOT` was firing on the `__sanitizer_print_stack_trace` on the top of the stacktrace.
> > > 
> > Ah, got it, this is the explicit negative case.  Could we make the regex for the positive case smarter so it doesn't match `0x0`?  This would remove the need for the separate `BADADDR` FileCheck run and the temporary output file, etc.
> Regexes are good for explicitly stating what you want to match. They are (IMHO) terrible if you want to explicitly not match a pattern unless the regex language variant has explicit support for it (e.g. `(?!...)` in Python's re module). LLVM's regex implementation is "POSIX extended regular expression (ERE)" which I don't think has `(?!...)`. We could probably come up with a complicated regex that does what we want but it would be difficult to understand.
> 
> So let's just go with what we have here. The approach here is **a lot easier to understand** than a complicated regex.
@yln Just be clear. If you have a suggested regex then I'll consider it but I don't see how to write a comprehensible regex that does what we want it to do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69549