<div dir="ltr">I don't think this actually solves the problem: InclusionRewriter::InclusionDirective also copies around a FileChange with an uninitialized FileType, so I think you'll need to initialize it or give it a copy constructor that doesn't copy the FileType until the Id has been set.<div><br></div><div>Ultimately, it seems like the design here is at fault; maybe we should be holding onto the HashLoc and Imported when we reach InclusionDirective, and not inserting the value into the map until we reach FileChanged and know the complete value we want to insert?<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 30, 2015 at 4:58 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">ping<br>
<div><div class="h5"><br>
Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> writes:<br>
> If a FileChange has an invalid file, the FileType won't be initialized,<br>
> so it's undefined to pass it into Process like this. We immediately bail<br>
> out because the FileID is invalid in that case, so we can avoid the<br>
> issue by checking that first.<br>
><br>
> The alternative is to initialize FileType in FileChange's constructor,<br>
> but it's not clear if there's a reasonable default. Should I do that<br>
> instead, or is this good to commit?<br>
><br>
</div></div>> commit bea2df0fed8f0ce08daa23d0dfc21aead1d12e63<br>
> Author: Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>><br>
> Date:   Mon Jun 22 12:45:00 2015 -0700<br>
><br>
>     Frontend: Avoid some UB by checking a FileChange's validity before a call<br>
<span class="">><br>
>     If a FileChange has an invalid file, the FileType won't be<br>
>     initialized, so it's undefined to pass it into Process like this. We<br>
>     immediately bail out because the FileID is invalid in that case, so we<br>
>     can avoid the issue by checking that first.<br>
><br>
</span>> diff --git a/lib/Frontend/Rewrite/InclusionRewriter.cpp b/lib/Frontend/Rewrite/InclusionRewriter.cpp<br>
> index b9ea051..2ab18cf 100644<br>
> --- a/lib/Frontend/Rewrite/InclusionRewriter.cpp<br>
> +++ b/lib/Frontend/Rewrite/InclusionRewriter.cpp<br>
> @@ -439,7 +439,8 @@ bool InclusionRewriter::Process(FileID FileId,<br>
>                  WriteImplicitModuleImport(Change->Mod);<br>
><br>
>                // else now include and recursively process the file<br>
> -              } else if (Process(Change->Id, Change->FileType)) {<br>
> +              } else if (!Change->Id.isInvalid() &&<br>
> +                         Process(Change->Id, Change->FileType)) {<br>
>                  // and set lineinfo back to this file, if the nested one was<br>
>                  // actually included<br>
>                  // `2' indicates returning to a file (after having included<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div></div>