[PATCH] D68164: [Commandline] Clear error on raw_ostream when there is parsing error.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 30 22:44:14 PDT 2019


ruiu added a comment.

In D68164#1689162 <https://reviews.llvm.org/D68164#1689162>, @ychen wrote:

> In D68164#1689155 <https://reviews.llvm.org/D68164#1689155>, @MaskRay wrote:
>
> > > The exit is called twice and somehow the test exit cleanly.
> >
> > The current behavior is undefined. POSIX says "If exit() is called more than once, the behavior is undefined."  Changing exit to abort in D67847 <https://reviews.llvm.org/D67847> will fix the issue. Thanks.
> >
> > The test `; RUN: sh -c 'opt --reject-this-option 2>&-; echo $?; opt -o /dev/null /dev/null 2>&-; echo $?;'` is testing an undefined behavior, but I can see `2>&-` is a simple alternative to a full-fledged test that makes write operations fail with some other errno. My feeling is that you can just update close-stderr.ll to (`not --crash opt --reject-this-option 2>&1`) in D67847 <https://reviews.llvm.org/D67847> and be prepared to update it if build bots on some platforms have different behaviors.
>
>
> sorry, I'm really confused now. Based on the comment in close-stderr.ll, the test is to make sure `abort` does not happen? Why is it checking the undefined behavior?


I think the current implementation is wrong in the sense that it calls exit more than once, but it happens to work just fine on most platforms, so the test is not failing at the moment. But in theory the test could hang instead of existing with 1, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68164





More information about the llvm-commits mailing list