<div dir="ltr">good idea, sending a follow up.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 7, 2022 at 11:07 AM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Might be worth a comment ath the FPICache declaration explaining that<br>
it does need std::map's invalidation (or non-invalidation) semantics?<br>
<br>
On Fri, Nov 4, 2022 at 4:07 PM Mircea Trofin via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
><br>
> Author: Mircea Trofin<br>
> Date: 2022-11-04T16:07:24-07:00<br>
> New Revision: 5617fb1411f765667c016b5b75daa9d1110c36af<br>
><br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/5617fb1411f765667c016b5b75daa9d1110c36af" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/5617fb1411f765667c016b5b75daa9d1110c36af</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/5617fb1411f765667c016b5b75daa9d1110c36af.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/5617fb1411f765667c016b5b75daa9d1110c36af.diff</a><br>
><br>
> LOG: [MLGO][NFC] Use std::map instead of DenseMap to avoid use after free<br>
><br>
> In `MLInlineAdvisor::getAdviceImpl`, we call `getCachedFPI` twice, once<br>
> for the caller, once for the callee, so the second may invalidate the<br>
> reference obtained by the first because the underlying implementation of<br>
> the cache is a `DenseMap`. `std::map` doesn't have that problem.<br>
><br>
> Added:<br>
><br>
><br>
> Modified:<br>
>     llvm/include/llvm/Analysis/MLInlineAdvisor.h<br>
>     llvm/lib/Analysis/MLInlineAdvisor.cpp<br>
><br>
> Removed:<br>
><br>
><br>
><br>
> ################################################################################<br>
> diff  --git a/llvm/include/llvm/Analysis/MLInlineAdvisor.h b/llvm/include/llvm/Analysis/MLInlineAdvisor.h<br>
> index 00e8d7d7dd4de..3db948d365c77 100644<br>
> --- a/llvm/include/llvm/Analysis/MLInlineAdvisor.h<br>
> +++ b/llvm/include/llvm/Analysis/MLInlineAdvisor.h<br>
> @@ -69,7 +69,7 @@ class MLInlineAdvisor : public InlineAdvisor {<br>
>    getSkipAdviceIfUnreachableCallsite(CallBase &CB);<br>
>    void print(raw_ostream &OS) const override;<br>
><br>
> -  mutable DenseMap<const Function *, FunctionPropertiesInfo> FPICache;<br>
> +  mutable std::map<const Function *, FunctionPropertiesInfo> FPICache;<br>
><br>
>    LazyCallGraph &CG;<br>
><br>
><br>
> diff  --git a/llvm/lib/Analysis/MLInlineAdvisor.cpp b/llvm/lib/Analysis/MLInlineAdvisor.cpp<br>
> index f55de71ea98ae..a20c05243b773 100644<br>
> --- a/llvm/lib/Analysis/MLInlineAdvisor.cpp<br>
> +++ b/llvm/lib/Analysis/MLInlineAdvisor.cpp<br>
> @@ -415,8 +415,8 @@ void MLInlineAdvisor::print(raw_ostream &OS) const {<br>
>       << " EdgesOfLastSeenNodes: " << EdgesOfLastSeenNodes << "\n";<br>
>    OS << "[MLInlineAdvisor] FPI:\n";<br>
>    for (auto I : FPICache) {<br>
> -    OS << I.getFirst()->getName() << ":\n";<br>
> -    I.getSecond().print(OS);<br>
> +    OS << I.first->getName() << ":\n";<br>
> +    I.second.print(OS);<br>
>      OS << "\n";<br>
>    }<br>
>    OS << "\n";<br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>