[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

Kirill Stoimenov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 3 14:13:54 PDT 2022


kstoimenov added inline comments.


================
Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:48-50
+  GVSanitizerMetadata Meta;
+  if (GV->hasSanitizerMetadata())
+    Meta = GV->getSanitizerMetadata();
----------------
hctim wrote:
> kstoimenov wrote:
> > Would it make sense to create two separate functions something like reportGlobalNew and reportGlobalLegacy to make it more clear which one is which? Then you code will be something like the one below? You can come up with better names for functions, I am sure. 
> > 
> > ```
> > reportGlobal(...) {
> >   reportGlobalNew(...);
> >   reportGlobalLegacy(...);
> > }
> > ```
> > 
> I think there's too much crossover (like calculating `IsExcluded` and setting of `Meta.Sanitizer`) to warrant splitting them out; and the follow-up patch is going to immediately delete the legacy code anyway :).
In that case, could you please add a FIXME comment indicating which code is supposed to be deleted? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126929/new/

https://reviews.llvm.org/D126929



More information about the cfe-commits mailing list