saudi added inline comments.

================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:677
+  SwapOpts(Res);
+  bool Success2 = Parse(Res, GeneratedArgs1, Diags);
+
----------------
Hello,

I encountered crashes on Windows targets, related to this line, when rebasing 
https://reviews.llvm.org/D80833, where `CodeGenOpts::CommandLineArgs` is read 
later in the compilation process.

When the function exits, `GeneratedArgs1` is destroyed but 
`Res.getCodeGenOpts().CommandLineArgs` still references its contents. The code 
has changed since this patch was submitted, but the logic remains the same in 
more recent code.

Note that the bug doesn't happen when round-trip is skipped, as 
`CommandLineArgs` then refers to `ArgV` which content is valid for the entire 
compiler run.

As a solution I considered allowing `CodeGenOptions` to optionally own the 
memory by introducing `BumpPtrAllocator CodeGenOptions::CommandLineArgsAlloc`.
 However it has drawbacks:
 - need to customize `CodeGenOptions` copy constructor/operator,
 - lifetime for copies of `CodeGenOptions::CommandLineArgs` (e.g. to other 
structures) would be bound to the lifetime of the original `CodeGenOptions` 
object.
 
Another solution (slower) would be to use 2 dummy `CompilerInvocation` to 
perform the round-trip test, and use the regular arguments on the main 
instance. It would change the behavior since the main instance currently uses 
`GeneratedArgs1`.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472

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

Reply via email to