[PATCH] D147652: [symbolizer] Check existence of input file in GNU mode

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D147652#4441851 <https://reviews.llvm.org/D147652#4441851>, @dblaikie wrote:

> In D147652#4439871 <https://reviews.llvm.org/D147652#4439871>, @MaskRay wrote:
>
>> There is a time-of-check-to-time-of-use issue that we can ignore (as we may wait on the addresses),
>
> I had the same concern there (& maybe @jhenderson did too) & claimed to be addressed/not a problem in https://reviews.llvm.org/D147652#4314914 but I didn't quite follow.
>
> What do you mean "as we may wait on the address" - like we could deal with this problem later? Any particular reason to do that, rather than avoiding introducing the time-of-check-to-time-of-use issue in the first place?

Apologies, this might be all caused by me (and I figured given nobody else really pushed against it, others were fine with it). Specifically this comment:

> IMHO, if an explicitly specified input file does not exist (at the time of command-line parsing at least), I could get behind an error message being reported by BOTH llvm-symbolizer and llvm-addr2line during command-line parsing (or more likely, slightly after, but before the business logic of the symbolizer code kicks in). We'd still need to check for file existence as in the current library code, in case the file has been deleted since command-line parsing happened. The early check should just be a file existence check (don't try to open the file), and should exit the program in the event of failure. It might be worth raising this issue on the mailing lists to confirm changing the llvm-symbolizer behaviour too is okay with others. My reasoning for why this should impact both tools is that a user has requested an explicit file, so if it doesn't exist, trying to use it doesn't make sense, but I could see theoretical use cases where the file is created after the opening of the program in interactive mode (so hence the need for discussion).

It was my suggestion to have an early file existence check, which didn't attempt to open the file properly, on the basis that the later code would still detect the issue at the time of use. We could then restructure the code later as @dblaikie had suggested. The early check is not too dissimilar to, I believe, a few instances in LLD, which provide early feedback on input options (also highlighted above). I don't recall why I didn't just go with the opening of the file at this point, rather than later (reading my previous comments, it was implied somewhere that it wasn't straightforward to open the file upfront for some reason).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147652