[llvm] [CodeGen] Fix `MachineModuleInfo`'s move constructor to be more safe with `MCContext` ownership. (PR #104834)

weiwei chen via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 19 18:58:07 PDT 2024


weiweichen wrote:

> This function is not used by anyone in the LLVM monorepo AFAIK. I have a related PR (#98770) to this, which exposes the move constructor on the MMIWP side, so that I can inspect its contents after I run passes on it without requiring the PM to stay alive.
> 
> However, as discussions in the PR shows, when I used it in the use case I showed [here](https://github.com/llvm/llvm-project/pull/98770#issue-2407150383), I would get segfaults. I suspected this was because MFs had references to the old, unmoved MMI fields deleted by the PM destructor, which @arsenm has fixed since then. I have not tested the MMIWP constructor since then due to time constraints.
> 
> TLDR; I think there should be a test for this constructor in LLVM proper (something along the lines of my example use case with the PM explicitly deleted should be enough), to prove that it is working correctly in the first place.

Thank you for context and suggestion on adding a test! I agree that a test is good to have, though do you have any suggestions where should it be added to? [Your PR](https://github.com/llvm/llvm-project/pull/98770) doesn't seem to have a real one doing the testing but only code snippet in the comment, and I can't find any in `llvm/test` at all that's remotely testing this sort of thing? 

https://github.com/llvm/llvm-project/pull/104834


More information about the llvm-commits mailing list