[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

[Sylvain Audi via Phabricator via cfe-commits]

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