[PATCH] D71967: [opt] Fix run-twice crash and detection problem

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 29 01:18:34 PST 2019


MaskRay added a comment.

In D71967#1798108 <https://reviews.llvm.org/D71967#1798108>, @Kokan wrote:

> In D71967#1798089 <https://reviews.llvm.org/D71967#1798089>, @xbolva00 wrote:
>
> > Tests?
>
>
> Hello, @xbolva00
>
> Thanks for checkout out my patch, and sorry for the lack of test or explanation beforehand.
>
> Regarding the tests I seek some support. I was thinking about the following test scenarios, but failed to find any of the proper here:
>
> 1. unittests: With proper refactoring of the `opt` that should be doable, although that would be out of the scope of this current fix.
> 2. test-suit[1]: The issue only happens when the output file is a TTY, a bitcode format is requested and `-run-twice` is used. I could not trigger the failure with llvm-lit test. Please advice.
> 3. Creating a negative test for `-run-twice`, as every Pass should behave the same way after executing multiple time, there is not test that confirms that `-run-twice` actually could catch different results (just like it does not work now with bitcode format), there could be a dummy Pass created that in fact does reproduce different results with each execution in order to test this.
>
>   Could you or anyone please help with it ?
>
> [1] I am referring to those tests that are executed via llvm-lit


Due to

  if (!Force && !NoOutput && !AnalyzeOnly && !OutputAssembly)
    if (CheckBitcodeOutputToConsole(Out->os(), !Quiet))
      NoOutput = true;

This is difficult to test. The bug relies on stdout being a pty.

I think it may be worth defining a variable for `!NoOutput && !AnalyzeOnly`, because the pattern occurs multiple times.


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

https://reviews.llvm.org/D71967





More information about the llvm-commits mailing list