[PATCH] D79165: [DebugInfo] - DWARFDebugFrame: do not call abort() on errors.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 13 03:43:54 PDT 2020
grimar marked an inline comment as done.
grimar added inline comments.
================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:419
+ return createStringError(
+ errc::illegal_byte_sequence,
+ "unknown augmentation character in entry at 0x%" PRIx64,
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > Don't use `illegal_byte_sequence` - although it's not clear from the name, this is for Unicode encoding errors, so isn't the right error code for other invalid byte sequences. I've tended to use `invalid_argument` in similar situations.
> > > This file already use `illegal_byte_sequence` (line 50, 65). It is also used widely in LLVM to report invalid non-text data.
> > Just because it's already used widely //incorrectly// doesn't mean we should continue to do so - I'm pretty confident that people are all making the same, perfectly understandable, mistake.
> Where it is said that `illegal_byte_sequence` is about unicode?
>
> MSVS defines it as "illegal_byte_sequence = 42, // EILSEQ" (xerrc.h)
>
> POSIX seems descrives it as "[EILSEQ] Illegal byte sequence."
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
>
> C95 says the same:
> https://en.cppreference.com/w/c/error/errno_macros
>
>
Seems that `EILSEQ` is used by functions like `fputwc` to report errors about wide characters. But I haven't found that it can't be used for other cases. And "Illegal byte sequence" description fits here much better than "Invalid argument".
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79165/new/
https://reviews.llvm.org/D79165
More information about the llvm-commits
mailing list