Adding a scripted test for PR25717

[ygao]

Summary

Hi,
I am trying to add a test for PR25717. The test itself is fairly straight-forward: it generates a
large .c temporary file on the fly and then runs the preprocessor over the generated file to
make sure we do not see any unexpected fatal errors.
I have never added a scripted test before, so it would be really nice if someone with this
experience could take a look that I did make all the necessary changes.
Many thanks in advance,

• Gao

[ygao]

test/Preprocessor/bigoutput.py

7 ↗

(On Diff #43420)

For example, it would be nice to use %t as the temporary file name (so lit will fill in with a generated
name) instead of a hard-coded name here, but I am trying to force UNIX line ending here. So I could
not figure out how to combine %t with the open() call.

[• rafael]

Can't you use the preprocessor to produce a large output? See the
testcase in https://llvm.org/bugs/show_bug.cgi?id=10651#c10 for
example.

Cheers,
Rafael

[ygao]

Hi Rafael,
Yes I can do that, and it works for me. Do you know whether there is a way to keep the UNIX line ending on the test file upon checkout rather than having svn or git auto-translate the line endings? The UNIX line ending is necessary to trigger the original bug,

• Gao

[ygao]

I think what I can do is to run
svn propset svn:eol-style 'LF'
on the new file, and hopefully the subversion or git client will get the hint and not auto-translate the line endings.

[• rafael]

Are you sure the line ending in a problem? The preprocessor output has
16 lines, but the last one is very long.

In any case, it looks like at least on linux clang produces an output
with '\n' even when the input has CRLF:

$ file test.c
test.c: ASCII text, with CRLF line terminators
$ clang -E test.c > test.i
$ file test.i
test.i: C source, ASCII text, with very long lines

Cheers,
Rafael

[ygao]

If you take a look at

void PrintPreprocessedAction::ExecuteAction() {
      ...
      while (next < end) {
      if (*cur == 0x0D) {  // CR
        if (*next == 0x0A)  // CRLF
          BinaryMode = false;
      ...

The value of this BinaryMode reflects whether the line ending style of the input file
is CRLF or LF. And it is passed all the way down to the constructor of raw_fd_ostream,

std::unique_ptr<llvm::raw_pwrite_stream> CompilerInstance::createOutputFile(...) {
    ...
  if (!Binary || OS->supportsSeeking())
    return std::move(OS);

  auto B = llvm::make_unique<llvm::buffer_ostream>(*OS);
  ...

So I think the line ending style of the input file does affect whether the output is buffered.

[• rafael]

Gosh, that is quite unfortunate. The problems I see are

• The code only work on windows, as opening a file as text on other

systems is not special.

• There is a confusion in the code about binary being "crlf

translation" or "produce a .o". The logic for creating a buffer is
there because the .o writer needs something it can seek on.

What I would suggest is:

• Check that this test fails at least on windows with your patch

reverted. If so, commit it. BTW, don't you need to drop the "|
FileCheck" to cause a crash?

• Pass two flags to createDefaultOutputFile: TranslateCrLf and

NeedsSeek instead of just Binary.

• Stop using F_Text flag and just print the correct line ending so

this works on any system.

The last two can be just a bug report for now :-)

Cheers,
Rafael

[ygao]

What I would suggest is:

Check that this test fails at least on windows with your patch

reverted. If so, commit it. BTW, don't you need to drop the "|
FileCheck" to cause a crash?

Hmm, even with the pipe removed, I still have difficulty reproducing the crash within lit. I was doing:
C:\llvm\build>python ..\llvm\utils\lit\lit.py -v --no-progress-bar --param build_mode=Release tools\clang\test\Preprocessor\macro.c
But the test is passing. And the run line is simply:
// RUN: %clang_cc1 -E -x c %s

And, if I use the RUN line directly on a command-prompt, the test crashes as expected.

[ygao]

In llvm/utils/lit/lit/TestRunner.py line#202 inside function executeShCmd(), the test is spawned by
subprocess.Popen(command, ..., stdout = subprocess.PIPE, stderr = subprocess.PIPE, ...)
And the test passes. I can verify with a small python script doing just a subprocess.Popen.

If the test is spawned with stdout = None instead, the test will crash as expected.

I could not find a way to trick TestRunner into leaving stdout as None. Is there supposed to be
a command-line option to lit? Or maybe a special redirection mark on the RUN line? Maybe a lit
expert can help me here.

[• rafael]

LGTM assuming you can get the test to fail by reverting the fix

[ygao]

Thanks, Rafael!
I did verify that the test would fail without my fix. But what do I need to do to make buildbots turn on the new lit parameter?

[ygao]

The test is commited in rL258898 and rL258902, and it is enabled on buildbot in rL259248.
Thanks!

test/Preprocessor/bigoutput.c

+// RUN: %clang_cc1 -E -x c %s > /dev/tty
+// The original bug requires UNIX line endings to trigger.
+// The original bug triggers only when outputting directly to console.
+// REQUIRES: console
+
+// Make sure clang does not crash during preprocessing
+
+#define M0 extern int x;
+#define M2  M0  M0  M0  M0
+#define M4  M2  M2  M2  M2
+#define M6  M4  M4  M4  M4
+#define M8  M6  M6  M6  M6
+#define M10 M8  M8  M8  M8
+#define M12 M10 M10 M10 M10
+#define M14 M12 M12 M12 M12
+
+M14

test/lit.cfg

 
 # Allow use of an explicit path for gmalloc library.
 # Will default to '/usr/lib/libgmalloc.dylib' if not set.
 gmalloc_path_str = lit_config.params.get('gmalloc_path',
                                          '/usr/lib/libgmalloc.dylib')
 if use_gmalloc:
      config.environment.update({'DYLD_INSERT_LIBRARIES' : gmalloc_path_str})
 
+# Check if we should allow outputs to console.
+run_console_tests = int(lit_config.params.get('enable_console', '0'))
+if run_console_tests != 0:
+  config.available_features.add('console')
+
 lit.util.usePlatformSdkOnDarwin(config, lit_config)