Vedant Kumar <v...@apple.com> writes: > vsk created this revision. > vsk added a reviewer: bogner. > vsk added a subscriber: cfe-commits. > > When handling 'if' statements, we crash if the condition and the consequent > branch are spanned by a single macro expansion. > > The crash occurs because of a sanity 'reset' in `popRegions()`: if an > expansion > exactly spans an entire region, we set `MostRecentLocation` to the start of > the > expansion (its 'include location'). This ensures we don't `handleFileExit()` > ourselves out of the expansion before we're done processing all of the regions > within it. This is tested in test/CoverageMapping/macro-expressions.c. > > This causes a problem when an expansion spans both the condition and the > consequent branch of an 'if' statement. `MostRecentLocation` is updated to the > start of the 'if' statement in `popRegions()`, so the file for the expansion > isn't exited by the time we're done handling the statement. We then crash with > 'fatal: File exit not handled before popRegions'. > > The fix for this is to detect these kinds of expansions, and conservatively > update `MostRecentLocation` to the end of expansion region containing the > conditional. I've added tests to make sure we don't have the same problem with > other kinds of statements.
LGTM, with a question about generalising a bit below. > rdar://problem/23630316 > > http://reviews.llvm.org/D16934 > > Files: > lib/CodeGen/CoverageMappingGen.cpp > test/CoverageMapping/macro-expressions.cpp > > Index: test/CoverageMapping/macro-expressions.cpp > =================================================================== > --- test/CoverageMapping/macro-expressions.cpp > +++ test/CoverageMapping/macro-expressions.cpp > @@ -12,6 +12,38 @@ > #define PRIo64 PRI_64_LENGTH_MODIFIER "o" > #define PRIu64 PRI_64_LENGTH_MODIFIER "u" > > +#define STMT(s) s > + > +void fn1() { > + STMT(if (1)); > + STMT(while (1)); > + STMT(for (;;)); > + if (1) > + STMT(if (1)) 0; > + if (1) > + STMT(while (1)) 0; > + if (1) > + STMT(for (;;)) 0; > + while (1) > + STMT(if (1)) 0; > + while (1) > + STMT(while (1)) 0; > + while (1) > + STMT(for (;;)) 0; > + for (;;) > + STMT(if (1)) 0; > + for (;;) > + STMT(while (1)) 0; > + for (;;) > + STMT(for (;;)) 0; > +} > + > +void STMT(fn2()) { > +} > + > +void STMT(fn3)() { > +} > + > // CHECK: foo > // CHECK-NEXT: File 0, [[@LINE+1]]:17 -> {{[0-9]+}}:2 = #0 > void foo(int i) { > Index: lib/CodeGen/CoverageMappingGen.cpp > =================================================================== > --- lib/CodeGen/CoverageMappingGen.cpp > +++ lib/CodeGen/CoverageMappingGen.cpp > @@ -807,6 +807,14 @@ > // counter for the body when looking at the coverage. > propagateCounts(ParentCount, S->getCond()); > > + // The region containing the condition may be spanned by an expansion. > + // Make sure we handle a file exit out of this expansion before handling > + // the consequent branch. > + if (SM.isBeforeInTranslationUnit(getStart(S->getCond()), > + S->getCond()->getLocStart())) { > + MostRecentLocation = getEnd(S->getCond()); > + } Can/should this be folded into propagateCounts? I can't think of a place where that would be harmful, but I guess if it can't come up in other situations this is fine here. > + > extendRegion(S->getThen()); > Counter OutCount = propagateCounts(ThenCount, S->getThen()); > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
