[PATCH] D86170: PrintStackTrace: don't symbolize if LLVM_DISABLE_SYMBOLIZATION is set

Mon Aug 24 19:05:32 PDT 2020

zequanwu added inline comments.

================
Comment at: llvm/test/tools/not/disable-symbolization.test:1
+# RUN: not --crash env > %t || true
+# RUN: FileCheck %s < %t
----------------
MaskRay wrote:
> MaskRay wrote:
> > aganea wrote:
> > > MaskRay wrote:
> > > > zequanwu wrote:
> > > > > aganea wrote:
> > > > > > zequanwu wrote:
> > > > > > > MaskRay wrote:
> > > > > > > > aganea wrote:
> > > > > > > > > zequanwu wrote:
> > > > > > > > > > aganea wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > MaskRay wrote:
> > > > > > > > > > > > > aganea wrote:
> > > > > > > > > > > > > > This test also fails on Windows - I think lit doesn't like `env` alone.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > ******************** TEST 'LLVM :: tools/not/disable-symbolization.test' FAILED ********************
> > > > > > > > > > > > > > Script:
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > : 'RUN: at line 1';   not --crash env > D:\llvm-project\buildninjaRel\test\tools\not\Output\disable-symbolization.test.tmp || true
> > > > > > > > > > > > > > : 'RUN: at line 2';   d:\llvm-project\buildninjarel\bin\filecheck.exe D:\llvm-project\llvm\test\tools\not\disable-symbolization.test < D:\llvm-project\buildninjaRel\test\tools\not\Output\disable-symbolization.test.tmp
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > Exit Code: 127
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Command Output (stdout):
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > $ ":" "RUN: at line 1"
> > > > > > > > > > > > > > $ "not" "--crash" "env"
> > > > > > > > > > > > > > # command stderr:
> > > > > > > > > > > > > > Error: 'env' requires a subcommand
> > > > > > > > > > > > > > error: command failed with exit status: 127
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > `printenv` instead?
> > > > > > > > > > > > > Thanks! `printenv` is not specified by POSIX, so I did not use it. But I just checked: `printenv` appeared in 3.0BSD & Solaris has it as well, so the portability is good.
> > > > > > > > > > > > Any ideas if this'll work on Windows?
> > > > > > > > > > > @dblaikie It does work, since a Unix-like shell is required anyway to run the lit tests on Windows, either MinGW, CygWin or GnuWin32 on the top of the regular cmd shell.
> > > > > > > > > > Hi, this test failed on Windows
> > > > > > > > > > ```
> > > > > > > > > > ******************** TEST 'LLVM :: tools/not/disable-symbolization.test' FAILED ********************
> > > > > > > > > >  Script:
> > > > > > > > > >  --
> > > > > > > > > >  : 'RUN: at line 1';   not --crash printenv > C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\test\tools\not\Output\disable-symbolization.test.tmp || true
> > > > > > > > > >  : 'RUN: at line 2';   c:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\bin\filecheck.exe C:\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\test\tools\not\disable-symbolization.test < C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\test\tools\not\Output\disable-symbolization.test.tmp
> > > > > > > > > >  --
> > > > > > > > > >  Exit Code: 2
> > > > > > > > > >  
> > > > > > > > > >  Command Output (stdout):
> > > > > > > > > >  --
> > > > > > > > > >  $ ":" "RUN: at line 1"
> > > > > > > > > >  $ "not" "--crash" "printenv"
> > > > > > > > > >  # command stderr:
> > > > > > > > > >  error: unable to find `printenv' in PATH: no such file or directory
> > > > > > > > > > ```
> > > > > > > > > @zequanwu Are you running from a cmd shell or a Unix-like shell? Do you have GnuWin32 installed and in PATH?
> > > > > > > > > 
> > > > > > > > > This is what I see from cmd.exe:
> > > > > > > > > ```
> > > > > > > > > D:\llvm-project>which printenv
> > > > > > > > > C:\Users\aganea\Downloads\GetGnuWin32\gnuwin32\bin\printenv.EXE
> > > > > > > > > ```
> > > > > > > > > From a MinGW shell:
> > > > > > > > > ```
> > > > > > > > > $ which printenv
> > > > > > > > > /usr/bin/printenv
> > > > > > > > > ```
> > > > > > > > @aganea If printenv is not always available, I'd like to use
> > > > > > > > 
> > > > > > > > `UNSUPPORTED: system-windows` 
> > > > > > > > 
> > > > > > > > and switch back to `env` (POSIX).
> > > > > > > It's a building bot and running from cmd.exe
> > > > > > > ```
> > > > > > > cmd.exe /C "cd /D C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap && C:\b\s\w\ir\cipd_bin_packages\cpython3\bin\python3.exe C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/./bin/llvm-lit.py -sv --param USE_Z3_SOLVER=0 C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/projects/compiler-rt/test/profile/Profile-x86_64 C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/tools/clang/test C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/tools/lld/test C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/utils/lit C:/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/test"
> > > > > > >  ninja: build stopped: subcommand failed.
> > > > > > > ```
> > > > > > @MaskRay Ok, but I'd like to understand what is going on. [[ https://llvm.org/docs/GettingStartedVS.html#software | This page ]] mandates having GnuWin32 in PATH, and GnuWin32 hasn't been updated since 2010, so `printenv` should be there,
> > > > > > @zequanwu Is it a public bot? Would you have a link to the logs?
> > > > > Yes. https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8871047405536941408/+/steps/package_clang/0/stdout?format=raw
> > > > Sent D86496
> > > > 
> > > > I'd want to use 'env' instead of 'printenv' as well, since the former is specified by POSIX.
> > > It looks like the issue @zequanwu was seeing is specific to how the Chromium build environment is set up, and should not occur on users' machines. The first thing that it does, is to download & install this package `gnuwin-13.zip` which is a largely slimmed down version of GnuWin32. See: https://chromium.googlesource.com/chromium/src/+/refs/heads/master/tools/clang/scripts/build.py#262
> > > I suppose this zip archive was created empirically. If we wanted to re-enable this test on Windows, someone in charge with gnuwin-XX.zip (@hans ?) could add `printenv.exe` in the archive and that would solve the issue.
> > Thanks for investigating this issue!
> > 
> > Meanwhile, I've committed D86496. I was surprised that GnuWin32 env does not implement the POSIX specified behavior
> > 
> > > If no utility operand is specified, the resulting environment shall be written to the standard output, with one name= value pair per line.
> ```
> ******************** TEST 'LLVM :: tools/not/disable-symbolization.test' FAILED ********************
> Script:
> --
> : 'RUN: at line 2';   not --crash env > c:\b\slave\clang-x64-windows-msvc\build\stage1\test\tools\not\Output\disable-symbolization.test.tmp || true
> : 'RUN: at line 3';   cmp -s c:\b\slave\clang-x64-windows-msvc\build\stage1\test\tools\not\Output\disable-symbolization.test.tmp /dev/null || c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe c:\b\slave\clang-x64-windows-msvc\llvm-project\llvm\test\tools\not\disable-symbolization.test < c:\b\slave\clang-x64-windows-msvc\build\stage1\test\tools\not\Output\disable-symbolization.test.tmp
> --
> Exit Code: 127
> 
> Command Output (stdout):
> --
> $ ":" "RUN: at line 2"
> $ "not" "--crash" "env"
> # command stderr:
> Error: 'env' requires a subcommand
> error: command failed with exit status: 127
> ```
> 
> OK, I will not waste more time on the problem as I don't have Windows to test. Hope @aganea can probably address the problem.
Thanks for looking into it. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86170