[PATCH] D157210: [symbolizer] Change reaction on invalid input

Tue Aug 22 23:57:51 PDT 2023

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

I took a break away from this for a few days, and decided that I shouldn't block this patch based on the behaviour change. However, I do think the specific case that can now hang (i.e. the case that caused the test to fail) should be highlighted in the release note, so that users can have a hint as to why their script started hanging after they updated to the latest llvm-symbolizer. LGTM with that.

================
Comment at: llvm/docs/ReleaseNotes.rst:152
+* llvm-symbolizer now treats invalid input as an address for which source
+  information is not found.
+
----------------
It might be worth noting explicitly the change in output behaviour for invalid behaviour here, namely that none is produced on stdout. Specifically, I want users to at least be aware that the llvm-symbolizer change could introduce a hang in scripts that were potentially poorly written ebfore.

================
Comment at: llvm/test/tools/llvm-symbolizer/debuginfod.test:42
 RUN: env DEBUGINFOD_CACHE_PATH=%t llvm-symbolizer \
-RUN:   "BUILDID:BUILDID:127da749021c1fc1a58cba734a1f542cbe2b7ce4 0x40054d" | \
+RUN:   "BUILDID:BUILDID:127da749021c1fc1a58cba734a1f542cbe2b7ce4 0x40054d" 2>&1| \
 RUN:   FileCheck %s --check-prefix=BUILDIDBUILDID
----------------

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157210