woruyu wrote:
> Reverting the changes because these failures look related to the patch. I
> _think_ we're missing a call to `initSanitizers()` somewhere, possibly in
> `SanitizerSpecialCaseList::create()`. I don't have the bandwidth to try to
> debug it now, hence the revert.
I took a closer look at the build logs and I suspect the issue might be
related to this code:
```
// in ASTUnit.cpp
if (ToLoad >= LoadASTOnly)
AST->Ctx = new ASTContext(*AST->LangOpts, AST->getSourceManager(),
PP.getIdentifierTable(), PP.getSelectorTable(),
PP.getBuiltinInfo(),
AST->getTranslationUnitKind());
```
The failing cases seem to be triggered during C++20 module builds, and from
what I can tell, reproducing them locally is quite difficult. That said, this
made me reconsider the core design of the merged patch.
Currently, the code assumes that `ASTContext::initSanitizers()` will always be
called after constructing the context. However, this assumption introduces a
latent risk—if the caller forgets to call `initSanitizers()`, we can end up
with a `std::unique_ptr` being null at runtime. In my view, this is not a
particularly robust design.
To make this more resilient, I believe it would be safer to revert to the
previous approach, where construction and validation are combined via something
like:
```
std::unique_ptr<SanitizerSpecialCaseList>
SanitizerSpecialCaseList::createOrDie(...) {
...
if (auto SSCL = create(...))
return SSCL;
llvm::remove_fatal_error_handler();
llvm::report_fatal_error(StringRef(Error), false);
}
```
This way, we can ensure that a valid object is always returned, and we can
avoid unexpected crashes (or stack dumps) due to misuse. To me, this seems like
a safer and more maintainable direction.
https://github.com/llvm/llvm-project/pull/147959
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
