lebedev.ri added inline comments.

================
Comment at: cfe/trunk/utils/creduce-clang-crash.py:185
+    for msg in self.expected_output:
+      output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+
----------------
akhuang wrote:
> lebedev.ri wrote:
> > akhuang wrote:
> > > lebedev.ri wrote:
> > > > >>! In D59725#1477362, @arichardson wrote:
> > > > >>>! In D59725#1477042, @lebedev.ri wrote:
> > > > >> I've stumbled into an issue with the script:
> > > > >> It got a line: `clang: 
> > > > >> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
> > > > >> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
> > > > >> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run 
> > > > >> indvars!"' failed.`
> > > > >> And produced the following grep: `grep 
> > > > >> 'L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run 
> > > > >> indvars!"' t.log || exit 1`
> > > > >> But that doesn't work, escapes should be applied. it should be
> > > > >> `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to 
> > > > >> run indvars!\"' t.log || exit 1`
> > > > > 
> > > > > In my script I use `grep -F " + shlex.quote(self.args.crash_message)` 
> > > > > which should escape both the message and ensure that the grep 
> > > > > argument is not interpreted as a regex.
> > > > 
> > > > Great to hear!
> > > > It appears that the script is currently more primitive than that 
> > > > currently though.
> > > Thanks; I added a `-F` flag (r359216), and it seems like that fixes the 
> > > issue. 
> > Thanks!
> > What about `shlex`? How do you know bash won't expand that as regex already?
> Sorry, not sure what you mean by bash expanding `shlex` as regex? It should 
> be printing a quoted string to the bash script. 
```
$ ls konsole*
'konsole*'   konsole-ACNHfh.history   konsole-CVnmsn.history   
konsole-HhEJRA.history
$ echo "konsole\*" | grep -F "konsole*"
$ echo "konsole\*" | grep -F "konsole\*"
konsole\*
```

The regex still needs to be escaped, regardless of `-F`.
Otherwise bash will expand it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to