aaron.ballman added a comment.

In D118052#3401861 <https://reviews.llvm.org/D118052#3401861>, @joaomoreira 
wrote:

> I did track down the problem to clang/lib/Frontend/CompilerInvocation.cpp -- 
> RoundTrip method. There, we can se the following statement:
>
>   #ifndef NDEBUG
>     bool DoRoundTripDefault = true;
>   #else
>     bool DoRoundTripDefault = false;
>   #endif
>
> Comment in the file says: "By default, the round-trip is enabled in assert 
> builds... During round-trip, the command line arguments are parsed into a 
> dummy instance of CompilerInvocation which is used to generate the command 
> line arguments again. The real CompilerInvocation instance is then created by 
> parsing the generated arguments, not the original ones.". Then, because the 
> arguments set through this latest patch were not being generated, they would 
> be missing in the dummy instance of CompilerInvocation and then not repassed 
> into the real instance.
>
> Given the above, my understanding is that there was never an actual bug that 
> made ibt-seal stop working. The patch was just not tested enough and a 
> confusion in different build setups blinded me against what was really 
> happening.
>
> Thank you for pushing me into looking into this @aaron.ballman. Also, with 
> all this said, I'm no expert in clang/front-end, so it would be great if you 
> could give an additional look into it and confirm the above.
>
> If all is right, the patch remains correct and there are no hidden bugs. I'll 
> update the patch shortly to add the test requested by @pengfei.

That sounds like the correct analysis to me! Thanks for tracking that down.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118052

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

Reply via email to