sberg added subscribers: rnk, sberg.
sberg added a comment.
Herald added a project: All.

I started to experience Clang crashing when building Firebird (as part of 
building LibreOffice) in clang-cl mode on Windows, and I think it is due to 
this change in combination with D2733 <https://reviews.llvm.org/D2733> by @rnk:

When `HeaderSearch::LookupFile` exits through one of the `if 
(checkMSVCHeaderSearch...` branches introduced in D2733 
<https://reviews.llvm.org/D2733>, it does not update `CacheLookup.HitIt` 
(through one of the calls to `cacheLookupSuccess` or via the "Otherwise, didn't 
find it" step at the end of the function), even though it already may have done 
the "We will fill in our found location below, so prime the start point value" 
step of calling `CacheLookup.reset`.  That means that a later call to 
`HeaderSearch::LookupFile` can go into the `if (!SkipCache && 
CacheLookup.StartIt == NextIt)` true branch, set `It = CacheLookup.HitIt` to an 
invalid iterator (the default `HitIt = nullptr` value), and then assert/crash 
when dereferencing that invalid `It`.

Which was not an issue before this change, as the initial `HitIdx = 0` was not 
an invalid value, so `i = CacheLookup.HitIdx` never caused `i` to become 
invalid.

I locally worked around this in my build with

  -  if (!SkipCache && CacheLookup.StartIt == NextIt) {
  +  if (!SkipCache && CacheLookup.StartIt == NextIt && CacheLookup.HitIt) {

which seems to work well, but I'm not sure what the best real fix would be.  
(D2733 <https://reviews.llvm.org/D2733> stated "I believe the correct behavior 
here is to avoid updating the cache when we find the header via MSVC's search 
rules.")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119721

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D119721: [clang][... Stephan Bergmann via Phabricator via cfe-commits

Reply via email to