r240571 - [Preprocessor] Iterating over all macros should include those from modules.

[Richard Smith]

On Thu, Jun 25, 2015 at 1:19 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On Thu, Jun 25, 2015 at 12:53 PM, Jordan Rose <jordan_rose at apple.com>
> wrote:
>
>>
>> On Jun 25, 2015, at 11:54, Justin Bogner <mail at justinbogner.com> wrote:
>>
>> Jordan Rose <jordan_rose at apple.com> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150625/6405b810/attachment.html>