On Thu, Jun 25, 2015 at 1:19 PM, Richard Smith <[email protected]> wrote:
> On Thu, Jun 25, 2015 at 12:53 PM, Jordan Rose <[email protected]> > wrote: > >> >> On Jun 25, 2015, at 11:54, Justin Bogner <[email protected]> wrote: >> >> Jordan Rose <[email protected]> writes: >> >> Author: jrose >> Date: Wed Jun 24 14:27:02 2015 >> New Revision: 240571 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=240571&view=rev >> Log: >> [Preprocessor] Iterating over all macros should include those from >> modules. >> >> So, iterate over the list of macros mentioned in modules, and make sure >> those >> are in the master table. >> >> >> This introduces a null dereference in Preprocessor::findDirectiveAtLoc. >> It triggers in clang/test/Modules/crashes.m, so you can find it by >> adding an assert next to the very relevant sounding FIXME like so: >> >> --- a/include/clang/Lex/Preprocessor.h >> +++ b/include/clang/Lex/Preprocessor.h >> @@ -454,6 +454,7 @@ class Preprocessor : public >> RefCountedBase<Preprocessor> { >> MacroDirective::DefInfo findDirectiveAtLoc(SourceLocation Loc, >> SourceManager &SourceMgr) >> const { >> // FIXME: Incorporate module macros into the result of this. >> + assert(getLatest() != nullptr); >> return getLatest()->findDirectiveAtLoc(Loc, SourceMgr); >> } >> >> I found this with ubsan, but I still need a couple of clang patches >> reviewed before I can make a bot to catch such things. For anyone >> interested: >> >> >> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131431.html >> >> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131432.html >> >> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131433.html >> >> >> Hm, thanks. I'm not sure whether this means we/I should be more >> circumspect about what gets added to the Macros table, or whether >> findDirectiveAtLoc should not assume that everything in the Macros table is >> currently valid. >> > > I think this is a bug in findDirectiveAtLoc(): it's assuming that every > MacroState has at least one local MacroDirective, which is not necessarily > the case. I think we can just return a default-constructed DefInfo here if > there is no latest macro directive, at least for now. (Longer-term, it'd be > nice if providing a location within a module would get the latest macro > directive visible at that location within that module, but it doesn't seem > like a particularly high priority.) > Should be fixed in r240691.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
